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

withRunInIO as the foundation of the library #13

Closed
effectfully opened this issue Jan 2, 2018 · 3 comments
Closed

withRunInIO as the foundation of the library #13

effectfully opened this issue Jan 2, 2018 · 3 comments

Comments

@effectfully
Copy link
Collaborator

After my recent pull request gets merged, there will be no uses of withUnliftIO in the codebase. Instead it will be

withRunInIO :: MonadUnliftIO m => ((forall a. m a -> IO a) -> IO b) -> m b

everywhere. I think, the library should be build around this function instead of askUnliftIO. Consider a type class with a very similar shape:

class MonadCatch m => MonadMask m where
  mask :: ((forall a. m a -> m a) -> m b) -> m b
  -- `uninterruptibleMask` also

It's not something like

class MonadCatch m => MonadMask m where
  askMask :: m (Mask m)

This would be less readable and more clunky.

So I think it should be

class MonadIO m => MonadUnliftIO m where
  withRunInIO :: ((forall a. m a -> IO a) -> IO b) -> m b

Regarding laws for withRunInIO, how about this one:

forall m a. MonadUnliftIO m => pi (x :: m a) -> withRunInIO ($ x) ~ x

?

Having both withRunInIO and askUnliftIO in the type class along with a MINIMAL pragma is an option, but to me it looks like it complicates the library. It is more convenient to define withRunInIO than askUnliftIO, because there are no newtypes involved:

instance MonadUnliftIO m => MonadUnliftIO (ReaderT r m) where
  withRunInIO k =
    ReaderT $ \r -> withRunInIO $ \run -> k $ run . flip runReaderT r

I also like that return is not used in the definition of withRunInIO. This makes it possible to generalize MonadUnliftIO to other things, maybe some functors. Just in a case.

All in all, I think, withRunInIO is the way to go.

@snoyberg
Copy link
Member

snoyberg commented Jan 2, 2018

I like it. I'd like to do this in two phases, first with a non-breaking change moving this into the typeclass, and second with a breaking change. That will make it easy to write code that works with both the old and the new version. I'm going to open up a PR with that first bit.

snoyberg added a commit that referenced this issue Jan 2, 2018
@snoyberg
Copy link
Member

I'm going to skip the breaking change part and consider this done, thanks again!

@effectfully
Copy link
Collaborator Author

No problem, thanks for the great library!

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