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.

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 deeply strict liftM #13

Closed
treeowl opened this issue Jan 30, 2016 · 5 comments
Closed

Add deeply strict liftM #13

treeowl opened this issue Jan 30, 2016 · 5 comments

Comments

@treeowl
Copy link
Contributor

treeowl commented Jan 30, 2016

Control.Monad offers <$!>. By analogy, I believe this package should offer

(<$!!>) :: (Monad m, NFData b) => (a -> b) -> m a -> m b
f <$!!> m = m >>= \x -> return $!! f x
@RyanGlScott
Copy link
Member

Sanity-check: is this quite analogous to (<$!>) from base? Its definition is:

(<$!>) :: Monad m => (a -> b) -> m a -> m b
f <$!> m = do
  x <- m
  let z = f x
  z `seq` return z

Whereas with (<$!!>), we have something that's closer to:

(<$!!>) :: (Monad m, NFData b) => (a -> b) -> m a -> m b
f <$!!> m = do
  x <- m
  f x `deepseq` return (f x)

I'd hope that GHC is smart enough to apply CSE on the two occurrences of f x, but maybe it'd be wise to use explicit sharing here.

@treeowl
Copy link
Contributor Author

treeowl commented Aug 15, 2016

I don't understand how it could translate the way you say. Doesn't $!! take care of the sharing here? If not, that's broken.

@treeowl
Copy link
Contributor Author

treeowl commented Aug 15, 2016

Actually, $!! shouldn't have to worry about it either. Unless GHC actually copies the expression for no reason, we should be fine.

@RyanGlScott
Copy link
Member

Ah, you're right, I inlined one too many definitions in my head. The fact that you're using ($!!) here explicitly takes care of that issue. Carry on :)

@treeowl
Copy link
Contributor Author

treeowl commented Aug 15, 2016

The bigger problem is that <$!> is weird. It should really be called <!$>. <$!> should force the argument; <!$!> should force both. For deepseq, I think only <!!$> and <$!!> are really useful. But it's too late to fix this, so we should follow base for consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants