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

IO's Alternative, MonadPlus, and MonadFail instances leak exceptions #1

Open
shajra opened this issue Nov 12, 2018 · 3 comments
Open

Comments

@shajra
Copy link
Owner

shajra commented Nov 12, 2018

I could remove the MonadFail instance from Checked, but MonadPlus and Alternative are unfortunate to remove. Really, the problem isn't these type classes. The problem is that IO throws exceptions in the instances for these type classes.

So maybe for IO, I should make a newtype. If you use IO directly, then the warning is merely not to call mzero or fail. But if you use the newtype I provide for IO, then you're safer, because I won't expose exception-throwing instances for any type classes.

@shajra shajra changed the title IO's MonadPlus and MonadFail instances leak exceptions IO's MonadPlus and MonadFail instances leak exceptions Nov 12, 2018
@shajra shajra changed the title IO's MonadPlus and MonadFail instances leak exceptions IO's MonadPlus and MonadFail instances leak exceptions Nov 12, 2018
@parsonsmatt
Copy link

Seems like you could write:

instance (Throws IOException e) => Alternative/MonadPlus (Checked e IO) where ...

instance (Throws String? e) => MonadFail (Checked e IO) where ....

That may work to propagate the constraints. These would overlap any instance that delegated transparently to the underlying monad.

@shajra
Copy link
Owner Author

shajra commented Nov 12, 2018

@parsonsmatt

Interesting idea. But it doesn't deal with transformer stacks over IO. But I think we can do something similar to deal with that. I'll find out when I have it all compiling with some testing! Thanks for the idea. I think it may be less hassle for the user than creating a newtype for IO.

@shajra
Copy link
Owner Author

shajra commented Nov 13, 2018

@parsonsmatt

Okay, I played around with things. And your recommendation as written doesn't seem to work, even with overlapping (or even incoherent!) instances, because the targets of the instances are identical. You recommendation works for just IO, but because targets are identical, I get stuck trying to make it work more generically for transformer stacks.

But that said, there might be some advanced overlapping techniques to get around that involving functional dependencies (like the first option here).

Still, I'm apprehensive about this approach in light of transformer stacks. EitherT's Alternative instance doesn't delegate to the Alternative instance of its base, but ReaderT's does. Doing all the bookkeeping for this feels like a mess, breaking the encapsulation of just trusting instances to compose with one another. I'm apprehensive of adding complexity unless it solves the problem comprehensively, even for transformers.

At the least, I'll put in a warning in the docs being really clear about the MonadFail, MonadPlus, and Alternative instances. I think my newtype idea for IO gives some safety without too much complexity, though I'm worried I don't have construction really figured out with respect to MonadIO or MonadBase.

I'm interested in your thoughts on all this. We didn't talk about it yet, but I don't think dropping the instances of Alternative, MonadFail, and MonadPlus is the right thing either. The reality is that anyone can make instances of type classes that throw exceptions -- even the sacrosanct other ones. At the end of the day, I think we mostly don't because it's in poor taste. IO is more an exception (PUN!).

@shajra shajra changed the title IO's MonadPlus and MonadFail instances leak exceptions IO's Alternative, MonadPlus, and MonadFail instances leak exceptions Nov 13, 2018
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