[m-rev.] for review: add --warn-suspicious-foreign-code

Julien Fischer jfischer at opturion.com
Fri Sep 28 22:44:47 AEST 2018


For review by anyone.

-----------------------------

Add --warn-suspicious-foreign-code.

This causes the compiler to check the bodies of foreign_code pragmas in a
similar fashion to what --warn-suspicious-foreign-procs does for foreign_proc
pragmas.  Currently, the only check that is performed is for the presence of
MR_ALLOC_ID in the pragma bodies.

compiler/options.m:
      Add the new option.

compiler/add_pragma.m:
compiler/make_hlds_warn.m:
      Implement the new check.

doc/user_guide.texi:
      Document the new option.

tests/warnings/Mmakefile:
tests/warnings/Mercury.options:
tests/warnings/suspicious_foreign_code.{m,exp}:
      Add a test of the new warning.

Julien.

diff --git a/compiler/add_pragma.m b/compiler/add_pragma.m
index befc6c1..13cfae8 100644
--- a/compiler/add_pragma.m
+++ b/compiler/add_pragma.m
@@ -92,6 +92,7 @@
  :- import_module hlds.make_hlds.add_foreign_proc.
  :- import_module hlds.make_hlds.add_pragma.add_pragma_tabling.
  :- import_module hlds.make_hlds.add_pragma.add_pragma_type_spec.
+:- import_module hlds.make_hlds.make_hlds_warn.
  :- import_module hlds.make_hlds_error.
  :- import_module hlds.pred_table.
  :- import_module libs.
@@ -155,6 +156,7 @@ add_pass_2_pragma(SectionItem, !ModuleInfo, !Specs) :-
          Pragma = pragma_foreign_code(FCInfo),
          % XXX STATUS Check ItemMercuryStatus
          FCInfo = pragma_info_foreign_code(Lang, BodyCode),
+        warn_suspicious_foreign_code(Lang, BodyCode, Context, !Specs),
          ForeignBodyCode = foreign_body_code(Lang, BodyCode, Context),
          module_add_foreign_body_code(ForeignBodyCode, !ModuleInfo)
      ;
diff --git a/compiler/make_hlds_warn.m b/compiler/make_hlds_warn.m
index 2089e84..e69891e 100644
--- a/compiler/make_hlds_warn.m
+++ b/compiler/make_hlds_warn.m
@@ -68,6 +68,14 @@
  :- pred check_promise_ex_decl(prog_vars::in, promise_type::in, goal::in,
      prog_context::in, list(error_spec)::in, list(error_spec)::out) is det.

+    % Warn about suspicious things in the bodies of foreign_code pragmas.
+    % Currently, this just checks for the presence of the MR_ALLOC_ID macro
+    % inside the bodies of a foreign_code pragmas.
+    %
+:- pred warn_suspicious_foreign_code(foreign_language::in,
+    foreign_literal_or_include::in, prog_context::in,
+    list(error_spec)::in, list(error_spec)::out) is det.
+
  %----------------------------------------------------------------------------%
  %----------------------------------------------------------------------------%

@@ -891,5 +899,43 @@ promise_ex_error(PromiseType, Context, Message, !Specs) :-
      !:Specs = [Spec | !.Specs].

  %-----------------------------------------------------------------------------%
+
+warn_suspicious_foreign_code(Lang, BodyCode, Context, !Specs) :-
+    (
+        BodyCode = floi_include_file(_)
+    ;
+        BodyCode = floi_literal(Code),
+        (
+            Lang = lang_c,
+            c_code_to_name_list(Code, C_CodeList),
+            ( if list.member("MR_ALLOC_ID", C_CodeList) then
+                Pieces = [
+                    words("Warning: the body of this"),
+                    pragma_decl("foreign_code"),
+                    words("declaration may refer to the"),
+                    quote("MR_ALLOC_ID"), words("macro."),
+                    words("That macro is only defined within the body of"),
+                    pragma_decl("foreign_proc"), words("declarations.")
+                ],
+                Msg = simple_msg(Context,
+                    [option_is_set(warn_suspicious_foreign_code, yes,
+                    [always(Pieces)])]),
+                Severity = severity_conditional(
+                    warn_suspicious_foreign_code, yes,
+                    severity_warning, no),
+                Spec = error_spec(Severity, phase_parse_tree_to_hlds, [Msg]),
+                !:Specs = [Spec | !.Specs]
+            else
+                true
+            )
+        ;
+            ( Lang = lang_csharp
+            ; Lang = lang_java
+            ; Lang = lang_erlang
+            )
+        )
+    ).
+
+%-----------------------------------------------------------------------------%
  :- end_module hlds.make_hlds.make_hlds_warn.
  %-----------------------------------------------------------------------------%
diff --git a/compiler/options.m b/compiler/options.m
index 03aed55..9e7cf45 100644
--- a/compiler/options.m
+++ b/compiler/options.m
@@ -193,6 +193,7 @@
      ;       inform_incomplete_switch_threshold
      ;       warn_unresolved_polymorphism
      ;       warn_suspicious_foreign_procs
