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

Julien Fischer jfischer at opturion.com
Wed May 22 14:30:00 AEST 2013


For review by anyone
-------------------

Fix bug #183

--warn-dead-procs should not emit warnings for procedures that are foreign
exported.  However, for procedures that were foreign exported only to languages
that were *not* supported by the current backend, it did emit warnings.
This was because we only kept track of foreign_export pragmas for languages
that were supported by the current backend.  The fix is to (separately) keep
track of whether there were any foreign_export pragmas at all for a procedure.

compiler/hlds_pred.m:
 	Add a new field to the proc_info structure that keeps track
 	of whether the were any foreign_export pragmas for the procedure.

compiler/add_pragma.m:
 	Fill in the new field.

compiler/dead_proc_elim.m:
 	Only emit a warning about a procedure being dead if it is not
 	the subject of any foreign_export pragmas.

tests/valid/Mmakefile:
tests/valid/bug183.m:
 	Add a regression test for the above.

Julien.

diff --git a/compiler/add_pragma.m b/compiler/add_pragma.m
index 1d3ace2..9062446 100644
--- a/compiler/add_pragma.m
+++ b/compiler/add_pragma.m
@@ -459,8 +459,8 @@ add_pragma_foreign_export_2(Arity, PredTable, Origin, Lang, Name, PredId,
          get_procedure_matching_declmodes_with_renaming(ExistingProcs,
              Modes, !.ModuleInfo, ProcId)
      ->
-        map.lookup(Procs, ProcId, ProcInfo),
-        proc_info_get_declared_determinism(ProcInfo, MaybeDetism),
+        map.lookup(Procs, ProcId, ProcInfo0),
+        proc_info_get_declared_determinism(ProcInfo0, MaybeDetism),
          % We cannot catch those multi or nondet procedures that don't have
          % a determinism declaration until after determinism analysis.
          (
@@ -481,6 +481,9 @@ add_pragma_foreign_export_2(Arity, PredTable, Origin, Lang, Name, PredId,
          ;
              % Only add the foreign export if the specified language matches
              % one of the foreign languages available for this backend.
+            %
+ 
+
              module_info_get_globals(!.ModuleInfo, Globals),
              globals.get_backend_foreign_languages(Globals, ForeignLanguages),
              ( list.member(Lang, ForeignLanguages) ->
@@ -494,7 +497,18 @@ add_pragma_foreign_export_2(Arity, PredTable, Origin, Lang, Name, PredId,
                      !ModuleInfo)
              ;
                  true
-            )
+            ),
+
+            % Record that there was a foreign_export pragma for this procedure,
+            % regardless of the specified language.  We do this so that dead
+            % procedure elimination does not generate incorrect warnings about
+            % dead procedures (e.g.  those that are foreign_exported to
+            % languages other than those languages that are supported by the
+            % current backend.)
+            %
+            proc_info_set_has_any_foreign_exports(yes, ProcInfo0, ProcInfo), 
+            module_info_set_pred_proc_info(PredId, ProcId, PredInfo, ProcInfo,
+                !ModuleInfo)
          )
      ;
          % We do not warn about errors in export pragmas created by the
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)
      ).
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
@@ -2055,6 +2055,8 @@ attribute_list_to_attributes(Attributes, Attributes).
      is det.
  :- 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.

      % Predicates to set fields of proc_infos.

@@ -2125,6 +2127,8 @@ 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,
+    proc_info::in, proc_info::out) is det.

  :- pred proc_info_get_termination2_info(proc_info::in,
      termination2_info::out) is det.
@@ -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
          ).

  :- type structure_sharing_info
@@ -2606,7 +2615,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, []),
+        SharingInfo, ReuseInfo, [], no),
      ProcInfo = proc_info(MContext, BodyVarSet, BodyTypes, HeadVars, InstVarSet,
          DeclaredModes, Modes, no, MaybeArgLives, MaybeDet, InferredDet,
          ClauseBody, CanProcess, ModeErrors, RttiVarMaps, eval_normal,
@@ -2640,7 +2649,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, []),
+        SharingInfo, ReuseInfo, [], no),
      ProcInfo = proc_info(Context, VarSet, VarTypes, HeadVars,
          InstVarSet, no, HeadModes, no, MaybeHeadLives,
          MaybeDeclaredDetism, Detism, Goal, yes, ModeErrors,
@@ -2694,6 +2703,8 @@ proc_info_get_maybe_untuple_info(PI, PI ^ proc_sub_info ^ maybe_untuple_info).
  proc_info_get_var_name_remap(PI, PI ^ proc_sub_info ^ proc_var_name_remap).
  proc_info_get_statevar_warnings(PI, PI ^ proc_sub_info ^ statevar_warnings).
  proc_info_get_oisu_kind_fors(PI, PI ^ proc_sub_info ^ psi_oisu_kind_fors).
+proc_info_get_has_any_foreign_exports(PI,
+    PI ^ proc_sub_info ^ psi_has_any_foreign_exports).

  proc_info_set_varset(VS, !PI) :-
      !PI ^ prog_varset := VS.
@@ -2761,6 +2772,8 @@ proc_info_set_statevar_warnings(SVW, !PI) :-
      !PI ^ proc_sub_info ^ statevar_warnings := SVW.
  proc_info_set_oisu_kind_fors(KFs, !PI) :-
      !PI ^ proc_sub_info ^ psi_oisu_kind_fors := KFs.
+proc_info_set_has_any_foreign_exports(AFE, !PI) :-
+    !PI ^ proc_sub_info ^ psi_has_any_foreign_exports := AFE.

  proc_info_head_modes_constraint(ProcInfo, HeadModesConstraint) :-
      MaybeHeadModesConstraint = ProcInfo ^ maybe_head_modes_constraint,
diff --git a/tests/valid/Mmakefile b/tests/valid/Mmakefile
index 62f01f1..c2d6a2b 100644
--- a/tests/valid/Mmakefile
+++ b/tests/valid/Mmakefile
@@ -74,6 +74,7 @@ OTHER_PROGS= \
  	bug142 \
  	bug159 \
  	bug180 \
+	bug183 \
  	bug190 \
  	bug257b \
  	builtin_false \
diff --git a/tests/valid/bug183.m b/tests/valid/bug183.m
new file mode 100644
index 0000000..d38d187
--- /dev/null
+++ b/tests/valid/bug183.m
@@ -0,0 +1,26 @@
+% Regression test for bug #183.
+%
+% When compiled with --warn-dead-procs and one of the C backends (or indeed the
+% Erlang backend), rotd-2013-05-21 and before emitted an incorrect warning
+% about handle_event_excp/1 being dead.  This was because dead procedure
+% elimination did not take foreign_export pragmas for languages other than
+% those supported by the current backend into account when determining if that
+% warning should be emitted.
+
+:- module bug183.
+:- interface.
+
+:- type foo ---> foo.
+
+:- implementation.
+
+:- import_module univ.
+
+:- impure pred handle_event_excp(string::in, string::in, univ::in) is det.
+:- pragma foreign_export("C#", handle_event_excp(in, in, in),
+    "SSDB_handle_event_excp").
+:- pragma foreign_export("Java", handle_event_excp(in, in, in),
+    "SSDB_handle_event_excp").
+
+handle_event_excp(_, _, _) :-
+	impure impure_true.



More information about the reviews mailing list