Add NFData instances to Closure #4
Conversation
|
What is this needed for? Also, the |
| @@ -348,6 +349,8 @@ instance Typeable a => Binary (Closure a) where | |||
| put (Closure static env) = put static >> put env | |||
| get = Closure <$> get <*> get | |||
|
|
|||
| instance NFData (Closure a) where rnf x = x `seq` () | |||
There was a problem hiding this comment.
I was expecting something like:
instance NFData (Closure a) where rnf (Closure st b) = rnf st `seq` rnf b
|
This instance is required in order to write correct |
|
@facundominguez can you recheck this Pull request? |
|
According to travis it doesn't work for 7.4 because ByteString.Lazy do not have |
| @@ -257,10 +258,17 @@ data StaticLabel = | |||
| | StaticApply StaticLabel StaticLabel | |||
| deriving (Eq, Ord, Typeable, Show) | |||
|
|
|||
| instance NFData StaticLabel where | |||
| rnf (StaticLabel s) = rnf s `seq` () | |||
| rnf (StaticApply a b) = rnf a `seq` rnf b `seq` () | |||
There was a problem hiding this comment.
- rnf (StaticApply a b) = rnf a `seq` rnf b `seq` ()
+ rnf (StaticApply a b) = rnf a `seq` rnf b
I'd prefer this. |
|
@facundominguez comments were addressed and now PR pass tests. |
|
Merged as feef72e. |
No description provided.