Opened 21 months ago

Closed 18 months ago

Last modified 14 months ago

#11357 closed bug (fixed)

Regression when deriving Generic1 on poly-kinded data family

Reported by: RyanGlScott Owned by:
Priority: highest Milestone: 8.0.1
Component: Compiler (CodeGen) Version: 7.11
Keywords: Generics, TypeInType, TypeFamilies Cc: goldfire, aaditmshah@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC rejects valid program Test Case: deriving/should_compile/T11357
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

On GHC 7.10.3, the following code compiles:

{-# LANGUAGE DeriveGeneric #-}
{-# LANGUAGE KindSignatures #-}
{-# LANGUAGE PolyKinds #-}
{-# LANGUAGE TypeFamilies #-}
module ProxyFam where

import GHC.Generics (Generic1)

data family   ProxyFam (a :: k)
data instance ProxyFam (a :: k) = ProxyCon deriving Generic1

But on GHC 8.1, it fails:

$ /opt/ghc/head/bin/ghc ProxyFam.hs 
[1 of 1] Compiling ProxyFam         ( ProxyFam.hs, ProxyFam.o )

ProxyFam.hs:10:53: error:
    • Can't make a derived instance of ‘Generic1 ProxyFam’:
        ProxyFam must not be instantiated; try deriving `ProxyFam k a' instead
    • In the data instance declaration for ‘ProxyFam’

I'm not sure what exactly is going on here, but I have a hunch. The Generic1 typeclass is of kind * -> *, which means that in a Generic1 ProxyFam instance, the kind of a is instantiated to *.

Curiously, though, the same error does not appear when deriving Generic for a normal datatype (e.g., data ProxyFam (a :: k) = ProxyCon deriving Generic1).

Richard, I'm stuck as to how to fix this. I suspect this was triggered by -XTypeInType-related changes, specifically, this change:

  • compiler/typecheck/TcGenGenerics.hs

    diff --git a/compiler/typecheck/TcGenGenerics.hs b/compiler/typecheck/TcGenGenerics.hs
    index 2c5b80e..fb18517 100644 (file)
    a b canDoGenerics tc tc_args 
    147146          --
    148147          -- Data family indices can be instantiated; the `tc_args` here are
    149148          -- the representation tycon args
    150               (if (all isTyVarTy (filterOut isKind tc_args))
     149              (if (all isTyVarTy (filterOutInvisibleTypes tc tc_args))
    151150                then IsValid
    152151                else NotValid (tc_name <+> text "must not be instantiated;" <+>
    153152                               text "try deriving `" <> tc_name <+> tc_tys <>

What exactly does filterOutInvisibleTypes do?

Change History (16)

comment:1 Changed 21 months ago by RyanGlScott

Milestone: 8.0.1

I'm going to set the milestone to 8.0.1, since this affects the upcoming GHC 8.0 and it breaks a fair bit of Generics-related code in the wild. Please change the milestone if you disagree.

comment:2 Changed 21 months ago by thomie

Priority: highhighest
Version: 8.17.11

I believe regressions are release blockers. Setting priority to highest.

comment:3 Changed 21 months ago by RyanGlScott

Something fishy is going on with the kind of the ProxyFam instance TyCon. I added pprTrace-ed the result of tyConKind tc right before the call to filterOutInvisibleTypes mentioned above, and noticed a discrepancy between datatypes and data families:

$ inplace/bin/ghc-stage2 --interactive
GHCi, version 8.1.20160109: http://www.haskell.org/ghc/  :? for help
λ> :set -XDeriveGeneric -XTypeFamilies -XPolyKinds -fprint-explicit-foralls
λ> import GHC.Generics
λ> data Proxy (a :: k) = Proxy deriving Generic1
tc: Proxy
tyConKind tc: forall k_a1Gj. k_a1Gj -> *
tc_args: [*]
filterOutInvisibleTypes tc tc_args: []
all isTyVarTy: True
λ> data family ProxyFam (a :: k)
λ> data instance ProxyFam (a :: k) = ProxyCon deriving Generic1
tc: R:ProxyFamka
tyConKind tc: forall k_a1Ps -> k_a1Ps -> *
tc_args: [*]
filterOutInvisibleTypes tc tc_args: [*]
all isTyVarTy: False

<interactive>:5:53: error:
    • Can't make a derived instance of ‘Generic1 ProxyFam’:
        ProxyFam must not be instantiated; try deriving `ProxyFam k a' instead
    • In the data instance declaration for ‘ProxyFam’

Notice that tyConKind for the datatype Proxy is forall k. k -> * (i.e., k is specified but not visible), but the tyConKind for the ProxyFam instance is forall k -> k -> * (k is visible)! This is definitely the reason why this bug is manifesting itself, although I'm not sure why k is a visible type argument in the ProxyFam instance.

comment:4 Changed 20 months ago by simonpj

Owner: set to goldfire

Richard this one too is "highest". Thanks!

comment:5 Changed 20 months ago by thomie

Keywords: TypeFamilies added

comment:6 Changed 20 months ago by RyanGlScott

Cc: aaditmshah@… added

comment:7 Changed 19 months ago by Richard Eisenberg <eir@…>

In 1eefedf7/ghc:

Fix #11357.

We were looking at a data instance tycon for visibility info,
which is the wrong place to look. Look at the data family tycon
instead.

Also improved the pretty-printing near there to suppress kind
arguments when appropriate.

comment:8 Changed 19 months ago by goldfire

Status: newmerge
Test Case: deriving/should_compile/T11357

comment:9 Changed 18 months ago by RyanGlScott

Well, that patch seemed to fix that problem, but it introduced another one. Vanilla datatypes and data family instances are still inconsistent w.r.t. which type variables are considered "instantiated" in a Generic1 instance. For instance, this is rejected:

λ> data Proxy k (a :: k) = ProxyCon deriving Generic1

<interactive>:32:43: error:
    • Can't make a derived instance of ‘Generic1 (Proxy *)’:
        Proxy must not be instantiated; try deriving `Proxy k a' instead
    • In the data declaration for ‘Proxy’

And rightfully so, since the visible kind binder k is instantiated to *. But now it's possible to have an equivalent instance for a data family that squeaks past this check!

λ> data family ProxyFam (a :: y) (b :: z)
λ> data instance ProxyFam k (a :: k) = ProxyFamCon deriving Generic1

==================== Derived instances ====================
Derived instances:
  instance GHC.Generics.Generic1 (Ghci13.ProxyFam *) where
    ...

I need to investigate further to see why this is the case.

comment:10 Changed 18 months ago by simonpj

So, is comment:9 a release blocker?

comment:11 Changed 18 months ago by bgamari

Resolution: fixed
Status: mergeclosed
Last edited 18 months ago by bgamari (previous) (diff)

comment:12 Changed 18 months ago by bgamari

Owner: goldfire deleted
Resolution: fixed
Status: closednew

Re-opening due to comment:9. Richard, do you think you could have a look at this?

comment:13 in reply to:  10 Changed 18 months ago by goldfire

Replying to simonpj:

So, is comment:9 a release blocker?

I say "no". It seems to be triggered only when you're deriving Generic1 for a datatype parameterized over mixed type and kind variables. This last bit is possible only with -XTypeInType and thus is not a regression.

I'm frankly unsure (without more research) as to what the desired behavior should be here.

comment:14 Changed 18 months ago by goldfire

Resolution: fixed
Status: newclosed

I've spun off comment:9 into #11732. I don't think it's closely related to the original problem reported here, which was a real regression.

comment:15 Changed 14 months ago by Ben Gamari <ben@…>

In 9513fe6/ghc:

Clean up interaction between name cache and built-in syntax

This cleans up various aspects of the handling of built-in syntax in the
original name cache (hopefully resulting in a nice reduction in compiler
allocations),

  * Remove tuple types from original name cache: There is really no
    reason for these to be in the name cache since we already handle
    them specially in interface files to ensure that we can resolve them
    directly to Names, avoiding extraneous name cache lookups.

  * Sadly it's not possible to remove all traces of tuples from the
    name cache, however. Namely we need to keep the tuple type
    representations in since otherwise they would need to be wired-in

  * Remove the special cases for (:), [], and (##) in isBuiltInOcc_maybe
    and rename it to isTupleOcc_maybe

  * Split lookupOrigNameCache into two variants,

     * lookupOrigNameCache': Merely looks up an OccName in the original
       name cache, making no attempt to resolve tuples

     * lookupOrigNameCache: Like the above but handles tuples as well.
       This is given the un-primed name since it does the "obvious"
       thing from the perspective of an API user, who knows nothing of
       our special treatment of tuples.

Arriving at this design took a significant amount of iteration. The
trail of debris leading here can be found in #11357.

Thanks to ezyang and Simon for all of their help in coming to this
solution.

Test Plan: Validate

Reviewers: goldfire, simonpj, austin

Reviewed By: simonpj

Subscribers: thomie, ezyang

Differential Revision: https://phabricator.haskell.org/D2414

GHC Trac Issues: #11357

comment:16 Changed 14 months ago by bgamari

Oh dear, the commit mentioned in comment:15 is actually for #12357.

Note: See TracTickets for help on using tickets.