[m-rev.] for post-commit review: better diagnostic for missing higher order insts

[Peter Wang]

On Wed, 19 Jul 2023 08:06:21 +1000 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> For review by everyone, if possible.
> 
> Questions I would like feeback on:
> 
> - Is the set of pieces of information it conveys right?
>   Is it missing any important info?
>   Does it include info it shouldn't?
> 
> - Is the structure of the error message, the general approach
>   it uses to convey the info it conveys, broadly right?
>   If not, there is no point in discussing its details.
> 
> - If the structure is right, are the details of its text right?
> 
> - Can anyone propose an alternative that is better than
>   the use of the _t and _i suffixes to distinguish the
>   type and inst names?
> 
> - The text would probably be improved if we added blank lines
>   (it would break up the "wall" of text). If so, where should
>   the blank lines go?
> 
> - Should anything not currently quoted be quoted, or vice versa?
> 
> Zoltan.

> diff --git a/tests/invalid/no_ho_inst.err_exp b/tests/invalid/no_ho_inst.err_exp
> index e69de29bb..7cba3af08 100644
> --- a/tests/invalid/no_ho_inst.err_exp
> +++ b/tests/invalid/no_ho_inst.err_exp
> @@ -0,0 +1,22 @@
> +no_ho_inst.m:044: In clause for `run_loop(in, in, out, di, uo)':
> +no_ho_inst.m:044:   in argument 1 (i.e. the predicate term) of higher-order
> +no_ho_inst.m:044:   predicate call:
> +no_ho_inst.m:044:   mode error: context requires a predicate of arity 4, and
> +no_ho_inst.m:044:   the type of AppHandler does match that expectation, but to
> +no_ho_inst.m:044:   check the correctness of the call, the compiler also needs
> +no_ho_inst.m:044:   to know the modes of the arguments and the determinism of
> +no_ho_inst.m:044:   the predicate that AppHandler represents, and AppHandler's
> +no_ho_inst.m:044:   inst does not contain that information.

My attempt:

    In clause for `run_loop(in, in, out, di, uo)':
      in argument 1 (i.e. the predicate term) of higher-order
      predicate call:
      mode error: the context requires a predicate of arity 4.
      The type of AppHandler matches that expectation, but the inst of
      AppHandler at this point is `ground', which lacks the higher-order
      inst information required to make the call.

in which I think the improvements are:

  - removes an interpretation where the reader might think the mode
    error is due to a compiler limitation

  - keeps the part which says that it's (probably) not a type error.
    That seems like a good idea.

  - prints out the problematic inst

  - tells the user that "higher-order inst information" is required to
    make a higher-order call, using those exact words so they can be
    searched for in the reference manual

  - removes the line about the "usual fix" which is vague
    (although that might have been intended as a heading to the next part)

> +no_ho_inst.m:044:   The usual fix for this error is to add this information.
> +no_ho_inst.m:044:   Given a higher order type such as
> +no_ho_inst.m:044:     :- type callback_t == (pred(world, world, io, io).
> +no_ho_inst.m:044:   you would define a corresponding inst, such as
> +no_ho_inst.m:044:     :- inst callback_i == (pred(in, out, di, uo) is det).
> +no_ho_inst.m:044:   This inst specifies the modes of the arguments and the
> +no_ho_inst.m:044:   determinism of a predicate. You can then tell the compiler
> +no_ho_inst.m:044:   that a value of type callback_t has inst callback_i by
> +no_ho_inst.m:044:   specifying either the mode `in(callback_i)' (when taking a
> +no_ho_inst.m:044:   value of type callback_t as input) or the mode
> +no_ho_inst.m:044:   `out(callback_i)' (when returning a value of type
> +no_ho_inst.m:044:   callback_t as output).
> +For more information, recompile with `-E'.

I personally prefer not to see things like this in error messages.
Once you learn what's going on, it's tiring to be told the same long
piece of (non-specific) advice over and over.

A common situation where the error would arise is when someone tries to
take a higher-order term out of a container and tries to call it, then
the advice would not apply.

You could hide the advice behind the -E option.

Peter