[Caml-list] Record field disambiguation in 4.01

[Caml-list] Record field disambiguation in 4.01

[Yaron Minsky]

This is all in the vein of complaining about a feature that has not
yet been released, so, apologies if all of this is already known and
plans are there for fixing it.

I've been playing around with a recent 4.01 snapshot of the compiler
in OPAM, and have run against an interesting issue with the record
field disambiguation.  In particular, I have some code that looks
roughly like this:

open Core.Std

type posn = { x: float; y: float }

let cross_product p1 p2 =
  p1.x *. p2.y -. p1.y *. p2.x
;;

And when I try to compile this, I get the following error:

File "geometry.ml", line 68, characters 13-14:
Warning 41: this use of y is ambiguous.
File "geometry.ml", line 1:
Error: Error-enabled warnings (1 occurrences)
Command exited with code 2.

This can be fixed by adding an annotation:

let cross_product p1 (p2:posn) =
  p1.x *. p2.y -. p1.y *. p2.x
;;

A few concerns: one, it's really hard to figure out where the source
of the conflict is from this message.  It would be nice to get some
clue from the compiler as to the two definitions that are conflicting.

Also, I'm wondering if anyone has thoughts as to how much code this
change will break?  The answer might be "quite a lot", and it might
nonetheless be worth it.  But I'm curious what people's thoughts are.

y

Re: [Caml-list] Record field disambiguation in 4.01

[Jacques Garrigue]

On 2013/03/10, at 8:39, Yaron Minsky <yminsky@gmail.com> wrote:

> This is all in the vein of complaining about a feature that has not
> yet been released, so, apologies if all of this is already known and
> plans are there for fixing it.
> 
> I've been playing around with a recent 4.01 snapshot of the compiler
> in OPAM, and have run against an interesting issue with the record
> field disambiguation.  In particular, I have some code that looks
> roughly like this:
> 
> open Core.Std
> 
> type posn = { x: float; y: float }
> 
> let cross_product p1 p2 =
>  p1.x *. p2.y -. p1.y *. p2.x
> ;;
> 
> And when I try to compile this, I get the following error:
> 
> File "geometry.ml", line 68, characters 13-14:
> Warning 41: this use of y is ambiguous.
> File "geometry.ml", line 1:
> Error: Error-enabled warnings (1 occurrences)
> Command exited with code 2.
> 
> This can be fixed by adding an annotation:
> 
> let cross_product p1 (p2:posn) =
>  p1.x *. p2.y -. p1.y *. p2.x
> ;;
> 
> A few concerns: one, it's really hard to figure out where the source
> of the conflict is from this message.  It would be nice to get some
> clue from the compiler as to the two definitions that are conflicting.

I see. This shouldn't be difficult.

> Also, I'm wondering if anyone has thoughts as to how much code this
> change will break?  The answer might be "quite a lot", and it might
> nonetheless be worth it.  But I'm curious what people's thoughts are.

Just to clarify: warning 41 is off by default.
You should just decide whether you want to enable it for core or not.
(The practice of using -w A -warn-error A is dangerous, as new warnings
may be added at any time)

	Jacques

Re: [Caml-list] Record field disambiguation in 4.01

[Maxence Guesdon]

On Sun, 10 Mar 2013 10:24:16 +0900
Jacques Garrigue <garrigue@math.nagoya-u.ac.jp> wrote:
>
> ...
> Just to clarify: warning 41 is off by default.
> You should just decide whether you want to enable it for core or not.
> (The practice of using -w A -warn-error A is dangerous, as new warnings
> may be added at any time)

Maybe additional aliases with a naming convention could be useful, e.g.
 -w 4.01-A
could mean "all flags as defined in 4.01 release" ?

-- 
Maxence

Re: [Caml-list] Record field disambiguation in 4.01

[Alain Frisch]

On 03/11/2013 12:38 PM, Maxence Guesdon wrote:
> Maybe additional aliases with a naming convention could be useful, e.g.
>   -w 4.01-A
> could mean "all flags as defined in 4.01 release" ?

