[m-rev.] for review: Improve coerce mode error messages.

[Julien Fischer]

Hi Peter,

On Wed, 12 May 2021, Peter Wang wrote:

> compiler/mode_errors.m:
>    Replace the two existing options for coerce mode errors with one
>    that includes:
>    - the path to the input subterm where the error was detected
>    - the type of the input subterm
>    - the target type that the subterm should be converted to
>    - the reason for the error
>
> compiler/modecheck_coerce.m:
>    Modify the code such to produce the information above.

     Modify the code to produce the above information.


>    Semidet predicates that previously failed when an error is detected
>    are changed to det predicates that return the error information.
>
>    In a couple of places where predicates previously would fail,
>    we now abort because the conditions should not occur.

...


> diff --git a/tests/invalid/coerce_clobbered.err_exp b/tests/invalid/coerce_clobbered.err_exp
> index 2bc874620..fd7140fa4 100644
> --- a/tests/invalid/coerce_clobbered.err_exp
> +++ b/tests/invalid/coerce_clobbered.err_exp
> @@ -1,4 +1,4 @@
> coerce_clobbered.m:021: In clause for `bad(in(clobbered), out)':
> coerce_clobbered.m:021:   in coerce:

I think we should replace that with either:

     in coerce expression:

or:

     in type conversion expression:

> -coerce_clobbered.m:021:   mode error: `X' has instantiatedness `clobbered',
> -coerce_clobbered.m:021:   but it must be ground.
> +coerce_clobbered.m:021:   mode error: the input term has instantiatedness
> +coerce_clobbered.m:021:   `clobbered', but it must be ground.

I would be inclined to use "argument" rather than "input term"  (Or
perhaps, "argument term".)
(If it does change the terminology in the reference manual should be
updated too.)

Also, ground should be quoted there.

> diff --git a/tests/invalid/coerce_instvar.err_exp b/tests/invalid/coerce_instvar.err_exp
> index 292f9a29d..0cb0e84a7 100644
> --- a/tests/invalid/coerce_instvar.err_exp
> +++ b/tests/invalid/coerce_instvar.err_exp
> @@ -1,7 +1,7 @@
> coerce_instvar.m:043: In clause for `bad(in((I =< ground)), out((I =<
> coerce_instvar.m:043:   ground)))':
> coerce_instvar.m:043:   in coerce:
> -coerce_instvar.m:043:   mode error: the input term has instantiatedness
> -coerce_instvar.m:043:   `( I =< ground )',
> -coerce_instvar.m:043:   and cannot be converted to the type
> -coerce_instvar.m:043:   `coerce_instvar.citrus'.
> +coerce_instvar.m:043:   mode error: cannot convert the input term from type
> +coerce_instvar.m:043:   `coerce_instvar.fruit' to `coerce_instvar.citrus'
> +coerce_instvar.m:043:   because it has instantiatedness `ground', and the
> +coerce_instvar.m:043:   former type is not a subtype of the latter type.

I suggest using the type names here rather than "former type" and "latter type".
(Yes, it's a bit repetitious, but it reads less awkwardly.)

> diff --git a/tests/invalid/coerce_int.err_exp b/tests/invalid/coerce_int.err_exp
> index 197c8bc8d..a93597419 100644
> --- a/tests/invalid/coerce_int.err_exp
> +++ b/tests/invalid/coerce_int.err_exp
> @@ -24,23 +24,7 @@ coerce_int.m:026:       ).
> coerce_int.m:032: In clause for `bad_wrong_type(in(coerce_int.wrap(bound(1 ;
> coerce_int.m:032:   2)))) = out(coerce_int.wrap(bound(1 ; 2)))':
> coerce_int.m:032:   in coerce:
> -coerce_int.m:032:   mode error: the input term has instantiatedness
> -coerce_int.m:032:     named inst wrap(
> -coerce_int.m:032:       bound(
> -coerce_int.m:032:         1
> -coerce_int.m:032:       ;
> -coerce_int.m:032:         2
> -coerce_int.m:032:       )
> -coerce_int.m:032:     ),
> -coerce_int.m:032:     which expands to
> -coerce_int.m:032:       bound(
> -coerce_int.m:032:         wrap(
> -coerce_int.m:032:           bound(
> -coerce_int.m:032:             1
> -coerce_int.m:032:           ;
> -coerce_int.m:032:             2
> -coerce_int.m:032:           )
> -coerce_int.m:032:         )
> -coerce_int.m:032:       ),
> -coerce_int.m:032:   and cannot be converted to the type
> -coerce_int.m:032:   `coerce_int.wrap(uint)'.
> +coerce_int.m:032:   mode error: in the input term:
> +coerce_int.m:032:   in the argument of function symbol `wrap':
> +coerce_int.m:032:   the subterm has instantiatedness `bound(1 ; 2)', which is
> +coerce_int.m:032:   invalid for the type `uint'.

The problem there is that the new message does not make it clear
that 'wrap' is part of the input term's inst.  Perhaps:


     mode error: in the instantiatedness of the input term:
         ...

Mind, as noted in the code, this is one of those things that
should really be checked for elsewhere and earlier.  For one, thing
the actual here is either on line 28 or 29 (depending on which part
of the mismatch is wrong.)

> diff --git a/tests/invalid/coerce_mode_error.err_exp b/tests/invalid/coerce_mode_error.err_exp
> index 0899016bb..46f750ea0 100644
> --- a/tests/invalid/coerce_mode_error.err_exp
> +++ b/tests/invalid/coerce_mode_error.err_exp
> @@ -1,28 +1,37 @@
> coerce_mode_error.m:035: In clause for `bad_coerce_free_input(in(free), out)':
> coerce_mode_error.m:035:   in coerce:
> -coerce_mode_error.m:035:   mode error: `X' has instantiatedness `free',
> -coerce_mode_error.m:035:   but it must be ground.
> +coerce_mode_error.m:035:   mode error: the input term has instantiatedness
> +coerce_mode_error.m:035:   `free', but it must be ground.

Quote ground there.

> coerce_mode_error.m:064: In clause for `bad_from_list_to_least2(in, out)':
> coerce_mode_error.m:064:   in coerce:
> -coerce_mode_error.m:064:   mode error: the input term has instantiatedness
> -coerce_mode_error.m:064:   `bound('[|]'(ground, ground))',
> -coerce_mode_error.m:064:   and cannot be converted to the type
> -coerce_mode_error.m:064:   `coerce_mode_error.least2(V_1)'.
> +coerce_mode_error.m:064:   mode error: in the input term:
> +coerce_mode_error.m:064:   in argument 2 of function symbol `'[|]'':

     in the second argument of the function symbol

Everywhere else you use:

(Use nth_fixed from error_util.)

And anywhere else you use the "argument N" form.

Apart from my quibbles over the wording of the error messages,
the diff is fine.

Julien.