[m-rev.] for post-commit review: improve error messages for foreign_export pragmas

[Julien Fischer]

Note: a similar change should also be made for the first argument
of the pragma, but that's handled by code that is shared across most
of the foreign language interface pragmas and is best handled separately.

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

Improve error messages for foreign_export pragmas.

compiler/parse_pragma.m:
       Provide more context for error messages about misformed
       foreign_export pragmas.

       Address an XXX: generate a specific error message for the
       case where the third argument of a foreign_export pragma
       is the empty string.

tests/invalid/bad_foreign_export.{m,err}:
       Add a case for the new error message.

       Conform to the above changes.

Julien.

diff --git a/compiler/parse_pragma.m b/compiler/parse_pragma.m
index a7d3c87..be936e0 100644
--- a/compiler/parse_pragma.m
+++ b/compiler/parse_pragma.m
@@ -842,30 +842,18 @@ parse_pragma_foreign_export(VarSet, ErrorTerm, PragmaTerms, Context, SeqNum,
      ( if PragmaTerms = [LangTerm, PredAndModesTerm, FunctionTerm] then
          parse_foreign_language("foreign_export", VarSet, LangTerm,
              MaybeForeignLang),
-        ContextPieces = cord.from_list([words("In"),
-            pragma_decl("foreign_export"), words("declaration:")]),
-        parse_pred_or_func_and_arg_modes(no, VarSet, ContextPieces,
+        PredAndModesContextPieces = cord.from_list([
+            words("In second argument of"), pragma_decl("foreign_export"),
+            words("declaration:")
+        ]),
+        parse_pred_or_func_and_arg_modes(no, VarSet, PredAndModesContextPieces,
              PredAndModesTerm, MaybePredAndModes),
-        ( if FunctionTerm = term.functor(term.string(Function0), [], _) then
-            MaybeFunction = ok1(Function0)
-            % XXX TODO: do some additional checks here:
-            % 1. check that Function0 is not the empty string.
-            % 2. if we have a valid foreign language, check that Function0
-            %    is a valid identifier in that language.
-        else
-            FunctionPieces = [
-                words("In"), pragma_decl("foreign_export"),
-                words("declaration: error:"),
-                words("expected a string that is the foreign language name of"),
-                words("the exported procedure at"),
-                quote(describe_error_term(VarSet, FunctionTerm)),
-                suffix("."), nl
-            ],
-            FunctionSpec = error_spec(severity_error, phase_term_to_parse_tree,
-                [simple_msg(get_term_context(FunctionTerm),
-                    [always(FunctionPieces)])]),
-            MaybeFunction = error1([FunctionSpec])
-        ),
+        ForeignFunctionContextPieces = cord.from_list([
+            words("In third argument of"), pragma_decl("foreign_export"),
+            words("declaration:")
+        ]),
+        parse_foreign_function_name(VarSet, ForeignFunctionContextPieces,
+            FunctionTerm, MaybeFunction),
          ( if
              MaybeForeignLang = ok1(ForeignLang),
              MaybePredAndModes = ok3(PredName, PredOrFunc, Modes),
@@ -893,6 +881,40 @@ parse_pragma_foreign_export(VarSet, ErrorTerm, PragmaTerms, Context, SeqNum,
          MaybeIOM = error1([Spec])
      ).

+:- pred parse_foreign_function_name(varset::in, cord(format_component)::in,
+    term::in, maybe1(string)::out) is det.
+
+parse_foreign_function_name(VarSet, ContextPieces, FunctionTerm,
+        MaybeFunction) :-
+    ( if FunctionTerm = term.functor(term.string(Function), [], _) then
+        ( if Function = "" then
+            EmptyNamePieces = cord.list(ContextPieces) ++ [
+                words("error: expected a non-empty string for the"),
+                words("foreign language name of the exported procedure,"),
+                words("got empty string.")
+            ],
+            FunctionSpec = error_spec(severity_error, phase_term_to_parse_tree,
+                [simple_msg(get_term_context(FunctionTerm),
+                    [always(EmptyNamePieces)])]),
+            MaybeFunction = error1([FunctionSpec])
+        else
+            % XXX TODO: if we have a valid foreign language, check that
+            % Function is a valid identifier in that language.
+            MaybeFunction = ok1(Function)
+        )
+    else
+        FunctionPieces = cord.list(ContextPieces) ++ [
+            words("error: expected a non-empty string for the foreign"),
+            words("language name of the exported procedure, got"),
+            quote(describe_error_term(VarSet, FunctionTerm)),
+            suffix("."), nl
+        ],
+        FunctionSpec = error_spec(severity_error, phase_term_to_parse_tree,
+            [simple_msg(get_term_context(FunctionTerm),
+                [always(FunctionPieces)])]),
+        MaybeFunction = error1([FunctionSpec])
+    ).
+
  %---------------------------------------------------------------------------%

  :- pred parse_pragma_foreign_import_module(varset::in, term::in,
diff --git a/tests/invalid/bad_foreign_export.err_exp b/tests/invalid/bad_foreign_export.err_exp
index e8f51ae..05b1f5e 100644
--- a/tests/invalid/bad_foreign_export.err_exp
+++ b/tests/invalid/bad_foreign_export.err_exp
@@ -1,16 +1,22 @@
  bad_foreign_export.m:020: Error: wrong number of arguments in
  bad_foreign_export.m:020:   `:- pragma foreign_export' declaration.
-bad_foreign_export.m:024: In `:- pragma foreign_export' declaration: error:
-bad_foreign_export.m:024:   atom expected at 11111.
-bad_foreign_export.m:028: In `:- pragma foreign_export' declaration: error:
-bad_foreign_export.m:028:   expected a string that is the foreign language name
-bad_foreign_export.m:028:   of the exported procedure at `22222'.
+bad_foreign_export.m:024: In second argument of `:- pragma foreign_export'
+bad_foreign_export.m:024:   declaration: error: atom expected at 11111.
+bad_foreign_export.m:028: In third argument of `:- pragma foreign_export'
+bad_foreign_export.m:028:   declaration: error: expected a non-empty string for
+bad_foreign_export.m:028:   the foreign language name of the exported
+bad_foreign_export.m:028:   procedure, got `22222'.
  bad_foreign_export.m:032: Error: invalid foreign language `"InvalidLanguage"'
  bad_foreign_export.m:032:   in `:- pragma foreign_export' declaration.
  bad_foreign_export.m:037: Error: invalid foreign language `"InvalidLanguage"'
  bad_foreign_export.m:037:   in `:- pragma foreign_export' declaration.
-bad_foreign_export.m:038: In `:- pragma foreign_export' declaration: error:
-bad_foreign_export.m:038:   atom expected at 3333.
-bad_foreign_export.m:039: In `:- pragma foreign_export' declaration: error:
-bad_foreign_export.m:039:   expected a string that is the foreign language name
-bad_foreign_export.m:039:   of the exported procedure at `4444'.
+bad_foreign_export.m:038: In second argument of `:- pragma foreign_export'
+bad_foreign_export.m:038:   declaration: error: atom expected at 3333.
+bad_foreign_export.m:039: In third argument of `:- pragma foreign_export'
+bad_foreign_export.m:039:   declaration: error: expected a non-empty string for
+bad_foreign_export.m:039:   the foreign language name of the exported
+bad_foreign_export.m:039:   procedure, got `4444'.
+bad_foreign_export.m:044: In third argument of `:- pragma foreign_export'
+bad_foreign_export.m:044:   declaration: error: expected a non-empty string for
+bad_foreign_export.m:044:   the foreign language name of the exported
+bad_foreign_export.m:044:   procedure, got empty string.
diff --git a/tests/invalid/bad_foreign_export.m b/tests/invalid/bad_foreign_export.m
index d497a28..dfa5fa3 100644
--- a/tests/invalid/bad_foreign_export.m
+++ b/tests/invalid/bad_foreign_export.m
@@ -38,3 +38,7 @@ foo(X, Y) = X + Y.
      3333,
      4444
  ).
+
+    % Third arg is the empty string.
+    %
+:- pragma foreign_export("C", foo(in, in) = out, "").