Warning numbers are allocated incrementally, so this can be emulated by 
-w 1..XX where XX is the last warning number for a specific release. 
However, it happens once in a while that the exact meaning of an 
existing warning changes between releases, so this is not bullet proof.

-- Alain

Re: [Caml-list] Record field disambiguation in 4.01

[Markus Mottl]

On Sat, Mar 9, 2013 at 6:39 PM, Yaron Minsky <yminsky@gmail.com> wrote:
> A few concerns: one, it's really hard to figure out where the source
> of the conflict is from this message.  It would be nice to get some
> clue from the compiler as to the two definitions that are conflicting.

I haven't investigated this, but my bet would be that the ambiguity is
due to the record in the Core.Std.Date module (y = year).

Regards,
Markus

-- 
Markus Mottl        http://www.ocaml.info        markus.mottl@gmail.com

Re: [Caml-list] Record field disambiguation in 4.01

[Gabriel Scherer]

[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]

In case some people feel the need for more context here, the feature that
is discussed was explained in a blog post of Alain Frisch:
  http://www.lexifi.com/blog/type-based-selection-label-and-constructors

On Sun, Mar 10, 2013 at 4:04 AM, Markus Mottl <markus.mottl@gmail.com>wrote:

> On Sat, Mar 9, 2013 at 6:39 PM, Yaron Minsky <yminsky@gmail.com> wrote:
> > A few concerns: one, it's really hard to figure out where the source
> > of the conflict is from this message.  It would be nice to get some
> > clue from the compiler as to the two definitions that are conflicting.
>
> I haven't investigated this, but my bet would be that the ambiguity is
> due to the record in the Core.Std.Date module (y = year).
>
> Regards,
> Markus
>
> --
> Markus Mottl        http://www.ocaml.info        markus.mottl@gmail.com
>
> --
> Caml-list mailing list.  Subscription management and archives:
> https://sympa.inria.fr/sympa/arc/caml-list
> Beginner's list: http://groups.yahoo.com/group/ocaml_beginners
> Bug reports: http://caml.inria.fr/bin/caml-bugs
>

