[m-rev.] for review: improved error messages for unexpected ::mode suffixes

Mark Brown mark at mercurylang.org
Mon May 23 00:49:53 AEST 2016


Hi Zoltan,

On Sun, May 22, 2016 at 8:42 PM, Zoltan Somogyi
<zoltan.somogyi at runbox.com> wrote:
> I think that is fine, except for one minor thing.
> The .err_exp file now contains wording like this:
>
>   In type definition: in the first argument:
>
> The thing is, a type definition has more than one part
> that has arguments: the type constructor may have arguments,
> in eqv type definitions the equivalent type has arguments,
> in du types, the function symbols have arguments, etc.
> So I think the ContextPieces need have added to them
> the identity of the thing whose arguments you are parsing
> before you parse the arguments. There was an XXX about
> this, but the missing update didn't matter as much,
> because we never added anything to the too-short context;
> now we do.

That wording was also used for when the ::mode suffix was expected but
missing. Attached is a diff that provides the extra context info about
arguments for both cases.

Thanks for the review.

Mark
-------------- next part --------------
commit 0352613f459b0435cf2e2a2d964a63eeb60803aa
Author: Mark Brown <mark at mercurylang.org>
Date:   Mon May 23 00:33:17 2016 +1000

    Improve error messages for combined types and modes.
    
    compiler/parse_type_name.m:
    	Change the predicates parse_type_and_modes and
    	parse_types_no_modes to take a function from argument
    	number to context pieces. Use this to describe not just
    	the argument number but what the argument is an argument
    	of, both when parsing arguments that require a ::mode
    	suffix and when parsing arguments that must not have such
    	a suffix.
    
    compiler/parse_item.m:
    	Just provide the argument number rather than saying what
    	the argument is an argument of, as this information is
    	already in the initial part of the context.
    
    tests/invalid/combined_ho_type_inst.err_exp:
    tests/invalid/combined_ho_type_inst_2.err_exp:
    	Update test outputs.

