Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rnfFoldableLTR and rnfFoldableRTL #18

Closed
wants to merge 1 commit into from
Closed

Add rnfFoldableLTR and rnfFoldableRTL #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 96 additions & 2 deletions Control/DeepSeq.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,24 @@
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE TypeOperators #-}
# if MIN_VERSION_array(0,4,0)
#if __GLASGOW_HASKELL__ >= 708
-- We can't use Safe because Data.Coerce is not marked safe.
{-# LANGUAGE Trustworthy #-}
#else
{-# LANGUAGE Safe #-}
#endif
# endif
#endif
#if __GLASGOW_HASKELL__ >= 706
{-# LANGUAGE PolyKinds #-}
#endif
#if __GLASGOW_HASKELL__ >= 708
{-# LANGUAGE ScopedTypeVariables #-}
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to enable ScopedTypeVariables unconditionally (without an #ifdef), since all versions of GHC that we test against support ScopedTypeVariables anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine.

-----------------------------------------------------------------------------
-- |
-- Module : Control.DeepSeq
-- Copyright : (c) The University of Glasgow 2001-2009
-- Copyright : (c) The University of Glasgow 2001-2016
-- License : BSD-style (see the file LICENSE)
--
-- Maintainer : [email protected]
Expand Down Expand Up @@ -57,7 +65,7 @@
-- @since 1.1.0.0
module Control.DeepSeq (
deepseq, ($!!), force,
NFData(..),
NFData(..), rnfFoldableLTR, rnfFoldableRTL
) where

import Control.Applicative
Expand All @@ -72,6 +80,7 @@ import Data.Array
import Data.Fixed
import Data.Version
import Data.Monoid as Mon
import Data.Foldable (Foldable (foldMap))
import Data.Unique ( Unique )
import Foreign.Ptr
import Foreign.C.Types
Expand Down Expand Up @@ -105,6 +114,10 @@ import GHC.Stack ( CallStack(..) )
import GHC.SrcLoc ( SrcLoc(..) )
#endif

#if __GLASGOW_HASKELL__ >= 708
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The #if __GLASGOW_HASKELL__ >= 708 checks should be changed to #if MIN_VERSION_base(4,7,0), since it's just guarding the use of Data.Coerce.coerce from base.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree. The whole Coercible apparatus is very much GHC-only. I don't think pretending this is about base does anyone any favors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it's not something that personally bothers me that much, but I know @hvr has been wanting to be able to decouple base installations from GHC versions for a while, so I don't know if tying an conditional import from base to a GHC version will make things shaky in the future...

Copy link
Member

@hvr hvr Aug 15, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, in general I dislike __GLASGOW_HASKELL__ being abused as a proxy for MIN_VERSION_base.

In this case, however, it's not so clear cut IMHO, as Coercible is both a base-feature as well as depending on facilities provided only by certain GHC versions (although I'm not sure if you couldn't actually fake/emulate Data.Coerce.coerce via unsafeCoerce... in which case I would see this more as a case for MIN_VERSION_base)

PS: Also, base is still terribly GHC-specific. I hope that at some point we can make some progress on the base-split idea, and at the very least split base into a reasonably not-so-GHC-specific portable part, and the rest...

import Data.Coerce
#endif

#if __GLASGOW_HASKELL__ >= 702
import GHC.Fingerprint.Type ( Fingerprint(..) )
import GHC.Generics
Expand Down Expand Up @@ -202,6 +215,87 @@ f $!! x = x `deepseq` f x
force :: (NFData a) => a -> a
force x = x `deepseq` x

newtype UnitLTR = UnitLTR {getUnitLTR :: ()}

#if MIN_VERSION_base(4,9,0)
instance Semi.Semigroup UnitLTR where
UnitLTR () <> y = y
#endif

instance Monoid UnitLTR where
mempty = UnitLTR ()
UnitLTR () `mappend` y = y
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid invoking the wrath of -Wnoncanonical-monoid-instances in later GHCs, it'd be prudent to define mappend like this:

instance Monoid UnitLTR where
  mempty = UnitLTR ()
#if MIN_VERSION_base(4,9,0)
  mappend = (<>)
#else
  UnitLTR () `mappend` y = y
#endif

And similarly for UnitRTL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.


-- It would be nice to use Data.Monoid.Dual instead of
-- another newtype, but that doesn't seem to inline
-- very well if it doesn't specialize.
newtype UnitRTL = UnitRTL {getUnitRTL :: ()}

#if MIN_VERSION_base(4,9,0)
instance Semi.Semigroup UnitRTL where
x <> UnitRTL () = x
#endif

instance Monoid UnitRTL where
mempty = UnitRTL ()
x `mappend` UnitRTL () = x

-- | A definition of `rnf` for `Foldable` types that evaluates them
-- from left to right.
--
-- @
-- data ConsList a = Cons a (ConsList a) | Nil deriving (Foldable)
-- instance NFData a => NFData (ConsList a) where
-- rnf = rnfFoldableLTR
-- @
--
-- Caution: this only works properly if the `Foldable` instance
-- folds over all relevant structure.
--
-- @
-- rnfFoldableLTR (Left undefined) = ()
-- rnfFoldableLTR (undefined, ()) = ()
-- @
#if __GLASGOW_HASKELL__ >= 708
rnfFoldableLTR :: forall f a . (Foldable f, NFData a) => f a -> ()
rnfFoldableLTR = getUnitLTR . foldMap (coerce (rnf :: a -> ()))
#else
rnfFoldableLTR :: (Foldable f, NFData a) => f a -> ()
rnfFoldableLTR = getUnitLTR . foldMap (UnitLTR . rnf)
#endif

-- | A definition of `rnf` for `Foldable` types that evaluates them
-- from right to left.
--
-- @
-- data SnocList a = Snoc (SnocList a) a | SNil deriving (Foldable)
-- instance NFData a => NFData (SnocList a) where
-- rnf = rnfFoldableRTL
-- @
--
-- Caution: this only works properly if the `Foldable` instance
-- folds over all relevant structure.
--
-- @
-- rnfFoldableRTL (Left undefined) = ()
-- rnfFoldableRTL (undefined, ()) = ()
-- @
#if __GLASGOW_HASKELL__ >= 708
rnfFoldableRTL :: forall f a . (Foldable f, NFData a) => f a -> ()
rnfFoldableRTL = getUnitRTL . foldMap (coerce (rnf :: a -> ()))
#else
rnfFoldableRTL :: (Foldable f, NFData a) => f a -> ()
rnfFoldableRTL = getUnitRTL . foldMap (UnitRTL . rnf)
#endif

#ifdef __GLASGOW_HASKELL__
{-# INLINABLE rnfFoldableLTR #-}
{-# INLINABLE rnfFoldableRTL #-}
#else
{-# INLINE rnfFoldableLTR #-}
{-# INLINE rnfFoldableRTL #-}
#endif

-- | A class of types that can be fully evaluated.
--
-- @since 1.1.0.0
Expand Down
4 changes: 4 additions & 0 deletions changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog for [`deepseq` package](http://hackage.haskell.org/package/deepseq)

## ????

* New functions `rnfFoldableLTR` and `rnfFoldableRTL`

## 1.4.2.0 *Apr 2016*

* Bundled with GHC 8.0.1
Expand Down
5 changes: 5 additions & 0 deletions deepseq.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ library
if impl(ghc < 7.6)
build-depends: ghc-prim == 0.2.*

if impl(ghc >= 7.8)
other-extensions:
ScopedTypeVariables
Trustworthy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these here if you enable them in Control.DeepSeq anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not really important. other-extensions is only informative; it doesn't actually enable anything. That's just saying that some modules use those extensions with those versions. I can step them out for sure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I misread the phrase other-extensions.

Hm, the current full list of other-extensions is a bit of a lie since some extensions are only enabled conditionally (e.g., DeriveGeneric), but since it's only informative, that doesn't bother me too much. You could probably just merge these two entries with the others for simplicity's sake.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, I misread again. I was looking at the other-extensions for the test suite, not the library, so my above comment doesn't really make sense.

So it's really only the placement of ScopedTypeVariables in this list that would need to change. Sorry for the confusion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RyanGlScott actually, other-extensions is more than just informal. Cabal's solver (since 1.24) takes that list into account when solving install plans and makes sure that the current compiler supports all other-extensions.


if impl(ghc < 7.4)
build-depends: array < 0.4

Expand Down