[GHC] #15149: Identical distinct type family fields miscompiled

[GHC]

#15149: Identical distinct type family fields miscompiled
-------------------------------------+-------------------------------------
        Reporter:  NeilMitchell      |                Owner:  (none)
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:  8.6.1
       Component:  Compiler          |              Version:  8.4.2
      Resolution:                    |             Keywords:  ORF
Operating System:  Unknown/Multiple  |         Architecture:
 Type of failure:  GHC rejects       |  Unknown/Multiple
  valid program                      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------
Changes (by simonpj):

 * cc: adamgundry (added)


Comment:

 I investigated.  Here is what is going on:

 * When compiling module `An`, there are two `an`'s in scope

 * You might resaonably think that with `DisamgiguateRecordFields`, the
 `an` in `AnInt { an = 1 }` is ''obviously'' the `an` from module `AnInt`.

 * But in fact GHC's renamer uses the ''type constructor'', not the ''data
 constructor'' to disambiguate which `an` you mean.  And both `an`'s have
 the same "parent", namely the type constructor `An`.

 This was fine before data families, because a single type constructor
 could not have data constructors with distinct `an` record fields.  But
 now it can.

 A short term fix is to complain about ambiguity rather than arbitrarily
 picking one (as is done now).  This happens here in `TcExpr`:
 {{{
 tcRecordField con_like flds_w_tys (L loc (FieldOcc sel_name lbl)) rhs
   | Just field_ty <- assocMaybe flds_w_tys sel_name
   = ...
 }}}
 That `assocMaybe` just picks the first.  So we could fix that.

 But how can we fix it properly?  After all, all the information is there,
 staring us in the face.

 * One way forward might be to say that the data constructor `AnInt` is the
 "parent" of `AnInt.an`, and the type constructor `An` is the parent of the
 data constructor `AnInt`.  But that change would mean we had a multi-level
 parent structure, with consequences that are hard to foresee.

 * But I think a better way is to ''stop'' trying to make the choice in the
 renamer.  Instead in a `HsRecordFields` structure, keep the field names as
 not-yet-looked-up `OccNames` in the renamer, and look them up in the
 typechecker.  At that point, we have the data constructor available in its
 full glory and don't need to bother with the tricky parent structures in
 the renamer.

 Adam, do you think that would work?

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