Opened 6 years ago

Closed 17 months ago

Last modified 15 months ago

#8740 closed bug (fixed)

Deriving instance conditionally compiles

Reported by: thomaseding Owned by:
Priority: normal Milestone: 8.6.1
Component: Compiler (Type checker) Version: 7.6.3
Keywords: GADTs, deriving Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC rejects valid program Test Case: deriving/should_compile/T8740
Blocked By: Blocking:
Related Tickets: #8128, #11066 Differential Rev(s):
Wiki Page:

Description

{-# LANGUAGE GADTs #-}
{-# LANGUAGE StandaloneDeriving #-}

data Abstract
data Reified
data Player

data Elect p a where
    ElectRefAsTypeOf :: Int -> Elect Abstract a -> Elect Abstract a
    ElectHandle :: a -> Elect Reified a
    Controller :: Elect Abstract Player
    Owner :: Elect Abstract Player
    You :: Elect Abstract Player

deriving instance (Eq a) => Eq (Elect p a)
deriving instance (Ord a) => Ord (Elect p a)

As is, the above code fails to compile. But if I move ElectRefAsTypeOf to be the last constructor for the GADT, the code does compile. If I remove one of the Elect Abstract Player constructors, the code still won't compile even if the ElectRefAsTypeOf is moved.

Change History (9)

comment:1 Changed 6 years ago by thomaseding

Summary: Code conditionally compilesDeriving instance conditionally compiles

comment:2 Changed 5 years ago by thomie

Component: CompilerCompiler (Type checker)
Operating System: MacOS XUnknown/Multiple

comment:3 Changed 5 years ago by simonpj

I grant that this is extremely confusing. Here's a more concrete example:

{-# LANGUAGE MagicHash, GADTs #-}

module Foo1 where
import GHC.Types
import GHC.Prim

data Abstract
data Reified
data Player

data Elect p a where
     ElectHandle :: a -> Elect Reified a
     Controller :: Elect Abstract Player
     Owner :: Elect Abstract Player
     You :: Elect Abstract Player
     ElectRefAsTypeOf :: Int -> Elect Abstract a

con2TagE :: Elect p a -> Int
con2TagE (ElectHandle {}) = 1
con2TagE Controller = 2
con2TagE Owner = 3
con2TagE You = 4
con2TagE (ElectRefAsTypeOf {}) = 5

cmp :: Ord a => Elect p a -> Elect p a -> Ordering

-- Works; ElectRefAsTypeOf is the LAST constructor

cmp (ElectHandle a1) (ElectHandle b1) = a1 `compare` b1
cmp (ElectHandle a1) _                = LT

cmp (ElectRefAsTypeOf a1) (ElectRefAsTypeOf b1) = compare a1 b1
cmp (ElectRefAsTypeOf a1) _                     = GT

cmp a b = con2TagE a `compare` con2TagE b


{-
-- Fails; ElectRefAsTypeOf is the FIRST constructor
cmp1 (ElectRefAsTypeOf a1) (ElectRefAsTypeOf b1) = compare a1 b1
cmp1 (ElectRefAsTypeOf a1) _                     = LT

cmp1 (ElectHandle a) (ElectRefAsTypeOf b) = GT   -- This line fails
cmp1 (ElectHandle a) (ElectHandle b)      = a `compare` b

cmp1 a b = con2TagE a `compare` con2TagE b
-}

Here

  • The code for cmp is generated by deriving(Ord) when ElectRefAsTypeOf is the LAST constructor; i.e. as declared above
  • the code for cmp is generated by deriving(Ord) when ElectRefAsTypeOf is the FIRST constructor; i.e. in the data decl, move that constructor from last to first.

The code for each is not unreasonable.

If you compile these two definitions you'll see that cmp succeeds, but cmp1 fails, because the -- This line fails case is unreachable.

This is all very tiresome and confusing. The main possibility that occurs to me would be to switch off the "inaccessible code" error in derived code. We already do that for other errors. I'm sure it'd be do-able but it's not entirely straightforward because of course there are other type errors we'd like to catch.

Anyway I thought I would at least document what is going on here.

Simon

Version 0, edited 5 years ago by simonpj (next)

comment:4 Changed 2 years ago by RyanGlScott

Keywords: GADTs deriving added

comment:5 Changed 21 months ago by bgamari

Indeed I was bitten by this just now. This is quite an unfortunate behavior.

comment:6 Changed 17 months ago by RyanGlScott

Now that inaccessible code is a warning instead of an error (see #11066), the original program now typechecks. I'll whip up a test case.

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

In 90e99c4c/ghc:

Add tests for #8128 and #8740

Commit 08073e16cf672d8009309e4e55d4566af1ecaff4 (#11066) ended up
fixing these, fortunately enough.

comment:8 Changed 17 months ago by RyanGlScott

Milestone: 8.6.1
Resolution: fixed
Status: newclosed
Test Case: deriving/should_compile/T8740

comment:9 Changed 15 months ago by Krzysztof Gogolewski <krz.gogolewski@…>

In 44a7b9b/ghc:

Suppress -Winaccessible-code in derived code

Summary:
It's rather unfortunate that derived code can produce inaccessible
code warnings (as demonstrated in #8128, #8740, and #15398), since
the programmer has no control over the generated code. This patch
aims to suppress `-Winaccessible-code` in all derived code. It
accomplishes this by doing the following:

* Generalize the `ic_env :: TcLclEnv` field of `Implication` to
  be of type `Env TcGblEnc TcLclEnv` instead. This way, it also
  captures `DynFlags`, which record the flag state at the time
  the `Implication` was created.
* When typechecking derived code, turn off `-Winaccessible-code`.
  This way, any insoluble given `Implication`s that are created when
  typechecking this derived code will remember that
  `-Winaccessible-code` was disabled.
* During error reporting, consult the `DynFlags` of an
  `Implication` before making the decision to report an inaccessible
  code warning.

Test Plan: make test TEST="T8128 T8740 T15398"

Reviewers: simonpj, bgamari

Reviewed By: simonpj

Subscribers: monoidal, rwbarton, thomie, carter

GHC Trac Issues: #8128, #8740, #15398

Differential Revision: https://phabricator.haskell.org/D4993
Note: See TracTickets for help on using tickets.