[m-rev.] for post-commit review: add color in mode_errors.m

Zoltan Somogyi zoltan.somogyi at runbox.com
Sat May 4 01:03:02 AEST 2024


On 2024-05-04 00:49 +10:00 AEST, "Julien Fischer" <jfischer at opturion.com> wrote:
>>>> diff --git a/tests/invalid/no_ho_inst.err_exp b/tests/invalid/no_ho_inst.err_exp
>>>> index cc44364a4..b615b813e 100644
>>>> --- a/tests/invalid/no_ho_inst.err_exp
>>>> +++ b/tests/invalid/no_ho_inst.err_exp
>>>> @@ -1,14 +1,14 @@
>>>>  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. The
>>>> -no_ho_inst.m:044:   type of AppHandler does match that expectation. However, 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. The insts of
>>>> -no_ho_inst.m:044:   higher order values should contain this information, but
>>>> -no_ho_inst.m:044:   AppHandler's inst, which is `ground' at this point, does
>>>> -no_ho_inst.m:044:   not.
>>>> +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.
>>>> +no_ho_inst.m:044:   However, to check the correctness of the call, the compiler
>>>> +no_ho_inst.m:044:   also needs to know the modes of the arguments and the
>>>> +no_ho_inst.m:044:   determinism of the predicate that AppHandler represents.
>>>> +no_ho_inst.m:044:   The insts of higher order values should contain this
>>>> +no_ho_inst.m:044:   information, but AppHandler's inst, which is `ground' at
>>>> +no_ho_inst.m:044:   this point, does not.
>>>>  no_ho_inst.m:044:
>>>>  no_ho_inst.m:044:   The fix for this error is to add this information. For
>>>>  no_ho_inst.m:044:   example, given a higher order type such as
>>>
>>> Unrelated: presumably AppHandler should quoted in that one since it's a program variable.
>>
>> As a variable, it must start with an upper case letter. I think that is sufficient
>> to make it clear that it is variable, especially when I color its first occurrence
>> with cyan. Do you disagree?
> 
> We quote {program,type,inst} variables in *many* other error message:
> why do we not do it here?

Which version of that error message do you think is more readable,
the version above, or a version with quotes around every occurrence
of AppHandler?

I happen to think that

- the version without quote is more readable, and
- this consideration is more important than following the
  usual pattern.

Which of those propositions do you disagree with?

Zoltan.


More information about the reviews mailing list