Add Data.Void to base (re #9814)
ClosedPublic

Authored by hvr on Nov 20 2014, 2:25 AM.

Details

Summary

This adds the module Data.Void (formerly provided by the void package)
to base.

The original Haskell98 compatible implementation has been modified to use
modern GHC features, and vacuousM was dropped since it's redundant now
with the AMP in place. Instances for classes not part of base had to be
dropped as well.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
hvr updated this revision to Diff 1581.Nov 20 2014, 2:25 AM
hvr retitled this revision from to Add Data.Void to base (re #9814).
hvr updated this object.
hvr edited the test plan for this revision. (Show Details)
hvr added reviewers: ekmett, shachaf.
hvr updated the Trac tickets for this revision.
hvr added a comment.Nov 20 2014, 2:34 AM

Rendered Haddock:

hvr updated this object.Nov 20 2014, 2:35 AM
hvr edited edge metadata.
hvr updated this revision to Diff 1586.Nov 20 2014, 3:48 AM
  • Add /Since: 4.8.0.0-annotations
dfeuer requested changes to this revision.Nov 20 2014, 8:44 AM
dfeuer added a reviewer: dfeuer.
dfeuer added a subscriber: dfeuer.

This mostly looks good, but I think a couple minor changes are in order.

libraries/base/Data/Void.hs
42

What's the reasoning behind this?

68

Why not absurd a = undefined?

75

How about vacuous x = undefined <$ x, in case the Functor defines a special <$?

This revision now requires changes to proceed.Nov 20 2014, 8:44 AM
austin requested changes to this revision.Nov 20 2014, 10:07 AM
austin edited edge metadata.

I think this is fine, modulo the discrepancy @dfeuer pointed out. i don't particularly care about the exact implementations of any of the primitives, though.

libraries/base/Data/Void.hs
42

I also wonder about this. In particular, this does not behave the same way as -XStandaloneDeriving:

$ ghci -XStandaloneDeriving
GHCi, version 7.6.3: http://www.haskell.org/ghc/  :? for help
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Prelude> data D
Prelude> deriving instance Eq D
Prelude> (undefined :: D) == (undefined :: D)
*** Exception: Void ==
Prelude>

Related discussion is in Trac #7401.

hvr added inline comments.Nov 20 2014, 11:54 AM
libraries/base/Data/Void.hs
42

@shachaf may have something to do with that (and I was hoping he'd comment about that here...)

68

Some context, in the original Data.Void it's defined like so

newtype Void = Void Void

absurd :: Void -> a
absurd a = a `seq` spin a where
   spin (Void b) = spin b

@ekmett actually suggested the EmptyCase implementation in order to get "the right bottom".

I guess

absurd a = a `seq` undefined

would have worked as well, but tbh, I consider the EmptyCase variant more elegant...

ekmett accepted this revision.Nov 20 2014, 3:40 PM
ekmett edited edge metadata.

LGTM

libraries/base/Data/Void.hs
42

Matches existing behavior and result is more defined, so you get nicer core when this gets inlined.

Pretty much all these argument strictnesses here were the subject of very long debates years ago and have seen long periods of productive usage. Randomly changing it just to change it would be a bad idea IMHO.

75

The problem is you 'get the wrong bottom'

dfeuer added inline comments.Nov 20 2014, 4:51 PM
libraries/base/Data/Void.hs
42

That's fine, but the behavior needs to be documented in the Haddocks, and the reasoning should be documented, at least in source comments.

75

How is the bottom you get from case a of {} better? That doesn't actually force anything, does it?

hvr added a comment.Nov 20 2014, 5:58 PM

@dfeuer btw, empty-case was motivated by void, see Trac #2431

dfeuer added inline comments.Nov 20 2014, 7:04 PM
libraries/base/Data/Void.hs
75

Ah, I understand now, I think. But since Void is a subtype of all types of kind *, I wonder if we could use vacuous = unsafeCoerce.

austin accepted this revision.Nov 20 2014, 9:23 PM
austin edited edge metadata.

LGTM then. (I also assumed EmptyCase generated nicer core here but didn't bother coming up with a situation for it).

This revision was automatically updated to reflect the committed changes.