[m-rev.] for review: fix bug #183

Julien Fischer jfischer at opturion.com
Wed May 22 15:28:56 AEST 2013


On Wed, 22 May 2013, Peter Wang wrote:

> On Wed, 22 May 2013 14:30:00 +1000 (EST), Julien Fischer <jfischer at opturion.com> wrote:
>> diff --git a/compiler/dead_proc_elim.m b/compiler/dead_proc_elim.m
>> index 121b0fd..8068aac 100644
>> --- a/compiler/dead_proc_elim.m
>> +++ b/compiler/dead_proc_elim.m
>> @@ -906,13 +906,16 @@ dead_proc_eliminate_proc(PredId, Keep, WarnForThisProc, ProcElimInfo,
>>               VeryVerbose = no
>>           ),
>>           map.lookup(!.ProcTable, ProcId, ProcInfo0),
>> +        proc_info_get_has_any_foreign_exports(ProcInfo0, HasForeignExports),
>>           (
>>               WarnForThisProc = yes,
>> +            HasForeignExports = no
>> +        ->
>>               proc_info_get_context(ProcInfo0, Context),
>>               Spec = warn_dead_proc(PredId, ProcId, Context, ModuleInfo),
>>               !:Specs = [Spec | !.Specs]
>>           ;
>> -            WarnForThisProc = no
>> +            true
>>           ),
>>           map.delete(ProcId, !ProcTable)
>>       ).
>
> WarnForThisProc should probably be WarnForThisPred.

Agreed.
>
>> diff --git a/compiler/hlds_pred.m b/compiler/hlds_pred.m
>> index 933c71b..8bd7648 100644
>> --- a/compiler/hlds_pred.m
>> +++ b/compiler/hlds_pred.m
> ...
>> @@ -2475,7 +2479,12 @@ attribute_list_to_attributes(Attributes, Attributes).
>>                   % Is the procedure mentioned in any order-independent-state-
>>                   % update pragmas? If yes, list the role of this procedure
>>                   % for the each of the types in those pragmas.
>> -                psi_oisu_kind_fors          :: list(oisu_pred_kind_for)
>> +                psi_oisu_kind_fors          :: list(oisu_pred_kind_for),
>> +
>> +                % Is the procedure mentioned in any foreign_export pragma,
>> +                % regardless of what the current supported foreign languages
>> +                % are?
>> +                psi_has_any_foreign_exports  :: bool
>>           ).
>
> A purpose-specific type would be better.

Done, diff follows.

Cheers,
Julien.


diff --git a/compiler/add_pragma.m b/compiler/add_pragma.m
index 9062446..e5219d6 100644
--- a/compiler/add_pragma.m
+++ b/compiler/add_pragma.m
@@ -506,7 +506,8 @@ add_pragma_foreign_export_2(Arity, PredTable, Origin, Lang, Name, PredId,
              % languages other than those languages that are supported by the
              % current backend.)
              %
-            proc_info_set_has_any_foreign_exports(yes, ProcInfo0, ProcInfo), 
+            proc_info_set_has_any_foreign_exports(has_foreign_exports,
+                ProcInfo0, ProcInfo),
              module_info_set_pred_proc_info(PredId, ProcId, PredInfo, ProcInfo,
                  !ModuleInfo)
          )
diff --git a/compiler/dead_proc_elim.m b/compiler/dead_proc_elim.m
index 8068aac..8630aa9 100644
--- a/compiler/dead_proc_elim.m
+++ b/compiler/dead_proc_elim.m
@@ -789,7 +789,7 @@ dead_proc_eliminate_pred(ElimOptImported, PredId, !ProcElimInfo) :-
                  % since they may be automatically generated
                  is_unify_or_compare_pred(PredInfo0)
              ->
-                WarnForThisProc = no
+                WarnForThisPred = no
              ;
                  % Don't warn for procedures introduced from lambda expressions.
                  % The only time those procedures will be unused is if the
@@ -802,24 +802,24 @@ dead_proc_eliminate_pred(ElimOptImported, PredId, !ProcElimInfo) :-
                  ; string.prefix(PredName, "TypeSpecOf__")
                  )
              ->
-                WarnForThisProc = no
+                WarnForThisPred = no
              ;
-                WarnForThisProc = yes
+                WarnForThisPred = yes
              )
          ;
              Status = status_pseudo_imported,
              Keep = no,
-            WarnForThisProc = no
+            WarnForThisPred = no
          ;
              Status = status_pseudo_exported,
              hlds_pred.in_in_unification_proc_id(InitProcId),
              Keep = yes(InitProcId),
-            WarnForThisProc = no
+            WarnForThisPred = no
          )
      ->
          ProcIds = pred_info_procids(PredInfo0),
          pred_info_get_procedures(PredInfo0, ProcTable0),
