[m-rev.] for review: better error message for bad type names

[Mark Brown]

Hi,

On Mon, Apr 18, 2016 at 1:59 PM, Zoltan Somogyi
<zoltan.somogyi at runbox.com> wrote:
> For review by anyone, but I would like Mark to have a look
> at the XXXs on the definition of the why_no_ho_inst_info type
> in parse_type_name.m, which lists the contexts in which types
> may not contain higher order inst information.

They should all be possible with varying degrees of additional
programming. I left them NYI because I needed all my brain capacity
for the core problem.

> Generate better error messages for bad type names.
>
> compiler/parse_type_name.m:
>     When trying to parse a type name and failing, we used to just print
>     the whole term (including the correct parts) and a message of the form
>     "ill-formed type". We know print a message for each incorrect *part*

s/know/now/

>     of a type term, and make its message specific to the particular problem.
>
>     To help do this, base the new code on the type-name-parsing code
>     that was already in this module (which could generate no error messages
>     at all), but on better, though still not perfect, code that used to be
>     in parse_item.m.

Do you mean base the new code *not* on the type-name-parsing code?

> diff --git a/compiler/parse_class.m b/compiler/parse_class.m
> index 371baad..d606a82 100644
> --- a/compiler/parse_class.m
> +++ b/compiler/parse_class.m
> @@ -578,9 +578,9 @@ parse_arbitrary_constraint(VarSet, ConstraintTerm, Result) :-
>          try_parse_sym_name_and_args(ConstraintTerm, ClassName, Args0)
>      then
>          ArgsResultContextPieces =
> -            cord.singleton(words("In class or inst constraint:")),
> -        parse_types(no_allow_ho_inst_info, VarSet, ArgsResultContextPieces,
> -            Args0, ArgsResult),
> +            cord.singleton(words("In class constraint:")),
> +        parse_types(no_allow_ho_inst_info(wnhii_class_constraint),
> +            VarSet, ArgsResultContextPieces, Args0, ArgsResult),
>          (
>              ArgsResult = ok1(Args),
>              Constraint = constraint(ClassName, Args),

The change to the message makes sense given what is on this code path,
but note that it can still be reached for an inst constraint.

It's not required as part of this change, but parse_inst_constraint/3
ought to succeed whenever the functor is '=<'/2, producing error specs
if the arguments cannot be parsed. That would prevent the above code
path being taken.

> diff --git a/tests/invalid/combined_ho_type_inst.err_exp b/tests/invalid/combined_ho_type_inst.err_exp
> index 3a89720..8d35d43 100644
> --- a/tests/invalid/combined_ho_type_inst.err_exp
> +++ b/tests/invalid/combined_ho_type_inst.err_exp
> +combined_ho_type_inst.m:035: Error: invalid determinism category `semiet'.

That's actually a typo in the test case, which I hadn't noticed with
the previous error messages.

The rest looks fine.

Cheers,
Mark