Fix GNFData instance for V1

[treeowl]

Currently, we have

instance GNFData V1 where
  grnf = error "Control.DeepSeq.rnf: uninhabited type"

For old GHC, we should have

    grnf x = x `pseq` error "Control.DeepSeq.rnf: uninhabited type"

For newer GHC, we should have

    grnf x = case x of {}

[RyanGlScott]

I agree, but is there any reason you chose pseq? Wouldn't seq suffice (especially since pseq is only available from base if you use a GHC-specific internal module)?

[treeowl]

I thought I remembered some trouble getting the right error message, but I
could be confused.

On Jul 17, 2016 9:29 AM, "Ryan Scott" notifications@github.com wrote:

I agree, but is there any reason you chose pseq? Wouldn't seq suffice
(especially since pseq is only available from base if you use a GHC-specific
internal module
http://hackage.haskell.org/package/base-4.9.0.0/docs/GHC-Conc.html#v:pseq
)?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#19 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABzi_faSLj0hVe92x8bGLDC-O68FHQvbks5qWi45gaJpZM4JOI2C
.

[treeowl]

Indeed, I was confused. The problem I remembered with seq showed up with the old definition of Void as

newtype Void = Void Void

where the error behavior of void under 7.6 is extremely sensitive to details of its definition. In any case, the current definition (which I initially copied incorrectly—I've now edited the description to reflect reality) is really quite awful.

[rwbarton]

Why can't it just be grnf x = x seq ()?

[treeowl]

@rwbarton, it can be, but I don't think that's nearly as clear, since the () is unreachable. Hypothetically, that could even trigger a dead code warning in the future, I would think.

[hvr]

@RyanGlScott @treeowl just double-checking, can you think of any (reasonable) program that could break or change its semantics due to this change when upgrading from deepseq-1.4.2 to the unreleased deepseq-1.4.3?

[RyanGlScott]

As far as a change in semantics goes, it depends on whether you're using a very recent GHC or not.

If you're using any GHC up to 8.2, then I don't think you'd notice much of a difference at all. The reason being is that the old GNFData V1 instance always threw error "Control.DeepSeq.rnf: uninhabited type", regardless of whether the V1 value was actually nonterminating or something which threw an exception itself. But even after this most recent deepseq change, if you're using GHC 8.2 or earlier, you'll still always get an error in such a scenario. Why? It's because DeriveGeneric always derives this implementation of from for empty data types:

instance Generic Empty where
  from x = M1 $ case x of
                  _ -> error "No generic representation for empty datatype Empty"
  ...

So you'll still get an exception, just with a different error message.

In GHC 8.4/HEAD, however, DeriveGeneric's behavior with respect to empty datatypes has changes. Now this would be the derived implementation of from:

instance Generic Empty where
  from x = M1 $ case x of {}
  ...

So in GHC 8.4, you can actually observe the difference between, say, let x = x in x :: V1 () and error "Boom" :: V1 (). Here is a program that witnesses this:

{-# LANGUAGE DeriveGeneric #-}
{-# LANGUAGE ScopedTypeVariables #-}
module Foo where

import Control.DeepSeq
import Control.Exception
import GHC.Generics

data Empty deriving Generic
instance NFData Empty

empty :: Empty
empty = let x = x in x

main :: IO ()
main = evaluate (rnf empty) `catch` \(_ :: SomeException) -> putStrLn "Caught error"

With GHC 8.4 and deepseq-1.4.2.0, this will print Caught error. With GHC 8.4 and deepseq-1.4.3.0, this will loop forever.