[m-rev.] for review: structured higher order types in type error messages

[Peter Wang]

On Thu, 24 Sep 2020 20:27:00 +1000 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> diff --git a/compiler/typecheck_errors.m b/compiler/typecheck_errors.m
> index fc2cb2c65..8b063aa6c 100644
> --- a/compiler/typecheck_errors.m
> +++ b/compiler/typecheck_errors.m

> @@ -2529,27 +2554,150 @@ make_list_term([Var | Vars]) =
>      --->    do_not_add_quotes
>      ;       add_quotes.
>  
> -:- func type_to_pieces(maybe_add_quotes, mer_type, tvarset,
> -    external_type_params) = list(format_component).
> +:- func type_to_pieces(maybe_add_quotes, tvarset, inst_varset,
> +    external_type_params, mer_type) = list(format_component).
>  
> -type_to_pieces(MaybeAddQuotes, Type0, TVarSet, ExternalTypeParams) = Pieces :-
> +type_to_pieces(MaybeAddQuotes, TVarSet, InstVarSet, ExternalTypeParams,
> +        Type0) = Pieces :-
>      strip_builtin_qualifiers_from_type(Type0, Type),
> -    unparse_type(Type, Term0),
> -    list.map(term.coerce_var, ExternalTypeParams, ExistQVars),
> -    maybe_add_existential_quantifier(ExistQVars, Term0, Term),
> -    varset.coerce(TVarSet, VarSet),
> -    TermPiece = words(mercury_term_to_string(VarSet, print_name_only, Term)),
>      (
>          MaybeAddQuotes = do_not_add_quotes,
> -        Pieces = [TermPiece]
> +        StartQuotePieces = [],
> +        EndQuotePieces = []
>      ;
>          MaybeAddQuotes = add_quotes,
> -        Pieces = [prefix("`"), TermPiece, suffix("'")]
> +        StartQuotePieces = [error_util.prefix("`")],
> +        EndQuotePieces = [error_util.suffix("'")]
> +    ),
> +    % For most types, we convert the type back to a term, and print that.
> +    % This is done by the final else part of this if-then-else.
> +    % This is ok for most types, which tend to be small.
> +    ( if
> +        % We handle higher order types differently, because when the
> +        % actual and expected types differ in e.g. arity, having each
> +        % argument type on its own line helps readability a *lot*.
> +        Type = higher_order_type(PorF,  ArgTypes, HOInstInfo, Purity,
> +            _LambdaEvalMethod)

Double space there.

> +    then
> +        ( Purity = purity_pure,     PurityPieces = []
> +        ; Purity = purity_semipure, PurityPieces = [words("semipure")]
> +        ; Purity = purity_impure,   PurityPieces = [words("impure")]
> +        ),
> +        ( PorF = pf_predicate,      PorFPieces = [words("pred")]
> +        ; PorF = pf_function,       PorFPieces = [words("func")]
> +        ),
> +        (
> +            HOInstInfo = none_or_default_func,
> +            ArgPieces = list.map(
> +                type_to_pieces(do_not_add_quotes, TVarSet, InstVarSet,
> +                    ExternalTypeParams),
> +                ArgTypes),
> +            FuncResultPrefixPieces = [],
> +            FuncResultSuffixPieces = [],
> +            DetismPieces = [],
> +            PorFMismatchPieces = [],
> +            ArityMismatchPieces = []
> +        ;
> +            HOInstInfo = higher_order(PredInstInfo),
> +            PorFStr = pred_or_func_to_full_str(PorF),
> +            PredInstInfo = pred_inst_info(HOPorF, ArgModes, _ArgRegs, Detism),
> +            ( if PorF = HOPorF then
> +                PorFMismatchPieces = []
> +            else
> +                HOPorFStr = pred_or_func_to_full_str(HOPorF),
> +                PorFMismatchPieces = [nl,
> +                    words("The type says this is a"),
> +                    words(PorFStr), suffix(","),
> +                    words("but its mode says it is a"),
> +                    words(HOPorFStr), suffix(".")]
> +            ),
> +            list.length(ArgTypes, NumArgTypes),
> +            list.length(ArgModes, NumArgModes),
> +            ( if NumArgTypes = NumArgModes then
> +                assoc_list.from_corresponding_lists(ArgTypes, ArgModes,
> +                    ArgTypesModes),
> +                % If this higher order type is a function, then the type::mode
> +                % for the function result must be wrapped in parentheses.
> +                FuncResultPrefixPieces = [error_util.prefix("(")],
> +                FuncResultSuffixPieces = [error_util.suffix("(")],

Fix the close parenthesis.

> @@ -2721,6 +2869,21 @@ arg_type_assign_set_msg_to_verbose_pieces(Info, ArgTypeAssignSet, VarSet,
>          VerboseComponents = [verbose_only(verbose_always, VerbosePieces)]
>      ).
>  
> +%-----------------------------------------------------------------------------%
> +
> +:- pred get_inst_varset(type_error_clause_context::in, inst_varset::out)
> +    is det.
> +
> +get_inst_varset(ClauseContext, InstVarSet) :-
> +    % XXX Typechecking works on pred_infos, which do NOT have an inst_varset.
> +    % I (zs) don't know where the inst variables in any ho_inst_infos
> +    % in higher-order types come from, but I am *guessing* that it is
> +    % from the varset of the clause itself. I am not even sure whether
> +    % this matters, since I don't know whether ho_inst_infos can ever
> +    % be filled in before the end of typechecking.
> +    ProgVarSet = ClauseContext ^ tecc_varset,
> +    varset.coerce(ProgVarSet, InstVarSet).

I don't know; would need to investigate.

The rest looks fine.

Peter