Opened 22 months ago

Closed 22 months ago

Last modified 17 months ago

#13272 closed bug (fixed)

DeriveAnyClass regression involving a rigid type variable

Reported by: RyanGlScott Owned by:
Priority: high Milestone: 8.2.1
Component: Compiler (Type checker) Version: 8.1
Keywords: Generics, deriving Cc: simonpj
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC rejects valid program Test Case: deriving/should_compile/T13272, T13272a
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description (last modified by RyanGlScott)

I was writing up an example to show off how DeriveAnyClass has improved since 639e702b6129f501c539b158b982ed8489e3d09c, and wouldn't you know it, the example doesn't actually compile anymore post-639e702b6129f501c539b158b982ed8489e3d09c. Oopsie.

{-# LANGUAGE DefaultSignatures #-}
{-# LANGUAGE DeriveAnyClass #-}
{-# LANGUAGE DeriveGeneric #-}
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TypeFamilies #-}
module TypeName where

import GHC.Generics

class TypeName a where
  typeName         :: forall proxy.
                      proxy a -> String
  default typeName :: forall proxy d f.
                      (Generic a, Rep a ~ D1 d f, Datatype d)
                   => proxy a -> String
  typeName _ = gtypeName $ from (undefined :: a)

gtypeName :: Datatype d => D1 d f p -> String
gtypeName = datatypeName

data T a = MkT a
  deriving (Generic, TypeName)

This compiles before that commit. After it, however, it fails with the error:

[1 of 1] Compiling TypeName         ( Bug.hs, interpreted )

Bug.hs:23:22: error:
    • Couldn't match type ‘f’
                     with ‘C1
                             ('MetaCons "MkT" 'PrefixI 'False)
                             (S1
                                ('MetaSel
                                   'Nothing 'NoSourceUnpackedness 'NoSourceStrictness 'DecidedLazy)
                                (Rec0 a))’
        arising from the 'deriving' clause of a data type declaration
      ‘f’ is a rigid type variable bound by
        the deriving clause for ‘TypeName (T a)’ at Bug.hs:14:38
    • When deriving the instance for (TypeName (T a))
   |
23 |   deriving (Generic, TypeName)
   |                      ^^^^^^^^

I'm not sure why it's complaining only about f and not, say, d.

Attachments (1)

13272-ddump-tc-trace-loop.txt (18.5 KB) - added by RyanGlScott 22 months ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 22 months ago by RyanGlScott

And it gets worse if you make a slight tweak to the original program by introducing an intermediary type variable:

{-# LANGUAGE DefaultSignatures #-}
{-# LANGUAGE DeriveAnyClass #-}
{-# LANGUAGE DeriveGeneric #-}
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TypeFamilies #-}
module TypeName where

import GHC.Generics

class TypeName a where
  typeName         :: proxy a -> String
  default typeName :: (Generic a, Rep a ~ gg, gg ~ D1 d f, Datatype d)
                   => proxy a -> String
  typeName _ = gtypeName $ from (undefined :: a)

gtypeName :: Datatype d => D1 d f p -> String
gtypeName = datatypeName

data T a = MkT a
  deriving (Generic, TypeName)

Then it sends the compiler (presumably the constraint solver) into an infinite loop! I'll attach the portion of -ddump-tc-trace's output that appears to be looping.

Changed 22 months ago by RyanGlScott

comment:2 Changed 22 months ago by RyanGlScott

Description: modified (diff)

comment:3 Changed 22 months ago by RyanGlScott

Cc: simonpj added

FWIW, in the example in comment:1, the loop happens upon the first call to solveWantedsAndDrop here.

I'm a bit clueless as to where to begin searching for a place to debug this. Simon, do you have any ideas?

comment:4 Changed 22 months ago by bgamari

Milestone: 8.2.18.4.1

We discussed this on the call and Simon said that fixing this correctly will be non-trivial. I'm (sadly) going to bump to 8.4 unless something changes.

comment:5 Changed 22 months ago by RyanGlScott

It's still a bit of a bummer that DeriveAnyClass has become worse. Perhaps we should just revert 639e702b6129f501c539b158b982ed8489e3d09c for now?

comment:6 Changed 22 months ago by rwbarton

Isn't it better now in other ways, though?

comment:7 Changed 22 months ago by RyanGlScott

Isn't it better now in other ways, though?

That's true. I really don't know how many pieces of code in the wild actually structure their default type signatures in a way that's analogous to the original program, so if not too many folks complain about it during the 8.2 release candidate window, I suppose we could deem 639e702b6129f501c539b158b982ed8489e3d09c a net positive and live with it for now.

comment:8 Changed 22 months ago by simonpj

I thikn I have a fix actually. Wait till tomorrow

comment:9 Changed 22 months ago by Simon Peyton Jones <simonpj@…>

In fd841f87/ghc:

Fix DeriveAnyClass (again)

This patch fixes Trac #13272.  The general approach was fine, but
we were simply not generating the correct implication constraint
(in particular generating fresh unification variables).  I added
a lot more commentary to Note [Gathering and simplifying
constraints for DeriveAnyClass]

I'm still not very happy with the overall architecture.  It feels
more complicate than it should.

comment:10 Changed 22 months ago by simonpj

Resolution: fixed
Status: newclosed
Test Case: deriving/should_compile/T13272, T13272a

comment:11 Changed 22 months ago by RyanGlScott

Milestone: 8.4.18.2.1

Many thanks Simon, you're a lifesaver.

For my edification, which part in particular was causing the reported error? Was it the fact that we were creating new variables in inferConstraintsDAC without using pushTcLevelM? Or was it due to not using explicit unification variables (via newMetaTyVarsX)?

comment:12 Changed 22 months ago by simonpj

The latter!

comment:13 Changed 17 months ago by RyanGlScott

Keywords: deriving added
Note: See TracTickets for help on using tickets.