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

[Zoltan Somogyi]

On 2023-07-19 04:56 +02:00 CEST, "Peter Wang" <novalazy at gmail.com> wrote:
>> +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.

I like most aspects of that, except one.

> in which I think the improvements are:
> 
>   - removes an interpretation where the reader might think the mode
>     error is due to a compiler limitation

Actually, it IS due to a compiler limitation. The limitation is that
even if you include the higher order inst info in the type, the
compiler won't pay attention to it. But I agree that there is no point
in emphasizing that fact.

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

Agree with both.

>   - 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

I agree that giving that search terms is useful. But your text also *requires*
a search term for novices, because it does not say itself what "higher order
inst information" *is*: the modes of the args, and the determinism.
I would strongly prefer to provide that info in this error message.

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

It was intended to be such a heading. It seems a blank line
would have helped here.

>> +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.

Yes, we *could* make everything from the "usual fix" reference onward
a verbose-only part of the message. The reason I did not do so is that
the very people who need the info in that part are the people who may not
pay attention to the "recompile with -E" at the bottom of the output,
partly because it may not follow the error message to which it applies
*immediately*.

> 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.

The four possible sources of a higher order term are:

- an input arg in the clause head
- an output arg in a call in the clause body
- a lambda expression (possibly implicit) in a construct unification
  the clause body
- a deconstruct unification in the clause body.

The third is not a problem, since in that case, the higher order inst
is present by construction. The "usual fix" message covers the first two.
The fourth is, in my experience, much rarer than the other three,
precisely because there *is* no good way to preserve the higher order
inst info of whatever closure was put into the data structure. There is
a way, consisting of always specifying an inst for the whole data structure
that includes the higher order inst of the higher order value, but
I wouldn't call it "good". In that case, the good solution would be
to include the higher order value's inst in its type, but this needs the compiler
to be able to use that info. I left out describing that scenario because anyone
who needs help with this issue would probably be more confused than helped
by a descrition of the above :-(

Zoltan.