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

[Zoltan Somogyi]

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.

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?

> Another possible issue: in these error message existentially quantified type
> variables are displayed in the same way as universially quantified ones.

True, but existentially typed args are so rare, I don't think that is a significant
issue.

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?

Zoltan.