Opened 3 years ago

Last modified 9 months ago

#13154 new bug

Standalone-derived anyclass instances aren't as permissive as empty instances

Reported by: RyanGlScott Owned by:
Priority: normal Milestone: 8.10.1
Component: Compiler (Type checker) Version: 8.1
Keywords: deriving Cc: dfeuer, Iceland_jack
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC rejects valid program Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D4337, Phab:D4370
Wiki Page:

Description

Note that these are accepted:

class Foo (a :: TYPE ('TupleRep '[]))
instance Foo (a :: TYPE ('TupleRep '[]))
instance Foo (##)

But if you try to write these same instances using DeriveAnyClass and StandaloneDeriving, they'll be rejected for no good reason:

deriving instance Foo (a :: TYPE ('TupleRep '[]))

{-
    • Can't make a derived instance of ‘Foo a’:
        The last argument of the instance must be a data or newtype application
-}

deriving instance Foo (##)

{-
    • Can't make a derived instance of ‘Foo (# #)’:
        The last argument of the instance cannot be an unboxed tuple
-}

The latter error is a vestige of #12512, and should be easy enough to rectify. The former error is a bit tricker to fix, since much of the code in TcDeriv/TcDerivInfer assumes that the type to which the class is applied to in the instead head is a datatype or newtype TyCon. It's apparent that this doesn't hold true in the presence of DeriveAnyClass, however, so that code should be refactored to reflect this.

For practical reasons, however, it might be best to wait until after #12144 is fixed to tackle this one, since that splits out some logic in TcDerivInfer for DeriveAnyClass so as to make it no longer rely on the aforementioned TyCon invariant.

Change History (21)

comment:1 Changed 3 years ago by RyanGlScott

Keywords: Generics added

comment:2 Changed 3 years ago by RyanGlScott

Milestone: 8.4.1

Another thing that standalone DeriveAnyClass currently doesn't allow (but bare instances do) is the use of nullary classes. That is, this is allowed:

{-# LANGUAGE MultiParamTypeClasses #-}
module Allowed where

class C
instance C

but this is not:

{-# LANGUAGE DeriveAnyClass #-}
{-# LANGUAGE MultiParamTypeClasses #-}
{-# LANGUAGE StandaloneDeriving #-}
module Bug where

class C
deriving instance C
Bug.hs:7:1: error:
    • Cannot derive instances for nullary classes
    • In the stand-alone deriving instance for ‘C’

comment:3 Changed 3 years ago by RyanGlScott

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

comment:4 Changed 3 years ago by dfeuer

I'm missing something: why is this a real problem? Adding a bunch of logic to fix it seems like overkill to me, personally.

comment:5 Changed 3 years ago by dfeuer

Cc: dfeuer added

comment:6 Changed 2 years ago by RyanGlScott

Keywords: deriving added; Generics removed

comment:7 Changed 20 months ago by bgamari

Milestone: 8.4.18.6.1

This ticket won't be resolved in 8.4; remilestoning for 8.6. Do holler if you are affected by this or would otherwise like to work on it.

comment:8 Changed 20 months ago by RyanGlScott

Another component of this issue which Iceland_jack noticed is that one cannot standalone-derive any instances for primitive types, among which include (->):

{-# LANGUAGE DataKinds #-}
{-# LANGUAGE DeriveAnyClass #-}
{-# LANGUAGE KindSignatures #-}
{-# LANGUAGE MagicHash #-}
{-# LANGUAGE StandaloneDeriving #-}
module Bug where

import GHC.Exts

class C1 a
deriving instance C1 (a -> b)

class C2 (a :: TYPE 'IntRep)
deriving instance C2 Int#
$ ghc/inplace/bin/ghc-stage2 Bug.hs
[1 of 1] Compiling Bug              ( Bug.hs, Bug.o )

Bug.hs:11:1: error:
    • Can't make a derived instance of ‘C1 (a -> b)’:
        The last argument of the instance must be a data or newtype application
    • In the stand-alone deriving instance for ‘C1 (a -> b)’
   |
11 | deriving instance C1 (a -> b)
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Bug.hs:14:1: error:
    • Can't make a derived instance of ‘C2 Int#’:
        The last argument of the instance must be a data or newtype application
    • In the stand-alone deriving instance for ‘C2 Int#’
   |
14 | deriving instance C2 Int#
   | ^^^^^^^^^^^^^^^^^^^^^^^^^

I'll lump this bug under this ticket.

comment:9 Changed 20 months ago by RyanGlScott

Differential Rev(s): Phab:D3337
Owner: RyanGlScott deleted
Status: patchnew

Removing Phab:D3337 as the Diff for this, since it's been abandoned.

comment:10 Changed 20 months ago by RyanGlScott

Owner: set to RyanGlScott

See also commits:

Which have made progress towards fixing this ticket (but on whose commit messages I forgot to mention it).

comment:11 Changed 20 months ago by Iceland_jack

Cc: Iceland_jack added
Differential Rev(s): Phab:D3337

comment:12 Changed 20 months ago by Iceland_jack

Differential Rev(s): Phab:D3337

comment:13 Changed 20 months ago by RyanGlScott

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

Phab:D4337 fixes the issue in comment:8. It does not fix the entire issue, as there is still the degenerate case of:

{-# LANGUAGE DeriveAnyClass #-}
{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE StandaloneDeriving #-}

class C a
deriving instance C a

Which needs further patching to fix.

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

In 1a911f21/ghc:

Sequester deriving-related validity check into cond_stdOK

Currently, any standalone-derived instance must satisfy the
property that the tycon of the data type having an instance being
derived for it must be either a normal ADT tycon or a data family
tycon. But there are several other primitive tycons—such as `(->)`,
`Int#`, and others—which cannot have standalone-derived instances
(via the `anyclass` strategy) as a result of this check! See
https://ghc.haskell.org/trac/ghc/ticket/13154#comment:8 for an
example of where this overly conservative restriction bites.

Really, this validity check only makes sense in the context of
`stock` deriving, where we need the property that the tycon is that
of a normal ADT or a data family in order to inspect its data
constructors. Other deriving strategies don't require this validity
check, so the most sensible way to fix this error is to move the
logic of this check into `cond_stdOK`, which is specific to
`stock` deriving.

This makes progress towards fixing (but does not entirely fix)

Test Plan: make test TEST=T13154a

Reviewers: bgamari

Reviewed By: bgamari

Subscribers: rwbarton, thomie, carter

GHC Trac Issues: #13154

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

comment:15 Changed 20 months ago by RyanGlScott

Differential Rev(s): Phab:D4337Phab:D4337, Phab:D4370

Phab:D4370 is another slight refactor that's working towards fixing this ticket.

comment:16 Changed 19 months ago by Ryan Scott <ryan.gl.scott@…>

In f4336593/ghc:

Slight refactor of stock deriving internals

Summary:
Before, the `hasStockDeriving` function, which determines
how derived bindings should be generated for stock classes, was
awkwardly separated from the `checkSideConditions` function, which
checks invariants of the same classes that `hasStockDeriving` does.
As a result, there was a fair deal of hoopla needed to actually use
`hasStockDeriving`.

But this hoopla really isn't required—we should be using
`hasStockDeriving` from within `checkSideConditions`, since they're
looking up information about the same classes! By doing this, we can
eliminate some kludgy code in the form of `mk_eqn_stock'`, which had
an unreachable `pprPanic` that was stinking up the place.

Reviewers: bgamari, dfeuer

Reviewed By: bgamari

Subscribers: dfeuer, rwbarton, thomie, carter

GHC Trac Issues: #13154

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

comment:17 Changed 15 months ago by bgamari

Owner: RyanGlScott deleted
Status: patchnew

Is this now fixed, RyanGlScott?

comment:18 Changed 15 months ago by RyanGlScott

No. There is a smattering of things that still aren't supported, which include (but may not be limited to):

  • Instances for bare type variables:
instance Foo a -- Works
deriving anyclass instance Foo a -- Doesn't work
  • Instances for type-level literals:
instance Bar 1 -- Works
deriving anyclass instance Bar 2 -- Doesn't work
  • Empty instances:
instance Baz -- Works
deriving instance Baz -- Doesn't work

I have an idea of how to fix these, but haven't had the time to try it out yet.

comment:19 Changed 15 months ago by bgamari

This will not be addressed in GHC 8.6.

comment:20 Changed 15 months ago by bgamari

Milestone: 8.6.18.8.1

These will not be addressed in GHC 8.6.

comment:21 Changed 9 months ago by osa1

Milestone: 8.8.18.10.1

Bumping milestones of low-priority tickets.

Note: See TracTickets for help on using tickets.