diff --git a/compiler/parse_item.m b/compiler/parse_item.m
index 3686de2..bd4c628 100644
--- a/compiler/parse_item.m
+++ b/compiler/parse_item.m
@@ -909,8 +909,11 @@ parse_pred_decl_base(PredOrFunc, ModuleName, VarSet, PredTypeTerm,
             MaybeIOM = error1(Specs)
         ;
             MaybePredNameAndArgs = ok2(Functor, ArgTerms),
+            ArgContextFunc = (func(ArgNum) = ContextPieces ++
+                    cord.from_list([words("in the"), nth_fixed(ArgNum),
+                    words("argument:"), nl])),
             parse_type_and_modes(constrain_some_inst_vars(InstConstraints),
-                dont_require_tm_mode, wnhii_pred_arg, VarSet, ContextPieces,
+                dont_require_tm_mode, wnhii_pred_arg, VarSet, ArgContextFunc,
                 ArgTerms, 1, TypesAndModes, [], TMSpecs),
             check_type_and_mode_list_is_consistent(TypesAndModes, no,
                 get_term_context(PredTypeTerm), MaybeTypeModeListKind),
@@ -1005,9 +1008,12 @@ parse_func_decl_base(ModuleName, VarSet, Term, MaybeDet, Context, SeqNum,
                 MaybeIOM = error1(Specs)
             ;
                 MaybeFuncNameAndArgs = ok2(FuncName, ArgTerms),
+                ArgContextFunc = (func(ArgNum) = ContextPieces ++
+                        cord.from_list([words("in the"), nth_fixed(ArgNum),
+                        words("argument:"), nl])),
                 parse_type_and_modes(constrain_some_inst_vars(InstConstraints),
                     dont_require_tm_mode, wnhii_func_arg,
-                    VarSet, ContextPieces, ArgTerms, 1,
+                    VarSet, ArgContextFunc, ArgTerms, 1,
                     ArgTypesAndModes, [], ArgTMSpecs),
                 RetContextPieces = ContextPieces ++
                     cord.from_list([words("in the return value"), nl]),
diff --git a/compiler/parse_type_name.m b/compiler/parse_type_name.m
index 8ab170f..ba15770 100644
--- a/compiler/parse_type_name.m
+++ b/compiler/parse_type_name.m
@@ -73,9 +73,11 @@
     --->    dont_require_tm_mode
     ;       require_tm_mode.
 
+:- type arg_context_func == (func(int) = cord(format_component)).
+
 :- pred parse_type_and_modes(maybe_constrain_inst_vars::in,
     maybe_require_tm_mode::in, why_no_ho_inst_info::in,
-    varset::in, cord(format_component)::in,
+    varset::in, arg_context_func::in,
     list(term)::in, int::in, list(type_and_mode)::out,
     list(error_spec)::in, list(error_spec)::out) is det.
 
@@ -108,6 +110,16 @@
 
 %---------------------------------------------------------------------------%
 
+:- func arg_context_pieces(cord(format_component), pred_or_func, int) =
+    cord(format_component).
+
+arg_context_pieces(ContextPieces, PorF, ArgNum) =
+    ContextPieces ++ cord.from_list([lower_case_next_if_not_first,
+        words("In the"), nth_fixed(ArgNum),
+        words("argument of higher-order"), p_or_f(PorF), words("type:"), nl]).
+
+%---------------------------------------------------------------------------%
+
 maybe_parse_type(AllowHOInstInfo, Term, Type) :-
     % The values of VarSet and ContextPieces do not matter since we succeed
     % only if they aren't used.
@@ -216,8 +228,9 @@ parse_compound_type(AllowHOInstInfo, Term, VarSet, ContextPieces,
         Result = error1([Spec])
     ;
         CompoundTypeKind = kctk_pure_pred(Args),
+        ArgContextFunc = arg_context_pieces(ContextPieces, pf_predicate),
         parse_types_no_modes(no_allow_ho_inst_info(wnhii_pred_arg),
-            VarSet, ContextPieces, Args, 1, ArgTypes, [], Specs),
+            VarSet, ArgContextFunc, Args, 1, ArgTypes, [], Specs),
         (
             Specs = [],
             construct_higher_order_pred_type(purity_pure, lambda_normal,
@@ -230,10 +243,12 @@ parse_compound_type(AllowHOInstInfo, Term, VarSet, ContextPieces,
     ;
         CompoundTypeKind = kctk_pure_func(BeforeEqTerm, AfterEqTerm),
         ( if BeforeEqTerm = term.functor(term.atom("func"), FuncArgs, _) then
+            ArgContextFunc = arg_context_pieces(ContextPieces, pf_function),
             parse_types_no_modes(no_allow_ho_inst_info(wnhii_func_arg),
-                VarSet, ContextPieces, FuncArgs, 1, ArgTypes, [], ArgSpecs),
-            RetContextPieces = ContextPieces ++
-                cord.from_list([words("in the return value:"), nl]),
+                VarSet, ArgContextFunc, FuncArgs, 1, ArgTypes, [], ArgSpecs),
+            RetContextPieces = ContextPieces ++ cord.from_list([
+                words("in the return value of higher-order function type:"),
+                nl]),
             parse_type_no_mode(no_allow_ho_inst_info(wnhii_func_return_arg),
                 VarSet, RetContextPieces, AfterEqTerm, MaybeRetType),
             ( if
@@ -284,10 +299,12 @@ parse_compound_type(AllowHOInstInfo, Term, VarSet, ContextPieces,
             Args = [BeforeEqTerm, AfterEqTerm],
             BeforeEqTerm = term.functor(term.atom("func"), FuncArgs, _)
         then
+            ArgContextFunc = arg_context_pieces(ContextPieces, pf_function),
             parse_types_no_modes(no_allow_ho_inst_info(wnhii_func_arg),
-                VarSet, ContextPieces, FuncArgs, 1, ArgTypes, [], ArgSpecs),
-            RetContextPieces = ContextPieces ++
-                cord.from_list([words("in the return value:"), nl]),
+                VarSet, ArgContextFunc, FuncArgs, 1, ArgTypes, [], ArgSpecs),
+            RetContextPieces = ContextPieces ++ cord.from_list([
+                words("in the return value of higher-order function type:"),
+                nl]),
             parse_type_no_mode(no_allow_ho_inst_info(wnhii_func_return_arg),
                 VarSet, RetContextPieces, AfterEqTerm, MaybeRetType),
             ( if
@@ -305,8 +322,9 @@ parse_compound_type(AllowHOInstInfo, Term, VarSet, ContextPieces,
             SubTerm = term.functor(term.atom(Name), Args, _),
             Name = "pred"
         then
+            ArgContextFunc = arg_context_pieces(ContextPieces, pf_predicate),
             parse_types_no_modes(no_allow_ho_inst_info(wnhii_pred_arg),
-                VarSet, ContextPieces, Args, 1, ArgTypes, [], Specs),
+                VarSet, ArgContextFunc, Args, 1, ArgTypes, [], Specs),
             (
                 Specs = [],
                 construct_higher_order_pred_type(Purity, lambda_normal,
@@ -372,11 +390,13 @@ parse_ho_type_and_inst(VarSet, ContextPieces, BeforeIsTerm, AfterIsTerm,
         BeforeIsTerm = term.functor(term.atom("="), [FuncTerm, RetTerm], _),
         FuncTerm = term.functor(term.atom("func"), ArgTerms, _)
     then
+        ArgContextFunc = arg_context_pieces(ContextPieces, pf_function),
         parse_type_and_modes(dont_constrain_inst_vars, require_tm_mode,
-            wnhii_func_arg, VarSet, ContextPieces, ArgTerms, 1,
+            wnhii_func_arg, VarSet, ArgContextFunc, ArgTerms, 1,
             ArgTypeAndModes, [], ArgTMSpecs),
-        RetContextPieces = ContextPieces ++
-            cord.from_list([words("in the return value:"), nl]),
+        RetContextPieces = ContextPieces ++ cord.from_list([
+            words("in the return value of higher-order function type:"),
+            nl]),
         parse_type_and_mode(dont_constrain_inst_vars, require_tm_mode,
             wnhii_func_return_arg, VarSet, RetContextPieces,
             RetTerm, MaybeRetTypeAndMode),
@@ -386,8 +406,9 @@ parse_ho_type_and_inst(VarSet, ContextPieces, BeforeIsTerm, AfterIsTerm,
     else if
         BeforeIsTerm = term.functor(term.atom("pred"), ArgTerms, _)
     then
+        ArgContextFunc = arg_context_pieces(ContextPieces, pf_predicate),
         parse_type_and_modes(dont_constrain_inst_vars, require_tm_mode,
-            wnhii_pred_arg, VarSet, ContextPieces, ArgTerms, 1,
+            wnhii_pred_arg, VarSet, ArgContextFunc, ArgTerms, 1,
             ArgTypeAndModes, [], ArgTMSpecs),
         parse_ho_type_and_inst_2(VarSet, ContextPieces, Purity,
             ArgTypeAndModes, ArgTMSpecs, no, MaybeDetism, MaybeType)
@@ -465,19 +486,16 @@ project_tm_type_and_mode(type_only(_), _, _) :-
 %---------------------------------------------------------------------------%
 
 :- pred parse_types_no_modes(allow_ho_inst_info::in, varset::in,
-    cord(format_component)::in, list(term)::in, int::in, list(mer_type)::out,
+    arg_context_func::in, list(term)::in, int::in, list(mer_type)::out,
     list(error_spec)::in, list(error_spec)::out) is det.
 
 parse_types_no_modes(_, _, _, [], _, [], !Specs).
-parse_types_no_modes(AllowHOInstInfo, Varset, ContextPieces, [Term | Terms],
+parse_types_no_modes(AllowHOInstInfo, Varset, ArgContextFunc, [Term | Terms],
         ArgNum, Types, !Specs) :-
-    parse_types_no_modes(AllowHOInstInfo, Varset, ContextPieces, Terms,
+    parse_types_no_modes(AllowHOInstInfo, Varset, ArgContextFunc, Terms,
         ArgNum + 1, TypesTail, !Specs),
-    ArgContextPieces = ContextPieces ++
-        cord.from_list([words("in the"), nth_fixed(ArgNum),
-        words("argument:"), nl]),
-    parse_type_no_mode(AllowHOInstInfo, Varset, ArgContextPieces, Term,
-        MaybeType),
+    parse_type_no_mode(AllowHOInstInfo, Varset, ArgContextFunc(ArgNum),
+        Term, MaybeType),
     (
         MaybeType = ok1(Type),
         Types = [Type | TypesTail]
@@ -507,14 +525,11 @@ parse_type_no_mode(AllowHOInstInfo, Varset, ContextPieces, Term, MaybeType) :-
 
 parse_type_and_modes(_, _, _, _, _, [], _, [], !Specs).
 parse_type_and_modes(MaybeInstConstraints, MaybeRequireMode, Why, VarSet,
-        ContextPieces, [Term | Terms], ArgNum, TypesAndModes, !Specs) :-
+        ArgContextFunc, [Term | Terms], ArgNum, TypesAndModes, !Specs) :-
     parse_type_and_modes(MaybeInstConstraints, MaybeRequireMode, Why,
-        VarSet, ContextPieces, Terms, ArgNum + 1, TypesAndModesTail, !Specs),
-    ArgContextPieces = ContextPieces ++
-        cord.from_list([words("in the"), nth_fixed(ArgNum),
-        words("argument:"), nl]),
+        VarSet, ArgContextFunc, Terms, ArgNum + 1, TypesAndModesTail, !Specs),
     parse_type_and_mode(MaybeInstConstraints, MaybeRequireMode, Why,
-        VarSet, ArgContextPieces, Term, MaybeTypeAndMode),
+        VarSet, ArgContextFunc(ArgNum), Term, MaybeTypeAndMode),
     (
         MaybeTypeAndMode = ok1(TypeAndMode),
         TypesAndModes = [TypeAndMode | TypesAndModesTail]
diff --git a/tests/invalid/combined_ho_type_inst.err_exp b/tests/invalid/combined_ho_type_inst.err_exp
index 07328dd..03f1124 100644
--- a/tests/invalid/combined_ho_type_inst.err_exp
+++ b/tests/invalid/combined_ho_type_inst.err_exp
@@ -16,22 +16,26 @@ combined_ho_type_inst.m:021: In type definition: error: the type
 combined_ho_type_inst.m:021:   `(((func (int :: in)) = (int :: out)) is semidet)'
 combined_ho_type_inst.m:021:   contains higher order inst information, but this
 combined_ho_type_inst.m:021:   is not allowed in a type constructor's argument.
-combined_ho_type_inst.m:024: In type definition: in the first argument:
+combined_ho_type_inst.m:024: In type definition: in the first argument of
+combined_ho_type_inst.m:024:   higher-order predicate type:
 combined_ho_type_inst.m:024:   error: the type
 combined_ho_type_inst.m:024:   `(pred((int :: in), (int :: out)) is det)'
 combined_ho_type_inst.m:024:   contains higher order inst information, but this
 combined_ho_type_inst.m:024:   is not allowed in a predicate's argument.
-combined_ho_type_inst.m:026: In type definition: in the first argument:
+combined_ho_type_inst.m:026: In type definition: in the first argument of
+combined_ho_type_inst.m:026:   higher-order predicate type:
 combined_ho_type_inst.m:026:   error: the type
 combined_ho_type_inst.m:026:   `(pred((int :: in), (int :: out)) is det)'
 combined_ho_type_inst.m:026:   contains higher order inst information, but this
 combined_ho_type_inst.m:026:   is not allowed in a predicate's argument.
-combined_ho_type_inst.m:028: In type definition: in the first argument:
+combined_ho_type_inst.m:028: In type definition: in the first argument of
+combined_ho_type_inst.m:028:   higher-order function type:
 combined_ho_type_inst.m:028:   error: the type
 combined_ho_type_inst.m:028:   `(((func (int :: in)) = (int :: out)) is semidet)'
 combined_ho_type_inst.m:028:   contains higher order inst information, but this
 combined_ho_type_inst.m:028:   is not allowed in a function's argument.
-combined_ho_type_inst.m:031: In type definition: in the first argument:
+combined_ho_type_inst.m:031: In type definition: in the first argument of
+combined_ho_type_inst.m:031:   higher-order function type:
 combined_ho_type_inst.m:031:   error: the type
 combined_ho_type_inst.m:031:   `(((func (int :: in)) = (int :: out)) is semidet)'
 combined_ho_type_inst.m:031:   contains higher order inst information, but this
diff --git a/tests/invalid/combined_ho_type_inst_2.err_exp b/tests/invalid/combined_ho_type_inst_2.err_exp
index 20c620d..2233bc1 100644
--- a/tests/invalid/combined_ho_type_inst_2.err_exp
+++ b/tests/invalid/combined_ho_type_inst_2.err_exp
@@ -1,16 +1,24 @@
-combined_ho_type_inst_2.m:015: In type definition: in the first argument:
+combined_ho_type_inst_2.m:015: In type definition: in the first argument of
+combined_ho_type_inst_2.m:015:   higher-order predicate type:
 combined_ho_type_inst_2.m:015:   error: missing `::mode' suffix.
-combined_ho_type_inst_2.m:018: In type definition: in the second argument:
+combined_ho_type_inst_2.m:018: In type definition: in the second argument of
+combined_ho_type_inst_2.m:018:   higher-order predicate type:
 combined_ho_type_inst_2.m:018:   error: missing `::mode' suffix.
-combined_ho_type_inst_2.m:021: In type definition: in the first argument:
+combined_ho_type_inst_2.m:021: In type definition: in the first argument of
+combined_ho_type_inst_2.m:021:   higher-order function type:
 combined_ho_type_inst_2.m:021:   error: missing `::mode' suffix.
-combined_ho_type_inst_2.m:024: In type definition: in the first argument:
+combined_ho_type_inst_2.m:024: In type definition: in the first argument of
+combined_ho_type_inst_2.m:024:   higher-order function type:
 combined_ho_type_inst_2.m:024:   error: missing `::mode' suffix.
-combined_ho_type_inst_2.m:024: In type definition: in the return value:
+combined_ho_type_inst_2.m:024: In type definition: in the return value of
+combined_ho_type_inst_2.m:024:   higher-order function type:
 combined_ho_type_inst_2.m:024:   error: missing `::mode' suffix.
-combined_ho_type_inst_2.m:027: In type definition: in the first argument:
+combined_ho_type_inst_2.m:027: In type definition: in the first argument of
+combined_ho_type_inst_2.m:027:   higher-order predicate type:
 combined_ho_type_inst_2.m:027:   error: unexpected `::mode' suffix.
-combined_ho_type_inst_2.m:030: In type definition: in the first argument:
+combined_ho_type_inst_2.m:030: In type definition: in the first argument of
+combined_ho_type_inst_2.m:030:   higher-order function type:
 combined_ho_type_inst_2.m:030:   error: unexpected `::mode' suffix.
-combined_ho_type_inst_2.m:030: In type definition: in the return value:
+combined_ho_type_inst_2.m:030: In type definition: in the return value of
+combined_ho_type_inst_2.m:030:   higher-order function type:
 combined_ho_type_inst_2.m:030:   error: unexpected `::mode' suffix.


More information about the reviews mailing list