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

[Zoltan Somogyi]

On 2023-07-19 08:38 +02:00 CEST, "Julien Fischer" <jfischer at opturion.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.
> 
> Rather than the "the context", could we instead say "this argument"?

No, we can't. "This argument" does require a predicate, but only
the rest of the context requires it to have arity 4.

>>  - 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)
> 
> It does rather imply there's some other, more unusual, fix.

There is. One could take the higher order type, ground inst var,
and pass it to a foreign_proc that returns it unchanged,
with the same type but now a higher order inst. This would do
an "inst cast". I think you know why I did not want to recommend that,
even though it is probably the convenient option in the case of
higher order values taken out of data structures.

>>> +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
> 
> I would prefix the example with:
> 
>      For example, given a higher-order type ...
> 
> just to make it absolutely clear that what's being described is an
> example and does not refer to the user's code.

Agreed.

>> 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.
> 
> I agree with Peter on that last point.

I agree that we can hide that advice behind -E, I disagree on whether
we should. If even people who have been programming in Mercury
for years are confused by this, then I think we should "take one for the team",
and suffer the extraordinary hardship of occasionally having a dozen lines we don't
need to read shoved in front of our eyes, in order to make things easier
for our users.

Zoltan.