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

[Zoltan Somogyi]

On 2023-07-19 14:37 +02:00 CEST, "Julien Fischer" <jfischer at opturion.com> 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 ...
> 
> ?

This text raises the question: *why* must AppHandler be a predicate
of arity 4? The "context" is intended to answer that question before
it is asked.

What do you think is wrong with saying "the context requires"?
I won't mind alternative wording, but I do think that wording should
answer the question above.

In general, an error message points out a mismatch between
the compiler's expectation of what it should find somewhere,
and what it actually finds there. The error is usually with what
it finds, but occasionally, what it finds there is actually correct,
and the error is with the part of the code that *established*
the compiler's expectation. This is why I feel strongly that
whenever possible, the compiler should tell programmers
*why* it expects things that it complains about not finding.

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

I don't understand. Distinguish for what purpose? What do you propose
the compiler should do differently in these two cases?

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

The reason why this is not a very good course of action is that
you would have to fix the inst (i.e. include the higher order inst info
in the inst) not just in the declaration of the predicates/functions
that unpack and use the higher order value, but also *everywhere else*
where you pass that data structure, at least for update. If you ever
pass that data structure anywhere while its inst includes the higher order
inst info, but get it back with a simple "out" mode, the higher order
inst info can never be recovered from that point on.

Because of this, this approach can easily require more complex argument
insts (i.e. insts that include the higher order inst info) in tens or even hundreds
more declarations. It would work, but it is tedious.

Note that including the higher order inst info in the *type* would not
have this problem, but of course this requires the mode checker to *pay attention*
to that info. At present, it does not do so.

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

My thought exactly.

Zoltan.