-        list.foldl3(dead_proc_eliminate_proc(PredId, Keep, WarnForThisProc,
+        list.foldl3(dead_proc_eliminate_proc(PredId, Keep, WarnForThisPred,
              !.ProcElimInfo),
              ProcIds, ProcTable0, ProcTable, Changed0, Changed, Specs0, Specs),
          pred_info_set_procedures(ProcTable, PredInfo0, PredInfo),
@@ -877,7 +877,7 @@ dead_proc_eliminate_pred(ElimOptImported, PredId, !ProcElimInfo) :-
      proc_elim_info::in, proc_id::in, proc_table::in, proc_table::out,
      bool::in, bool::out, list(error_spec)::in, list(error_spec)::out) is det.

-dead_proc_eliminate_proc(PredId, Keep, WarnForThisProc, ProcElimInfo,
+dead_proc_eliminate_proc(PredId, Keep, WarnForThisPred, ProcElimInfo,
          ProcId, !ProcTable, !Changed, !Specs) :-
      Needed = ProcElimInfo ^ proc_elim_needed_map,
      ModuleInfo = ProcElimInfo ^ proc_elim_module_info,
@@ -908,8 +908,8 @@ dead_proc_eliminate_proc(PredId, Keep, WarnForThisProc, ProcElimInfo,
          map.lookup(!.ProcTable, ProcId, ProcInfo0),
          proc_info_get_has_any_foreign_exports(ProcInfo0, HasForeignExports),
          (
-            WarnForThisProc = yes,
-            HasForeignExports = no
+            WarnForThisPred = yes,
+            HasForeignExports = no_foreign_exports
          ->
              proc_info_get_context(ProcInfo0, Context),
              Spec = warn_dead_proc(PredId, ProcId, Context, ModuleInfo),
diff --git a/compiler/hlds_pred.m b/compiler/hlds_pred.m
index 8bd7648..4c51c8a 100644
--- a/compiler/hlds_pred.m
+++ b/compiler/hlds_pred.m
@@ -1998,6 +1998,12 @@ attribute_list_to_attributes(Attributes, Attributes).
      --->    oisu_creator_for(type_ctor)
      ;       oisu_mutator_for(type_ctor)
      ;       oisu_destructor_for(type_ctor).
+ 
+    % Is a procedure the subject of any foreign_export pragmas?
+    %
+:- type proc_foreign_exports
+    --->    no_foreign_exports
+    ;       has_foreign_exports.

      % Predicates to get fields of proc_infos.

@@ -2056,7 +2062,7 @@ attribute_list_to_attributes(Attributes, Attributes).
  :- pred proc_info_get_oisu_kind_fors(proc_info::in,
      list(oisu_pred_kind_for)::out) is det.
  :- pred proc_info_get_has_any_foreign_exports(proc_info::in,
-    bool::out) is det.
+    proc_foreign_exports::out) is det.

      % Predicates to set fields of proc_infos.

@@ -2127,7 +2133,7 @@ attribute_list_to_attributes(Attributes, Attributes).
      proc_info::in, proc_info::out) is det.
  :- pred proc_info_set_oisu_kind_fors(list(oisu_pred_kind_for)::in,
      proc_info::in, proc_info::out) is det.
-:- pred proc_info_set_has_any_foreign_exports(bool::in,
+:- pred proc_info_set_has_any_foreign_exports(proc_foreign_exports::in,
      proc_info::in, proc_info::out) is det.

  :- pred proc_info_get_termination2_info(proc_info::in,
@@ -2484,7 +2490,7 @@ attribute_list_to_attributes(Attributes, Attributes).
                  % Is the procedure mentioned in any foreign_export pragma,
                  % regardless of what the current supported foreign languages
                  % are? 
-                psi_has_any_foreign_exports  :: bool
+                psi_has_any_foreign_exports :: proc_foreign_exports
          ).

  :- type structure_sharing_info
@@ -2615,7 +2621,7 @@ proc_info_init(MContext, Arity, Types, DeclaredModes, Modes, MaybeArgLives,
      ProcSubInfo = proc_sub_info(DetismDecl, no, no, Term2Info, IsAddressTaken,
          StackSlots, RegR_HeadVars, ArgInfo, InitialLiveness, no, no,
          no, no_tail_call_events, no, no, no, no, no, no, VarNameRemap, [],
-        SharingInfo, ReuseInfo, [], no),
+        SharingInfo, ReuseInfo, [], no_foreign_exports),
      ProcInfo = proc_info(MContext, BodyVarSet, BodyTypes, HeadVars, InstVarSet,
          DeclaredModes, Modes, no, MaybeArgLives, MaybeDet, InferredDet,
          ClauseBody, CanProcess, ModeErrors, RttiVarMaps, eval_normal,
@@ -2649,7 +2655,7 @@ proc_info_create_with_declared_detism(Context, VarSet, VarTypes, HeadVars,
      ProcSubInfo = proc_sub_info(DetismDecl, no, no, Term2Info, IsAddressTaken,
          StackSlots, RegR_HeadVars, no, Liveness, no, no, no,
          no_tail_call_events, no, no, no, no, no, no, VarNameRemap, [],
-        SharingInfo, ReuseInfo, [], no),
+        SharingInfo, ReuseInfo, [], no_foreign_exports),
      ProcInfo = proc_info(Context, VarSet, VarTypes, HeadVars,
          InstVarSet, no, HeadModes, no, MaybeHeadLives,
          MaybeDeclaredDetism, Detism, Goal, yes, ModeErrors,



More information about the reviews mailing list