Opened 4 years ago

Closed 3 years ago

Last modified 2 years ago

#11509 closed bug (fixed)

Incorrect error message in StandaloneDeriving: "The data constructors of <typeclass> are not all in scope"

Reported by: edsko Owned by:
Priority: normal Milestone: 8.2.1
Component: Compiler Version: 8.0.1-rc1
Keywords: deriving Cc: RyanGlScott
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D2558
Wiki Page:

Description

Consider

{-# OPTIONS_GHC -Wall #-}
{-# LANGUAGE ConstraintKinds            #-}
{-# LANGUAGE FlexibleInstances          #-}
{-# LANGUAGE GADTs                      #-}
{-# LANGUAGE GeneralizedNewtypeDeriving #-}
{-# LANGUAGE KindSignatures             #-}
{-# LANGUAGE ScopedTypeVariables        #-}
{-# LANGUAGE StandaloneDeriving         #-}
{-# LANGUAGE StaticPointers             #-}
{-# LANGUAGE UndecidableInstances       #-}
{-# LANGUAGE UndecidableSuperClasses    #-}
module Bug where

import Data.Kind
import Data.Typeable
import GHC.StaticPtr

{-------------------------------------------------------------------------------
  Standard Cloud-Haskell-like infrastructure

  See <https://ghc.haskell.org/trac/ghc/wiki/TypeableT> for a dicussion of 'SC'.
-------------------------------------------------------------------------------}

class Serializable a -- empty class, just for demonstration purposes

instance Serializable a => Serializable [a]

data Static :: * -> * where
  StaticPtr :: StaticPtr a -> Static a
  StaticApp :: Static (a -> b) -> Static a -> Static b

staticApp :: StaticPtr (a -> b) -> Static a -> Static b
staticApp = StaticApp .  StaticPtr

data Dict :: Constraint -> * where
  Dict :: c => Dict c

class c => SC c where
  dict :: Static (Dict c)

instance (Typeable a, SC (Serializable a)) => SC (Serializable [a]) where
  dict = aux `staticApp` dict
    where
      aux :: StaticPtr (Dict (Serializable a) -> Dict (Serializable [a]))
      aux = static (\Dict -> Dict)

{-------------------------------------------------------------------------------
  Demonstrate the bug
-------------------------------------------------------------------------------}

newtype MyList a = MyList [a]

deriving instance (Typeable a, SC (Serializable a)) => SC (Serializable (MyList a))

This gives the following type error:

Bug1.hs:40:1: error:
    • Can't make a derived instance of ‘SC (Serializable (MyList a))’:
        The data constructors of ‘Serializable’ are not all in scope
          so you cannot derive an instance for it
    • In the stand-alone deriving instance for
        ‘SC (Serializable a) => SC (Serializable (MyList a))’

This of course doesn't make much sense: Serializable is a type class, not a datatype, and doesn't have data constructors.

I wasn't sure if this deriving clause was going to work at all, or whether I would expect it to. Since MyList is a newtype wrapper around [a], and we have the requisite instance

instance (Typeable a, SC (Serializable a)) => SC (Serializable [a]) 

I was kind of hoping that GeneralizedNewtypeDeriving would work its magic. However, even if it cannot, the error message should probably change.

Attachments (1)

Bug11509.hs (2.2 KB) - added by edsko 4 years ago.
Test case with some CPP to make it work with 7.10 and 8.0

Download all attachments as: .zip

Change History (16)

comment:1 Changed 4 years ago by simonpj

Ha! Interesting. The error message embodies the assumption that in

instance .. => C (T a b c)

that T is an algebraic data type; but here is is a class. So a question arises:

  • Does GND work for classes? Well, does this work?
    instance (Typeable a, SC (Serializable a)) => SC (Serializable (MyList a)) where
       dict = coerce (dict :: Static (Dict (Serializable [a])))
    
    If we got past the test, I think that's what we'd generate. I suspect you'll get a similar error from the corece, but try it.

comment:2 Changed 4 years ago by edsko

After adding the required role annotation to Serializable, this works:

instance (Typeable a, SC (Serializable a)) => SC (Serializable (MyList a)) where
  dict = coerce (dict :: Static (Dict (Serializable [a])))

but the standalone deriving clause still gives the same error as reported above.

comment:3 Changed 4 years ago by edsko

Incidentally, when Serializable has a nominal role, that call to coerce is rejected by ghc 8.0 with

Bug.hs:70:10: error:
    • Couldn't match type ‘[a]’ with ‘MyList a’
        arising from a use of ‘coerce’
    • In the expression:
        coerce (dict :: Static (Dict (Serializable [a])))
      In an equation for ‘dict’:
          dict = coerce (dict :: Static (Dict (Serializable [a])))
      In the instance declaration for ‘SC (Serializable (MyList a))’
    • Relevant bindings include
        dict :: Static (Dict (Serializable (MyList a)))
          (bound at Bug.hs:70:3)

which I found somewhat confusing; "Couldn't match type [a] with MyList a arising from a use of coerce" isn't very obvious; ghc 7.10.3 here is much much clearer:

Bug.hs:70:10:
    Couldn't match type ‘MyList a’ with ‘[a]’
    arising from trying to show that the representations of
      ‘Static (Dict (Serializable [a]))’ and
      ‘Static (Dict (Serializable (MyList a)))’ are the same
    Relevant role signatures:
      type role [] representational
      type role MyList representational
      type role Dict representational
      type role Static representational
      type role Serializable nominal
    Relevant bindings include
      dict :: Static (Dict (Serializable (MyList a)))
        (bound at Bug.hs:70:3)
    In the expression:
      coerce (dict :: Static (Dict (Serializable [a])))
    In an equation for ‘dict’:
        dict = coerce (dict :: Static (Dict (Serializable [a])))
    In the instance declaration for ‘SC (Serializable (MyList a))’

Changed 4 years ago by edsko

Attachment: Bug11509.hs added

Test case with some CPP to make it work with 7.10 and 8.0

comment:4 Changed 3 years ago by RyanGlScott

Cc: RyanGlScott added

comment:5 Changed 3 years ago by RyanGlScott

As Simon says, GHC's various deriving bits rely heavily on the assumption that the outermost type constructor is that of a datatype. I honestly have no idea how you'd make this work for typeclasses, especially since in your example, GHC would need some way to "tunnel down" into a type and figure out what the newtype is (if there even is one!). Throw in things like MultiParamTypeClasses and type synonyms, and I imagine that this would get extremely hairy.

Would you be happy with a different error message to the tune of:

Bug1.hs:40:1: error:
    • Can't make a derived instance of ‘SC (Serializable (MyList a))’:
        Cannot derive an instance of the form ‘C (T a b c)‘
          where ‘T‘ is a type class
    • In the stand-alone deriving instance for
        ‘SC (Serializable a) => SC (Serializable (MyList a))’

comment:6 Changed 3 years ago by edsko

Yes, that would at least be a much clearer error message.

comment:7 Changed 3 years ago by RyanGlScott

Owner: set to RyanGlScott

After #10598, GHC has been refactored to propagate information about which deriving mechanism was chosen further into the compiler, so fixing this issue properly (i.e., erroring on stock or newtype deriving, but not on anyclass deriving) is much easier now. I'll take this one.

comment:8 Changed 3 years ago by RyanGlScott

Differential Rev(s): Phab:D2558
Status: newpatch

At least, I thought deriving strategies would come into play, but it turns out a different check already present in GHC gives a pretty reasonable error message for this scenario already. So if Phab:D2558 is applied, the code above gives the error message:

    • Can't make a derived instance of ‘SC (Serializable (MyList a))’:
        ‘SC’ is not a stock derivable class (Eq, Show, etc.)
        Try enabling DeriveAnyClass
    • In the stand-alone deriving instance for
        ‘(Typeable a, SC (Serializable a)) => SC (Serializable (MyList a))’

DeriveAnyClass technically can't be used derive instances of kind a -> Constraint right now, but that will change once #12144 is fixed.

Last edited 3 years ago by RyanGlScott (previous) (diff)

comment:9 Changed 3 years ago by Ben Gamari <ben@…>

In 23cf32da/ghc:

Disallow standalone deriving declarations involving unboxed tuples or sums

There was an awful leak where GHC permitted standalone `deriving`
declarations to create instances for unboxed sum or tuple types. This
fortifies the checks that GHC performs to catch this scenario and give
an appropriate error message.

Fixes #11509.

Test Plan: ./validate

Reviewers: goldfire, austin, bgamari

Reviewed By: bgamari

Subscribers: thomie

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

GHC Trac Issues: #11509

comment:10 Changed 3 years ago by bgamari

Milestone: 8.2.1
Resolution: fixed
Status: patchclosed

Unless others would prefer we merge this I think I will let this one slide until 8.2.

comment:11 Changed 3 years ago by RyanGlScott

Owner: RyanGlScott deleted
Resolution: fixed
Status: closednew

Ack! I apologize Ben, I accidentally linked to the wrong Trac ticket number from the Diff description. Phab:D2557 was meant for #12512, not this one.

comment:12 Changed 3 years ago by RyanGlScott

Status: newpatch

comment:13 Changed 3 years ago by Ben Gamari <ben@…>

In d5a4e49d/ghc:

Make error when deriving an instance for a typeclass less misleading

Before, when you attempted to derive an instance for a typeclass,
e.g.,

```
class C1 (a :: Constraint) where
class C2 where

deriving instance C1 C2
```

GHC would complain that `C2`'s data constructors aren't in scope. But
that
makes no sense, since typeclasses don't have constructors! By refining
the
checks that GHC performs when deriving, we can make the error message a
little more sensible.

This also cleans up a related `DeriveAnyClass` infelicity. Before, you
wouldn't have been able to compile code like this:

```
import System.IO (Handle)
class C a
deriving instance C Handle
```

Since GHC was requiring that all data constructors of `Handle` be in
scope. But `DeriveAnyClass` doesn't even generate code that mentions
any data constructors, so this requirement is silly!

Fixes #11509.

Test Plan: make test TEST=T11509

Reviewers: simonpj, austin, bgamari

Reviewed By: simonpj, bgamari

Subscribers: thomie, simonpj

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

GHC Trac Issues: #11509

comment:14 Changed 3 years ago by bgamari

Resolution: fixed
Status: patchclosed

This would be quite an effort to backport and the issue seems non-critical. Let's punt this to 8.2.

comment:15 Changed 2 years ago by RyanGlScott

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