Opened 3 years ago

Closed 2 years ago

Last modified 23 months ago

#13117 closed feature request (fixed)

Derived functor instance for void types handles errors badly

Reported by: dfeuer Owned by: dfeuer
Priority: normal Milestone: 8.2.1
Component: Compiler Version: 8.0.1
Keywords: deriving Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: #7401, #10577 Differential Rev(s):
Wiki Page:

Description

If we have

data V a deriving (Functor)

and we evaluate

f <$> (error "Boo!" :: Void)

we should really get a "Boo" error, rather than a useless "Void fmap" one. The offending code is in compiler/typecheck/TcGenFunctor.hs.

Change History (10)

comment:1 Changed 3 years ago by andreas.abel

What should the implementation of fmap be here?

fmap _ x = x `seq` error "Void fmap"

comment:2 Changed 3 years ago by dfeuer

My preference would be

fmap _ x = case x of

(effectively using EmptyCase).

mpickering indicates there was some prior dispute about this, but I don't know where.

comment:3 Changed 3 years ago by RyanGlScott

The discussion is at #7401, which proposed an overhaul of the way derived instances for void types work. Ultimately, a consensus was never reached as to how derived Eq instances should work. See https://ghc.haskell.org/trac/ghc/ticket/7401#comment:46.

If someone would be willing revive that discussion and advocate for a position, I'd be very appreciative, as I'm hesitant to poke that sleeping dragon again :)

comment:4 Changed 3 years ago by RyanGlScott

David has started a new discussion about this proposed change at the Haskell libraries mailing list here: https://mail.haskell.org/pipermail/libraries/2017-January/027590.html

comment:5 Changed 2 years ago by David Feuer <David.Feuer@…>

In 69f070d/ghc:

Deriving for phantom and empty types

Make `Functor`, `Foldable`, and `Traversable` take advantage
of the case where the type parameter is phantom. In this case,

* `fmap _ = coerce`
* `foldMap _ _ = mempty`
* `traverse _ x = pure (coerce x)`

For the sake of consistency and especially simplicity, make other types
with no data constructors behave the same:

* `fmap _ x = case x of`
* `foldMap _ _ = mempty`
* `traverse _ x = pure (case x of)`

Similarly, for `Generic`,

* `to x = case x of`
* `from x = case x of`

Give all derived methods for types without constructors appropriate
arities. For example,

```
    compare _ _ = error ...
```

rather than

```
    compare = error ...
```

Fixes #13117 and #13328

Reviewers: austin, bgamari, RyanGlScott

Reviewed By: RyanGlScott

Subscribers: ekmett, RyanGlScott, rwbarton, thomie

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

comment:6 Changed 2 years ago by dfeuer

There are still several types that have not reached what I'd consider a good final status, but as Ryan indicates, we need more discussion to get that done.

comment:7 Changed 2 years ago by RyanGlScott

Indeed. However, this ticket specifically concerns Functor, right? It might be best to tackle the other classes in #10577.

comment:8 in reply to:  7 Changed 2 years ago by dfeuer

Resolution: fixed
Status: newclosed

Replying to RyanGlScott:

Indeed. However, this ticket specifically concerns Functor, right? It might be best to tackle the other classes in #10577.

Indeed!

comment:9 Changed 2 years ago by RyanGlScott

Keywords: deriving added

comment:10 Changed 23 months ago by Ben Gamari <ben@…>

In 1317ba62/ghc:

Implement the EmptyDataDeriving proposal

This implements the `EmptyDataDeriving` proposal put forth in
https://github.com/ghc-proposals/ghc-proposals/blob/dbf51608/proposals/0006-deriving-empty.rst.
This has two major changes:

* The introduction of an `EmptyDataDeriving` extension, which
  permits directly deriving `Eq`, `Ord`, `Read`, and `Show` instances
  for empty data types.
* An overhaul in the code that is emitted in derived instances for
  empty data types. To see an overview of the changes brought forth,
  refer to the changes to the 8.4.1 release notes.

Test Plan: ./validate

Reviewers: bgamari, dfeuer, austin, hvr, goldfire

Reviewed By: bgamari

Subscribers: rwbarton, thomie

GHC Trac Issues: #7401, #10577, #13117

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