[-- Attachment #2: Type: text/html, Size: 1959 bytes --]

Re: [Caml-list] Record field disambiguation in 4.01

[Goswin von Brederlow]

On Sun, Mar 10, 2013 at 10:33:14AM +0100, Gabriel Scherer wrote:
> In case some people feel the need for more context here, the feature that
> is discussed was explained in a blog post of Alain Frisch:
>   http://www.lexifi.com/blog/type-based-selection-label-and-constructors
> 
> On Sun, Mar 10, 2013 at 4:04 AM, Markus Mottl <markus.mottl@gmail.com>wrote:
> 
> > On Sat, Mar 9, 2013 at 6:39 PM, Yaron Minsky <yminsky@gmail.com> wrote:
> > > A few concerns: one, it's really hard to figure out where the source
> > > of the conflict is from this message.  It would be nice to get some
> > > clue from the compiler as to the two definitions that are conflicting.
> >
> > I haven't investigated this, but my bet would be that the ambiguity is
> > due to the record in the Core.Std.Date module (y = year).

One more reason to be careful with using open.

On Sat, Mar 09, 2013 at 06:39:56PM -0500, Yaron Minsky wrote:
> open Core.Std
>
> type posn = { x: float; y: float }
>
> let cross_product p1 p2 =
>   p1.x *. p2.y -. p1.y *. p2.x
> ;;
>
> And when I try to compile this, I get the following error:
>
> File "geometry.ml", line 68, characters 13-14:
> Warning 41: this use of y is ambiguous.
> File "geometry.ml", line 1:
> Error: Error-enabled warnings (1 occurrences)
> Command exited with code 2.
>
> This can be fixed by adding an annotation:
>
> let cross_product p1 (p2:posn) =
>   p1.x *. p2.y -. p1.y *. p2.x
> ;;

I think this show a problem of how types propagate only in one direction.

p1 and p2 are both the same type but p1 gets infered corectly while p2
needs the type annotation to avoid a warning. Lets look at the types:

let cross_product p1 p2 =
      'a          'b 'c               | nothing known yet
   p1.x *. p2.y -. p1.y *. p2.x
   ^ label "x" --> posn
           ^ label "y" --> posn or Date
        ^ float -> float -> float

The "y" is ambiguous but just looking at the .* makes the type Date
impossible since Date.y is an integer (I assume). But the information
doesn't flow that way. Looking further:

                  ^ label "y" --> posn or Date, but we already know p1 is posn
                          ^ label "x" --> posn, too late for the warning
                        ^ float -> float -> float

The use of p2.x fixes the type of p2 to posn. But the information
comes too late for the warning when using p2.y. Again, it doesn
t flow that way.

It would be nice if the type inference and disambiguation would allow
type information to flow both ways. Not just from first use to last
but also from last use to first. Up, down, left and right in the tree
representation of the code.

Maybe the use of p2.y with .* (or in general disambiguation on the
expected type of the label) because it could lead to hard to follow
resolution of ambiguous record use. But the p2.x is a clear indication
of what type the programmer wanted.

MfG
	Goswin

Re: [Caml-list] Record field disambiguation in 4.01

[Leo White]

> I think this show a problem of how types propagate only in one direction.
>
> p1 and p2 are both the same type but p1 gets infered corectly while p2
> needs the type annotation to avoid a warning. Lets look at the types:
>
> [...]
>
> It would be nice if the type inference and disambiguation would allow
> type information to flow both ways. Not just from first use to last
> but also from last use to first. Up, down, left and right in the tree
> representation of the code.
>
> Maybe the use of p2.y with .* (or in general disambiguation on the
> expected type of the label) because it could lead to hard to follow
> resolution of ambiguous record use. But the p2.x is a clear indication
> of what type the programmer wanted.

While in a way it is the opposite of what you're asking for, you can get
more consistent behaviour by using the "-principal" argument:

    # let cross_product p1 p2 =
        p1.x *. p2.y -. p1.y *. p2.x
      ;;
    Characters 41-42:
          p1.x *. p2.y -. p1.y *. p2.x
                     ^
    Warning 41: this use of y is ambiguous.
    Characters 49-50:
          p1.x *. p2.y -. p1.y *. p2.x
                             ^
    Warning 41: this use of y is ambiguous.

Regards,

Leo

Re: [Caml-list] Record field disambiguation in 4.01

[Yaron Minsky]

On Sat, Mar 9, 2013 at 10:04 PM, Markus Mottl <markus.mottl@gmail.com> wrote:
> On Sat, Mar 9, 2013 at 6:39 PM, Yaron Minsky <yminsky@gmail.com> wrote:
>> A few concerns: one, it's really hard to figure out where the source
>> of the conflict is from this message.  It would be nice to get some
>> clue from the compiler as to the two definitions that are conflicting.
>
> I haven't investigated this, but my bet would be that the ambiguity is
> due to the record in the Core.Std.Date module (y = year).

Agreed.  I was able to figure it out in the end; it's just that the
error message wasn't terribly helpful.

> Regards,
> Markus
>
> --
> Markus Mottl        http://www.ocaml.info        markus.mottl@gmail.com
>
> --
> Caml-list mailing list.  Subscription management and archives:
> https://sympa.inria.fr/sympa/arc/caml-list
> Beginner's list: http://groups.yahoo.com/group/ocaml_beginners
> Bug reports: http://caml.inria.fr/bin/caml-bugs

Re: [Caml-list] Record field disambiguation in 4.01

[Alain Frisch]

On 03/10/2013 12:39 AM, Yaron Minsky wrote:
> Also, I'm wondering if anyone has thoughts as to how much code this
> change will break?  The answer might be "quite a lot", and it might
> nonetheless be worth it.  But I'm curious what people's thoughts are.

As Jacques said, this is only a warning, which is turned off by default. 
  I expect that all the "unused stuff" warnings also broke most code 
bases around which are compiled with "-w +A -warn-error A".  I'm sure 
you are not suggesting that new warnings should never be triggered on 
code which used to be well-typed by a previous version of the compiler...

The warning tells you something useful: with that record type 
declaration, the reference to field "y" becomes ambiguous, which means 
in particular that (i) your code might become fragile w.r.t. reordering 
of type declarations, (ii) the code might become harder to read (the 
reader might expect that field "y" is typically related to a different 
record type in your code base).  Of course, a more explicit warning 
message, as you suggest, would be even better.

During the discussion on type-based disambiguation, Mark Shinwell 
commented that most record types in JS code base are defined in nested 
modules (and often exposed only through builder functions) to avoid 
label clashes.  Is your example an instance where this is not the case?


Alain

Re: [Caml-list] Record field disambiguation in 4.01

[Yaron Minsky]

On Mon, Mar 11, 2013 at 7:52 AM, Alain Frisch <alain@frisch.fr> wrote:
> On 03/10/2013 12:39 AM, Yaron Minsky wrote:
>>
>> Also, I'm wondering if anyone has thoughts as to how much code this
>> change will break?  The answer might be "quite a lot", and it might
>> nonetheless be worth it.  But I'm curious what people's thoughts are.
>
>
> As Jacques said, this is only a warning, which is turned off by default.  I
> expect that all the "unused stuff" warnings also broke most code bases
> around which are compiled with "-w +A -warn-error A".  I'm sure you are not
> suggesting that new warnings should never be triggered on code which used to
> be well-typed by a previous version of the compiler...

That is entirely sensible.  I normally run with all warnings on, minus
a handful of explicit ones I don't want.  That normally works well
(since most new errors are in fact helpful!)

> The warning tells you something useful: with that record type declaration,
> the reference to field "y" becomes ambiguous, which means in particular that
> (i) your code might become fragile w.r.t. reordering of type declarations,
> (ii) the code might become harder to read (the reader might expect that
> field "y" is typically related to a different record type in your code
> base).  Of course, a more explicit warning message, as you suggest, would be
> even better.

Agreed.

> During the discussion on type-based disambiguation, Mark Shinwell commented
> that most record types in JS code base are defined in nested modules (and
> often exposed only through builder functions) to avoid label clashes.  Is
> your example an instance where this is not the case?

It is.  If I'd turned the warning off, would it have in fact picked
the more recent definition?

y

>
> Alain
>
>
> --
> Caml-list mailing list.  Subscription management and archives:
> https://sympa.inria.fr/sympa/arc/caml-list
> Beginner's list: http://groups.yahoo.com/group/ocaml_beginners
> Bug reports: http://caml.inria.fr/bin/caml-bugs

Re: [Caml-list] Record field disambiguation in 4.01

[Alain Frisch]

On 03/11/2013 07:52 PM, Yaron Minsky wrote:
> It is.  If I'd turned the warning off, would it have in fact picked
> the more recent definition?

Yes, indeed, the type-based disambiguation feature (for labels and 
constructors) did not change how well-typed programs are understood by 
the system.  It only makes more programs well-typed and adds some warnings.


-- Alain

Re: [Caml-list] Record field disambiguation in 4.01

[Jacques Garrigue]

On 2013/03/12, at 3:52, Yaron Minsky <yminsky@janestreet.com> wrote:
> On Mon, Mar 11, 2013 at 7:52 AM, Alain Frisch <alain@frisch.fr> wrote:
>> The warning tells you something useful: with that record type declaration,
>> the reference to field "y" becomes ambiguous, which means in particular that
>> (i) your code might become fragile w.r.t. reordering of type declarations,
>> (ii) the code might become harder to read (the reader might expect that
>> field "y" is typically related to a different record type in your code
>> base).  Of course, a more explicit warning message, as you suggest, would be
>> even better.

I have improved the warning (revision 13395), so that it tells you the types involved.
This should actually help in some hairy situations.

>> During the discussion on type-based disambiguation, Mark Shinwell commented
>> that most record types in JS code base are defined in nested modules (and
>> often exposed only through builder functions) to avoid label clashes.  Is
>> your example an instance where this is not the case?
> 
> It is.  If I'd turned the warning off, would it have in fact picked
> the more recent definition?

Yes, the new strategy is fully backward-compatible.
It just accepts more programs, and there are many new warnings so
that you can tune it to your taste. Using all of them is probably self-defeating,
as for instance warning 42 warns you when something has been accepted
that was not accepted before.

	Jacques

Re: [Caml-list] Record field disambiguation in 4.01

[Jacques Carette]

On 13-03-12 11:05 AM, Jacques Garrigue wrote:
> I have improved the warning (revision 13395), so that it tells you the 
> types involved. This should actually help in some hairy situations. 

Would it also make sense to give the fully qualified names for the 
ambiguous fields?  This should also give a strong hint as to where the 
conflict comes from.

(another)  Jacques

Re: [Caml-list] Record field disambiguation in 4.01

[Gabriel Scherer]

Also on the new warning (thanks for the quick change!): I find it a
bit strange that the message gives a list of possibilities then says
"The first one was selected", while in my mental model the *last*
declared type is chosen by default. I don't know if it's actually
possible to list types by declaration order (levels?), but maybe you
could at least arrange so that the selected one appears last? Or at
least you could reword into "The last declaration of this field has
type M.t, but types M.u and N.t would also be valid".

On Tue, Mar 12, 2013 at 4:29 PM, Jacques Carette <carette@mcmaster.ca> wrote:
> On 13-03-12 11:05 AM, Jacques Garrigue wrote:
>>
>> I have improved the warning (revision 13395), so that it tells you the
>> types involved. This should actually help in some hairy situations.
>
>
> Would it also make sense to give the fully qualified names for the ambiguous
> fields?  This should also give a strong hint as to where the conflict comes
> from.
>
> (another)  Jacques
>
>
>
> --
> Caml-list mailing list.  Subscription management and archives:
> https://sympa.inria.fr/sympa/arc/caml-list
> Beginner's list: http://groups.yahoo.com/group/ocaml_beginners
> Bug reports: http://caml.inria.fr/bin/caml-bugs

Re: [Caml-list] Record field disambiguation in 4.01

[Jacques Garrigue]

[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]

It's just that when you look up the label you get a list in reverse order:
last one first.
Would it be really clearer to reverse it at again when printing?

Jacques
2013/03/13 2:08 "Gabriel Scherer" <gabriel.scherer@gmail.com>:

> Also on the new warning (thanks for the quick change!): I find it a
> bit strange that the message gives a list of possibilities then says
> "The first one was selected", while in my mental model the *last*
> declared type is chosen by default. I don't know if it's actually
> possible to list types by declaration order (levels?), but maybe you
> could at least arrange so that the selected one appears last? Or at
> least you could reword into "The last declaration of this field has
> type M.t, but types M.u and N.t would also be valid".
>
> On Tue, Mar 12, 2013 at 4:29 PM, Jacques Carette <carette@mcmaster.ca>
> wrote:
> > On 13-03-12 11:05 AM, Jacques Garrigue wrote:
> >>
> >> I have improved the warning (revision 13395), so that it tells you the
> >> types involved. This should actually help in some hairy situations.
> >
> >
> > Would it also make sense to give the fully qualified names for the
> ambiguous
> > fields?  This should also give a strong hint as to where the conflict
> comes
> > from.
> >
> > (another)  Jacques
> >
> >
> >
> > --
> > Caml-list mailing list.  Subscription management and archives:
> > https://sympa.inria.fr/sympa/arc/caml-list
> > Beginner's list: http://groups.yahoo.com/group/ocaml_beginners
> > Bug reports: http://caml.inria.fr/bin/caml-bugs
>

[-- Attachment #2: Type: text/html, Size: 2224 bytes --]