[GHC] #14307: Nonexistent constructor name + NamedFieldPuns + DuplicateRecordFields can cause ambiguous occurrence message

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[GHC] #14307: Nonexistent constructor name + NamedFieldPuns + DuplicateRecordFields can cause ambiguous occurrence message

GHC - devs mailing list
#14307: Nonexistent constructor name + NamedFieldPuns + DuplicateRecordFields can
cause ambiguous occurrence message
-------------------------------------+-------------------------------------
           Reporter:  mgsloan        |             Owner:  (none)
               Type:  feature        |            Status:  new
  request                            |
           Priority:  low            |         Milestone:
          Component:  Compiler       |           Version:  8.2.1
           Keywords:                 |  Operating System:  Unknown/Multiple
       Architecture:                 |   Type of failure:  None/Unknown
  Unknown/Multiple                   |
          Test Case:                 |        Blocked By:
           Blocking:                 |   Related Tickets:
Differential Rev(s):                 |         Wiki Page:
-------------------------------------+-------------------------------------
 This is a minor issue with error message clarity. I was confused for a few
 minutes because in a more complicated example I did not see the out of
 scope error, and was instead focused on the ambiguity error.

 {{{
 {-# LANGUAGE DuplicateRecordFields #-}
 {-# LANGUAGE NamedFieldPuns #-}

 data A = A { field :: Int }
 data B = B { field :: Int }

 f :: A -> Int
 f C { field } = field
 }}}

 yields

 {{{
 duplicate_records_bug.hs:8:3: error:
     Not in scope: data constructor ‘C’
   |
 8 | f C { field } = field
   |   ^

 duplicate_records_bug.hs:8:7: error:
     Ambiguous occurrence ‘field’
     It could refer to either the field ‘field’,
                              defined at duplicate_records_bug.hs:5:14
                           or the field ‘field’, defined at
 duplicate_records_bug.hs:4:14
   |
 8 | f C { field } = field
   |       ^^^^^
 }}}

 I actually think it would make sense to allow ambiguous identifiers in
 field puns even if DuplicateRecordFields is not enabled.  This makes
 sense, because for an unambiguous constructor, a particular field name is
 always unambiguous.  So, that might be another way to frame this issue:
 Should ambiguous field identifiers always be allowed in puns?

 In particular, this would make things more consistent with
 RecordWildCards, which does not care if the field names shadow anything
 that is in scope / other field names.

 I realize that broadening the code allowed by NamedFieldPuns could lead to
 issues where code written for newer GHC versions does not work with older
 GHC versions.  This certainly will not change the meaning of older code.
 What's the policy on this?

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14307>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14307: Nonexistent constructor name + NamedFieldPuns + DuplicateRecordFields can cause ambiguous occurrence message

GHC - devs mailing list
#14307: Nonexistent constructor name + NamedFieldPuns + DuplicateRecordFields can
cause ambiguous occurrence message
-------------------------------------+-------------------------------------
        Reporter:  mgsloan           |                Owner:  (none)
            Type:  feature request   |               Status:  new
        Priority:  low               |            Milestone:
       Component:  Compiler          |              Version:  8.2.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------
Changes (by nh2):

 * cc: nh2 (added)


--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14307#comment:1>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14307: NamedFieldPuns should allow "ambiguous" field names (was: Nonexistent constructor name + NamedFieldPuns + DuplicateRecordFields can cause ambiguous occurrence message)

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14307: NamedFieldPuns should allow "ambiguous" field names
-------------------------------------+-------------------------------------
        Reporter:  mgsloan           |                Owner:  (none)
            Type:  feature request   |               Status:  new
        Priority:  low               |            Milestone:
       Component:  Compiler          |              Version:  8.2.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------
Description changed by mgsloan:

Old description:

> This is a minor issue with error message clarity. I was confused for a
> few minutes because in a more complicated example I did not see the out
> of scope error, and was instead focused on the ambiguity error.
>
> {{{
> {-# LANGUAGE DuplicateRecordFields #-}
> {-# LANGUAGE NamedFieldPuns #-}
>
> data A = A { field :: Int }
> data B = B { field :: Int }
>
> f :: A -> Int
> f C { field } = field
> }}}
>
> yields
>
> {{{
> duplicate_records_bug.hs:8:3: error:
>     Not in scope: data constructor ‘C’
>   |
> 8 | f C { field } = field
>   |   ^
>
> duplicate_records_bug.hs:8:7: error:
>     Ambiguous occurrence ‘field’
>     It could refer to either the field ‘field’,
>                              defined at duplicate_records_bug.hs:5:14
>                           or the field ‘field’, defined at
> duplicate_records_bug.hs:4:14
>   |
> 8 | f C { field } = field
>   |       ^^^^^
> }}}
>
> I actually think it would make sense to allow ambiguous identifiers in
> field puns even if DuplicateRecordFields is not enabled.  This makes
> sense, because for an unambiguous constructor, a particular field name is
> always unambiguous.  So, that might be another way to frame this issue:
> Should ambiguous field identifiers always be allowed in puns?
>
> In particular, this would make things more consistent with
> RecordWildCards, which does not care if the field names shadow anything
> that is in scope / other field names.
>
> I realize that broadening the code allowed by NamedFieldPuns could lead
> to issues where code written for newer GHC versions does not work with
> older GHC versions.  This certainly will not change the meaning of older
> code.  What's the policy on this?
New description:

 Consider the following example:

 {{{#!haskell
 {-# LANGUAGE NamedFieldPuns #-}

 import DupType

 data A = A { field :: Int }

 f :: A -> Int
 f A { field } = field
 }}}

 with

 {{{#!haskell
 module DupType where

 data B = B { field :: Int }
 }}}

 This results in the following error:

 {{{
 A.hs:8:7: error:
     Ambiguous occurrence ‘field’
     It could refer to either ‘DupType.field’,
                              imported from ‘DupType’ at A.hs:3:1-14
                              (and originally defined at
 DupType.hs:3:14-18)
                           or ‘Main.field’, defined at A.hs:5:14
   |
 8 | f A { field } = field
   |       ^^^^^
 }}}

 This seems like poor behavior, because since a particular constructor is
 used, it is unambiguous which field is intended.  In particular, this is
 inconsistent with `RecordWildCards`.  Consider that `f A { .. } = field`
 compiles perfectly fine.

 I actually encountered this issue in a bit of a different usecase.  I was
 using `NamedFieldPuns` along with `DuplicateFieldNames`.  However, I got
 the constructor name wrong.  After the scope error in the output, there
 was an ambiguous field name error.  This was quite confusing because
 `DuplicateFieldNames` was on, so ambiguity should be fine!  Took me a
 while to realize that the scope error was the root issue.  With the
 constructor name fixed, the code compiled. If the constructor was used to
 resolve field names, then the 2nd error wouldn't have been emitted.

 I realize that broadening the code allowed by `NamedFieldPuns` could lead
 to issues where code written for newer GHC versions does not work with
 older GHC versions.  This certainly will not change the meaning of older
 code.  What's the policy on this?

--

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14307#comment:2>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14307: NamedFieldPuns should allow "ambiguous" field names

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14307: NamedFieldPuns should allow "ambiguous" field names
-------------------------------------+-------------------------------------
        Reporter:  mgsloan           |                Owner:  (none)
            Type:  feature request   |               Status:  new
        Priority:  low               |            Milestone:
       Component:  Compiler          |              Version:  8.2.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 > This seems like poor behavior, because since a particular constructor is
 used, it is unambiguous which field is intended.

 Yes, but you need `-XDisambiguateRecordFields` for that
 ([http://downloads.haskell.org/~ghc/master/users-guide/glasgow_exts.html
 #record-field-disambiguation user manual entry]).  Haskell 2010 specifies
 that the code is should be rejected.  So I think GHC is behaving right
 here.

 Incidentally, `-XDisambiguateRecordFields` is also implied by
 `-XDuplicateRecordFields`.

 > I actually encountered this issue in a bit of a different usecase.

 Yes, here's the code
 {{{
 {-# LANGUAGE DuplicateRecordFields #-}
 {-# LANGUAGE NamedFieldPuns #-}

 module T14307 where

 data A = A { field :: Int }
 data B = B { field :: Int }

 f (C { field }) = field
 }}}
 You get two errors with 8.2
 {{{
 T14307.hs:10:4: error: Not in scope: data constructor ‘C’
    |
 10 | f (C { field }) = field
    |    ^

 T14307.hs:10:8: error:
     Ambiguous occurrence ‘field’
     It could refer to either the field ‘field’,
                              defined at T14307.hs:7:14
                           or the field ‘field’, defined at T14307.hs:6:14
    |
 10 | f (C { field }) = field
    |        ^^^^^
 }}}
 I think your point is that you'd like the second to be suppressed.  I see
 the point.  Are you also ok with getting just one error message from
 {{{
 f (A { fld = x }) = ...
 }}}
 namely `'A' is not in scope`; but no `fld is not in scope`?  That is: if
 the data constructor is not in scope, suppress out-of-scope or ambiguity
 messages for the fields.  I think that'd be fine.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14307#comment:3>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14307: NamedFieldPuns should allow "ambiguous" field names

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14307: NamedFieldPuns should allow "ambiguous" field names
-------------------------------------+-------------------------------------
        Reporter:  mgsloan           |                Owner:  (none)
            Type:  feature request   |               Status:  new
        Priority:  low               |            Milestone:
       Component:  Compiler          |              Version:  8.2.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by Simon Peyton Jones <simonpj@…>):

 In [changeset:"cb767542307b41c91061e743a4a4f448949b34cf/ghc"
 cb767542/ghc]:
 {{{
 #!CommitTicketReference repository="ghc"
 revision="cb767542307b41c91061e743a4a4f448949b34cf"
 Suppress error cascade in record fields

 When a record contruction or pattern uses a data constructor
 that isn't in scope, we may produce spurious ambiguous-field
 errors (Trac #14307).  E.g.

    f (A { fld = x }) = e

 where 'A' is not in scope.  We want to draw attention to the
 out-of-scope data constructor first; once that is fixed we
 can think about the fields.

 This patch suppresses the field errors if the data con is out
 of scope.
 }}}

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14307#comment:4>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14307: NamedFieldPuns should allow "ambiguous" field names

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14307: NamedFieldPuns should allow "ambiguous" field names
-------------------------------------+-------------------------------------
        Reporter:  mgsloan           |                Owner:  (none)
            Type:  feature request   |               Status:  closed
        Priority:  low               |            Milestone:
       Component:  Compiler          |              Version:  8.2.1
      Resolution:  fixed             |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
                                     |  rename/should_fail/T14307
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------
Changes (by simonpj):

 * status:  new => closed
 * testcase:   => rename/should_fail/T14307
 * resolution:   => fixed


Comment:

 OK I've done that.  We can revert if we decide we want both errors after
 all.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14307#comment:5>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14307: NamedFieldPuns should allow "ambiguous" field names

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14307: NamedFieldPuns should allow "ambiguous" field names
-------------------------------------+-------------------------------------
        Reporter:  mgsloan           |                Owner:  (none)
            Type:  feature request   |               Status:  closed
        Priority:  low               |            Milestone:
       Component:  Compiler          |              Version:  8.2.1
      Resolution:  fixed             |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
                                     |  rename/should_fail/T14307
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by mgsloan):

 Great, thanks for the quick fix Simon!

 > Yes, but you need -XDisambiguateRecordFields for that (​user manual
 entry). Haskell 2010 specifies that the code is should be rejected. So I
 think GHC is behaving right here.

 Interesting, I didn't know about that one, cool!  Glad there's a way
 around that one.  Perhaps the error message could mention the extension
 since it's a rarer one?

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14307#comment:6>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14307: NamedFieldPuns should allow "ambiguous" field names

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14307: NamedFieldPuns should allow "ambiguous" field names
-------------------------------------+-------------------------------------
        Reporter:  mgsloan           |                Owner:  (none)
            Type:  feature request   |               Status:  closed
        Priority:  low               |            Milestone:
       Component:  Compiler          |              Version:  8.2.1
      Resolution:  fixed             |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
                                     |  rename/should_fail/T14307
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 >  Perhaps the error message could mention the extension since it's a
 rarer one?

 That's a good idea, but as the code stands it would be fiddly to
 implement.  I had a look at the tricky logic in
 `RnEnv.lookupSubBndrOcc_helper` and backed off.

 Surely this code can be made more perspicuous! If anyone wants to have a
 go I'd be happy to help.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14307#comment:7>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14307: NamedFieldPuns should allow "ambiguous" field names

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14307: NamedFieldPuns should allow "ambiguous" field names
-------------------------------------+-------------------------------------
        Reporter:  mgsloan           |                Owner:  (none)
            Type:  feature request   |               Status:  new
        Priority:  low               |            Milestone:
       Component:  Compiler          |              Version:  8.2.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
                                     |  rename/should_fail/T14307
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------
Changes (by simonpj):

 * status:  closed => new
 * resolution:  fixed =>


--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14307#comment:8>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14307: NamedFieldPuns should allow "ambiguous" field names

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14307: NamedFieldPuns should allow "ambiguous" field names
-------------------------------------+-------------------------------------
        Reporter:  mgsloan           |                Owner:  (none)
            Type:  feature request   |               Status:  new
        Priority:  low               |            Milestone:
       Component:  Compiler          |              Version:  8.2.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
                                     |  rename/should_fail/T14307
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by mpickering):

 I've refactored that function once recently and I think the logic in this
 area is inherently tricky!

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14307#comment:9>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14307: NamedFieldPuns should allow "ambiguous" field names

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14307: NamedFieldPuns should allow "ambiguous" field names
-------------------------------------+-------------------------------------
        Reporter:  mgsloan           |                Owner:  (none)
            Type:  feature request   |               Status:  new
        Priority:  low               |            Milestone:
       Component:  Compiler          |              Version:  8.2.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
                                     |  rename/should_fail/T14307
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by Simon Peyton Jones <simonpj@…>):

 In [changeset:"7720c293e7f5ca5089e3d154aad99e8060d6ac63/ghc"
 7720c293/ghc]:
 {{{
 #!CommitTicketReference repository="ghc"
 revision="7720c293e7f5ca5089e3d154aad99e8060d6ac63"
 Tidy up some convoluted "child/parent" code

 In investigating something else (Trac #14307) I encountered the
 wonders of TcRnExports.lookupChildrenExport, and the data
 type ChildLookupResult.

 I managed to remove the NameErr constructor from ChildLookupResult,
 and simplify the code significantly at the same time.

 This is just refactoring; no change in behaviour.
 }}}

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14307#comment:10>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets