Remove the instantiation check when deriving Generic(1)
ClosedPublic

Authored by RyanGlScott on Mar 29 2016, 11:18 PM.

Details

Summary

Previously, deriving Generic(1) bailed out when attempting to
instantiate visible type parameters (Trac #5939), but this instantiation
check was quite fragile and doesn't interact well with -XTypeInType. It has
been decided that Generic(1) shouldn't be subjected to this check anyway,
so it has been removed, and gen_Generic_binds's machinery has been updated
to substitute the type variables in a generated Rep/Rep1 instance with
the user-supplied type arguments.

In addition, this also refactors Condition in TcDeriv a bit. Namely,
since we no longer need tc_args to check any conditions, the [Type]
component of Condition has been removed.

Fixes Trac #11732.

Test Plan

./validate

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
RyanGlScott retitled this revision from to Remove the instantiation check when deriving Generic(1).
RyanGlScott updated this object.
RyanGlScott edited the test plan for this revision. (Show Details)
RyanGlScott updated the Trac tickets for this revision.
RyanGlScott edited edge metadata.
simonpj requested changes to this revision.Mar 30 2016, 2:00 AM
simonpj edited edge metadata.

Good. But look at Note [Unify kinds in deriving] in TcDeriv.

  • It overlaps with your new Note; somehow the two should be combined
  • The combined Note should be referenced both from the tcUnifyTy in TcDeriv and from the relevant place in TcGenGenerics

I was a bit surprised to see that there is no call to unify in TcGenGenerics. So somehow it's less general than the code in TcDeriv.deriveTyData? Or maybe you just know enough that it's straight instantiation with no need to do matching or unification?

This revision now requires changes to proceed.Mar 30 2016, 2:00 AM
RyanGlScott updated this revision to Diff 7134.Mar 30 2016, 9:01 AM
RyanGlScott edited edge metadata.
  • Clarify Notes via Simon's suggestions

Good. But look at Note [Unify kinds in deriving] in TcDeriv.

  • It overlaps with your new Note; somehow the two should be combined
  • The combined Note should be referenced both from the tcUnifyTy in TcDeriv and from the relevant place in TcGenGenerics

Done.

I was a bit surprised to see that there is no call to unify in TcGenGenerics. So somehow it's less general than the code in TcDeriv.deriveTyData? Or maybe you just know enough that it's straight instantiation with no need to do matching or unification?

Ah, that was something I didn't make very clear. All of the necessary kind unification happens in TcDeriv (specifically, this line), but the problem is that the function in TcGenGenerics that generates a Rep(1) instance was completely oblivious to this (it works over the tyConTyVars of the datatype's TyCon). Thus all that had to be done was create a TCvSubst mapping the tyConTyVars to the fully instantiated types. Since this is pretty Generic(1)-specific, I've added its own note in TcGenGenerics (that gives lip service to Note [Unify kinds in deriving]).

RyanGlScott edited edge metadata.Mar 30 2016, 9:12 AM
RyanGlScott updated the Trac tickets for this revision.
bgamari accepted this revision.Apr 7 2016, 5:17 AM
bgamari edited edge metadata.

Looks good to me.

simonpj accepted this revision.Apr 7 2016, 7:54 AM
simonpj edited edge metadata.

ok by me

This revision is now accepted and ready to land.Apr 7 2016, 7:54 AM
RyanGlScott planned changes to this revision.Apr 10 2016, 12:54 PM

@bgamari pointed out in IRC-land that this trips up an ASSERT:

ghc-stage1: panic! (the 'impossible' happened)
  (GHC version 8.1.20160410 for x86_64-unknown-linux):
	ASSERT failed!
  file compiler/types/TyCoRep.hs line 2091
  in_scope InScope {a_12 b_13 c_14 d_15 e_16 f_17}
  tenv [12 :-> a_a2Cd[sk], 13 :-> b_a2Ce[sk], 14 :-> c_a2Cf[sk],
        15 :-> d_a2Cg[sk], 16 :-> e_a2Ch[sk], 17 :-> f_a2Ci[sk]]
  tenvFVs [a2Cd :-> a_a2Cd[sk], a2Ce :-> b_a2Ce[sk],
           a2Cf :-> c_a2Cf[sk], a2Cg :-> d_a2Cg[sk], a2Ch :-> e_a2Ch[sk],
           a2Ci :-> f_a2Ci[sk]]
  cenv []
  cenvFVs []
  tys [D1
         ('MetaData "(,,,,,,)" "GHC.Tuple" "ghc-prim" 'False)
         (C1
            ('MetaCons "(,,,,,,)" 'PrefixI 'False)
            ((S1
                ('MetaSel
                   'Nothing 'NoSourceUnpackedness 'NoSourceStrictness 'DecidedLazy)
                (Rec0 a_12)
              :*: (S1
                     ('MetaSel
                        'Nothing 'NoSourceUnpackedness 'NoSourceStrictness 'DecidedLazy)
                     (Rec0 b_13)
                   :*: S1
                         ('MetaSel
                            'Nothing 'NoSourceUnpackedness 'NoSourceStrictness 'DecidedLazy)
libraries/base/ghc.mk:4: recipe for target 'libraries/base/dist-install/build/GHC/Generics.o' failed
                         (Rec0 c_14)))
             :*: ((S1
                     ('MetaSel
                        'Nothing 'NoSourceUnpackedness 'NoSourceStrictness 'DecidedLazy)
                     (Rec0 d_15)
                   :*: S1
                         ('MetaSel
                            'Nothing 'NoSourceUnpackedness 'NoSourceStrictness 'DecidedLazy)
                         (Rec0 e_16))
                  :*: (S1
                         ('MetaSel
                            'Nothing 'NoSourceUnpackedness 'NoSourceStrictness 'DecidedLazy)
                         (Rec0 f_17)
                       :*: S1
                             ('MetaSel
                                'Nothing 'NoSourceUnpackedness 'NoSourceStrictness 'DecidedLazy)
                             Par1))))]
  cos []
RyanGlScott updated this revision to Diff 7232.Apr 10 2016, 3:02 PM
RyanGlScott edited edge metadata.
  • Fix assertion error, update test stderr
This revision is now accepted and ready to land.Apr 10 2016, 3:02 PM
This revision was automatically updated to reflect the committed changes.