[m-rev.] for review: diffs for arg type lists

[Julien Fischer]

Hi Zoltan,

On Fri, 4 Jan 2019, Zoltan Somogyi wrote:

>
>
> On Thu, 3 Jan 2019 08:38:35 +0000 (UTC), Julien Fischer <jfischer at opturion.com> wrote:
>> I've just had a play around with this and there is another issue:
>>
>>      :- module foo.
>>      :- interface.
>>
>>      :- pred q1(int::out) is det.
>>
>>      :- implementation.
>>
>>      q1(3, "abc").
>>
>> produces:
>>
>>     foo.m:004: Error: no clauses for predicate `q1'/1.
>>     foo.m:008: Error: clause for predicate `foo.q1'/2
>>     foo.m:008:   without corresponding `:- pred' declaration.
>>     foo.m:008:   However, a predicate of that name does exist with arity 1.
>>     foo.m:008: Inferred :- pred q1(int, string).
>>     foo.m:008:   The argument list difference from the arity 1 version is
>>     foo.m:008:     pred(
>>     foo.m:008:   -     int
>>     foo.m:008:   +     int,
>>     foo.m:008:   +     string
>>     foo.m:008:     )
>>
>> Here the declaration and inferred types agree on the type of the first
>> argument, so there should be no change there.
>
> There is no change in the type; there is a change in the addition of a comma,
> so the standard Unix diff program would produce this output as well.

Ah, true.

> We can either put commas between the argument types as usual and live
> with such spurious differences (which occur all the time in diffs, e.g. when
> one adds code to the end of a clause or a switch arm), or we can just list
> arg types without commas between them in this message, though this
> would look unusual in another way.
>
> A third solution would be to put a comma after the last arg type as well,
> which would be strange in a totally different way again.
>
> What do people prefer?

Leave it as-is.

...

> I am more concerned by the difference in how the old and new parts of that
> error message refer to type variables. One example:
>
> +bad_pred_arity.m:056: Inferred :- func f2(int, T2, int) = int.
> +bad_pred_arity.m:056:   The argument list difference from the arity 0 version
> +bad_pred_arity.m:056:   is
> +bad_pred_arity.m:056:   - func = string
> +bad_pred_arity.m:056:   + func(
> +bad_pred_arity.m:056:   +     int,
> +bad_pred_arity.m:056:   +     V_1,
> +bad_pred_arity.m:056:   +     int
> +bad_pred_arity.m:056:   + ) = int
>
> The old inference message uses the tvarset of the predicate, which
> is created during the inference process, which somehow names tvar 1
> as T2 (I have no idea how, or why). The new part uses an empty tvarset,
> because using the tvarset of *either* predicate being compared would be
> confusing with respect to the other. This is why the type var is printed as V_1.
>
> Should we change the inference message to use an empty tvarset as well,
> use the tvarset of the inferred predicate or of the actually declared predicate
> we are comparing to for both parts of the message, or just keep the output
> above?

Change the inference message to use an empty tvarset as well.

Julien.