+    ;       warn_suspicious_foreign_code
      ;       warn_state_var_shadowing
      ;       inform_inferred
      ;       inform_inferred_types
@@ -1209,6 +1210,7 @@ option_defaults_2(warning_option, [
      inform_incomplete_switch_threshold  -   int(0),
      warn_unresolved_polymorphism        -   bool(yes),
      warn_suspicious_foreign_procs       -   bool(no),
+    warn_suspicious_foreign_code        -   bool(no),
      warn_state_var_shadowing            -   bool(yes),
      inform_inferred                     -   bool_special,
      inform_inferred_types               -   bool(yes),
@@ -2121,6 +2123,7 @@ long_option("inform-incomplete-switch-threshold",
                      inform_incomplete_switch_threshold).
  long_option("warn-unresolved-polymorphism", warn_unresolved_polymorphism).
  long_option("warn-suspicious-foreign-procs", warn_suspicious_foreign_procs).
+long_option("warn-suspicious-foreign-code", warn_suspicious_foreign_code).
  long_option("warn-state-var-shadowing", warn_state_var_shadowing).
  long_option("inform-inferred",          inform_inferred).
  long_option("inform-inferred-types",    inform_inferred_types).
@@ -3942,6 +3945,9 @@ options_help_warning -->
          "--warn-suspicious-foreign-procs",
          "\tWarn about possible errors in the bodies of foreign",
          "\tprocedures.",
+        "--warn-suspicious-foreign-code",
+        "\tWarn about possible errors in the bodies of foreign code",
+        "\tpragmas.",
          "--no-warn-state-var-shadowing",
          "\tDo not warn about one state variable shadowing another.",
          "--no-inform-inferred",
diff --git a/doc/user_guide.texi b/doc/user_guide.texi
index a687884..0eb6880 100644
--- a/doc/user_guide.texi
+++ b/doc/user_guide.texi
@@ -6832,6 +6832,14 @@ is limited some warnings reported by this option may be spurious and
  some actual errors may not be detected at all.

  @sp 1
+ at item --warn-suspicious-foreign-code
+ at findex --warn-suspicious-forign-code
+Warn about possible errors in the bodies of foreign code pragmas.
+Note that since the compiler's ability to parse foreign language code
+is limited some warnings reported by this option may be spurious and
+some actual errors may not be detected at all.
+
+ at sp 1
  @item --no-warn-state-var-shadowing
  @findex --no-warn-state-var-shadowing
  Do not warn about one state variable shadowing another.
diff --git a/tests/warnings/Mercury.options b/tests/warnings/Mercury.options
index 6d6338b..37ee193 100644
--- a/tests/warnings/Mercury.options
+++ b/tests/warnings/Mercury.options
@@ -80,3 +80,4 @@ MCFLAGS-inconsistent_pred_order = \
  MCFLAGS-warn_self_import	= --warn-simple-code
  MCFLAGS-warn_return  		= --warn-suspicious-foreign-procs
  MCFLAGS-warn_succ_ind		= --warn-suspicious-foreign-procs
+MCFLAGS-suspicious_foreign_code = --warn-suspicious-foreign-code
diff --git a/tests/warnings/Mmakefile b/tests/warnings/Mmakefile
index 7df789e..6216631 100644
--- a/tests/warnings/Mmakefile
+++ b/tests/warnings/Mmakefile
@@ -42,6 +42,7 @@ ERRORCHECK_PROGS = \
  	singleton_test_state_var \
  	spurious_obsolete \
  	state_vars_test \
+	suspicious_foreign_code \
  	table_with_inline \
  	unify_f_g \
  	unify_x_f_x \
diff --git a/tests/warnings/suspicious_foreign_code.exp b/tests/warnings/suspicious_foreign_code.exp
index e69de29..a3938ef 100644
--- a/tests/warnings/suspicious_foreign_code.exp
+++ b/tests/warnings/suspicious_foreign_code.exp
@@ -0,0 +1,5 @@
+suspicious_foreign_code.m:010: Warning: the body of this
+suspicious_foreign_code.m:010:   `:- pragma foreign_code' declaration may refer
+suspicious_foreign_code.m:010:   to the `MR_ALLOC_ID' macro. That macro is only
+suspicious_foreign_code.m:010:   defined within the body of
+suspicious_foreign_code.m:010:   `:- pragma foreign_proc' declarations.
diff --git a/tests/warnings/suspicious_foreign_code.m b/tests/warnings/suspicious_foreign_code.m
index e69de29..4a2c75d 100644
--- a/tests/warnings/suspicious_foreign_code.m
+++ b/tests/warnings/suspicious_foreign_code.m
@@ -0,0 +1,13 @@
+% Test --warn-suspicious-foreign-code
+
+:- module suspicious_foreign_code.
+:- interface.
+
+:- type foo ---> foo.
+
+:- implementation.
+
+:- pragma foreign_code("C", "
+
+static int X = MR_ALLOC_ID;
+").


More information about the reviews mailing list