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

[Julien Fischer]

Hi Zoltan,

On Wed, 19 Jul 2023, Zoltan Somogyi wrote:

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

My issue here is "the context" part.  How about something like:

    mode error: AppHandler must be a predicate of arity 4: the type
      of AppHandler does match that expectation, but in order ...

?

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

Can we distinguish between a higher-order term that was an argument
of its containing predicate vs. one that was unpacked from a container
at this point in the mode analyser?  In the former case, which is
the common one, why would fixing the inst in the mode declaration
not be the only recommended course of action?

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

I can live with that. It seems a reasonable trade-off for the hardship
of having to repeatedly explain it on the users list ;-)

Julien.