[m-rev.] for post-commit review: better error messages for arg type errors

[Paul Bone]

On Sat, Dec 27, 2014 at 01:18:00PM +1100, Zoltan Somogyi wrote:
> I was working on module_sub_infos (which has about 30 fields)
> when I got a huge error message. I figured it would almost be simpler
> to make this change than to try to manually dig through it.
> 

This is a useful change.  Thanks.

> Improve messages for errors in functor argument types.
> 
> compiler/typecheck_errors.m:
>     Given a type error in X = f(Y1, ..., Yn), where the type of one or more
>     of the Yi does not match the type of the corresponding argument of f,
>     we have for a long time tried to print error messages that mention
>     the expected/actual type mismatches only for those arguments.
>     However, we used to insist on both the expected and actual types
>     being uniquely known. This diff lifts that restriction, so we can
>     now generate these more focused messages if e.g. the Yi is "no",
>     whose type may be either bool or maybe(T).
> 
>     We now also try to suppress any mention of mismatches in which the actual
>     type subsumes the expected type. This usually happens when the actual
>     argument value is something like [], whose type is list(T), but the
>     expected type is a list with a known element type. If there are any
>     argument type mismatches for which this is not true, we print error
>     messages only about them.
> 
>     Don't insist on printing the entire right hand side term on a single
>     like as a fixed quoted string. For the kinds of unifications for which
>     this improvement is most useful, that term won't fit on one line.
> 
>     Be more consistent aboiut putting quotes around types in error messages.

about

(I know you can't change this now.)

>     Move a predicate next to where it is used.
> 
> compiler/prog_type.m:
>     Provide a missing utility function for the new code in typecheck_errors.m.
>
> tests/invalid/overloading.err_exp:
>     Expect the error messages we now generate for this test case.
>     This is only 10 lines, compared to the old 44 lines. The first 6
>     of those 10 set the context, and the last 4 identify the problem
>     exactly. (The motivation for this change was an error like this.)
> 
> tests/invalid/type_error_ambiguous.err_exp:
>     Expect quotes around types.


> diff --git a/compiler/typecheck_errors.m b/compiler/typecheck_errors.m
> index 5a4fd0a..fa1b4f4 100644
> --- a/compiler/typecheck_errors.m
> +++ b/compiler/typecheck_errors.m
> @@ -723,35 +723,47 @@ report_error_functor_arg_types(Info, Var, ConsDefnList, Functor, Args,
>      StrippedFunctorStr = functor_cons_id_to_string(StrippedFunctor, Args,
>          VarSet, ModuleInfo, no),
>  
> -    Pieces1 = [words("in unification of")] ++
> +    VarAndTermPieces = [words("in unification of")] ++
>          argument_name_to_pieces(VarSet, Var) ++ [nl, words("and term"),
> -        quote(StrippedFunctorStr), suffix(":"), nl,
> +        words_quote(StrippedFunctorStr), suffix(":"), nl,
>          words("type error in argument(s) of")] ++
>          functor_name_to_pieces(StrippedFunctor, Arity) ++ [suffix("."), nl],
>  
>      ConsArgTypesSet = list.map(get_callee_arg_types, ArgsTypeAssignSet),
>  
> -    % If we know the type of the function symbol, and each argument
> -    % also has at most one possible type, then we prefer to print an
> -    % error message that mentions the actual and expected types of the
> -    % arguments only for the arguments in which the two types differ.
> +    % If we have consistent information about the argument types,
> +    % we prefer to print an error message that mentions only the arguments
> +    % that may be in error.
>      (
>          list.all_same(ConsArgTypesSet),
> -        ConsArgTypesSet = [ConsArgTypes | _],
> +        ConsArgTypesSet = [ConsArgTypes | _]
> +    ->
>          assoc_list.from_corresponding_lists(Args, ConsArgTypes, ArgExpTypes),
>          TypeAssigns = list.map(get_caller_arg_assign, ArgsTypeAssignSet),
> -        find_mismatched_args(ArgExpTypes, TypeAssigns, 1,
> -            SimpleMismatches, ComplexMismatches, AllMismatches),
> -        expect(list.is_not_empty(AllMismatches), $module, $pred,
> -            "no mismatches"),
> -        ComplexMismatches = []
> -    ->
> -        Pieces2 =
> -            mismatched_args_to_pieces(SimpleMismatches, yes, VarSet, Functor),
> +        find_mismatched_args(1, ArgExpTypes, TypeAssigns,
> +            [], RevSubsumesMismatches, [], RevNoSubsumeMismatches),
> +        % RevSubsumesMismatches will contain errors where the actual type
> +        % is e.g. list(T), while the expected type is list(some_actual_type).
> +        % Since the argument may be just list(T) because it is [],
> +        % we don't mention these arguments (which are likely to be red
> +        % herrings, i.e. not the actual cause of the problem) unless
> +        % there are no arguments whose possible the actual types do not
> +        % include one that subsumes the expected type.

"whose possible the actual"  I don't know what you mean.

> @@ -1387,6 +1480,16 @@ ambiguity_error_possibilities_to_pieces([Var | Vars], VarSet,
>          TypeAssign1, TypeAssign2),
>      Pieces = HeadPieces ++ TailPieces.
>  
> +    % Check whether two types are identical, i.e. whether they can be unified
> +    % without binding any type parameters.
> +    %
> +:- pred identical_types(mer_type::in, mer_type::in) is semidet.
> +
> +identical_types(Type1, Type2) :-
> +    map.init(TypeSubst0),
> +    type_unify(Type1, Type2, [], TypeSubst0, TypeSubst),
> +    TypeSubst = TypeSubst0.
> +
>  %-----------------------------------------------------------------------------%

Should this be in the other file with subsumes_type?

>  
> -    % Check whether two types are identical, i.e. whether they can be unified
> -    % without binding any type parameters.
> -    %
> -:- pred identical_types(mer_type::in, mer_type::in) is semidet.
> -
> -identical_types(Type1, Type2) :-
> -    map.init(TypeSubst0),
> -    type_unify(Type1, Type2, [], TypeSubst0, TypeSubst),
> -    TypeSubst = TypeSubst0.
> -

Ah, it has moved.



-- 
Paul Bone