-
Notifications
You must be signed in to change notification settings - Fork 65
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 ((->) r')
as a base monad for SelectT
.
#140
base: master
Are you sure you want to change the base?
Conversation
I just noticed, that the same sort of thing will work for Edit: I don't understand why the |
Not unless you want to overlap on the monoid instance. Every Monad may admit any number of accumulating monoids, so the general instance is not supplied. This is a case where we ask not "can we?" but "should we?". |
This case might actually be okay tho. @kozross thoughts? |
I'd be OK with a |
I initially dumped all my thoughts into this PR, but let's make sure we are talking about the same things.
This discussion should move to #141.
Referring to: instance MonadSelect r (SelectT r ((->) r')) where
select f = SelectT $ \k r' -> f $ \a -> k a r'
Also referring to: instance MonadSelect r (SelectT r ((->) r')) where
select f = SelectT $ \k r' -> f $ \a -> k a r' On another note, I'll fix the CI quickly. I was programming blind without compiling offline 🦺 |
`Identity` is the obvious choice, but we can pass an argument below `SelectT`.
@kozross What is the origin of the |
@Lysxia I agree, it does look odd. I played around with it for a bit just now and tried to generalize that instance. This is the best I could come up with. {-# OPTIONS_GHC -fno-warn-orphans #-}
module Control.Monad.Select.OrphanInstances where
import Control.Monad.Select
import Control.Monad.Turn
import qualified Control.Monad.Trans.Select as T
instance MonadTurn m => MonadSelect r (T.SelectT r m) where
select f = T.SelectT $ \ k -> returnWith $ \turn -> f $ turn . k
Here is the other boilerplate code. It depends on monad-control-identity to match If you were to use {-# LANGUAGE UndecidableInstances #-}
module Control.Monad.Turn where
import Control.Monad.Trans.Control.Identity
import Data.Functor.Identity
class Monad m => BaseMonadTurn m where
returnBaseWith :: ((forall x. m x -> x) -> a) -> m a
instance BaseMonadTurn Identity where
returnBaseWith f = Identity $ f runIdentity
instance BaseMonadTurn ((->) r) where
returnBaseWith f r = f ($ r)
class Monad m => MonadTurn m where
returnWith :: ((forall x. m x -> x) -> a) -> m a
-- TODO: Remove `Monad m` constraint. Found a GHC bug. :/
instance (BaseMonadTurn b, MonadBaseControlIdentity b m, Monad m) => MonadTurn m where
returnWith f =
liftBaseWithIdentity $ \ runInBase ->
returnBaseWith $ \ turn ->
f $ turn . runInBase
instance (MonadTurn m, MonadTransControlIdentity t) => MonadTurn (t m) where
returnWith f =
liftWithIdentity $ \ runT ->
returnWith $ \ turn ->
f $ turn . runT If you want to try it out: Overall I'm not sure if I would even call this a monad transformer, since it only works for a subclass of monads. |
I think we agree that something odd is going on with that class, but "not a monad transformer" isn't quite the right way to put it. Strictly speaking, a monad transformer is just something that implements It's difficult to pinpoint the root of the issue because there is no established criterion for what makes a class suitable for inclusion in mtl. Nevertheless, I think the problem is more that It would be helpful to know of some uses for |
Seems like you are right. In the example they even use The Maybe we need: select :: ((a -> m r) -> m a) -> m a Or something similar. I guess I have to gain a better understanding of the paper. |
I don't mind either getting rid of it, or reworking it to better suit its intended purpose, but it'd have to be by someone who understands it better than I do. |
Practically, it may be wiser to just not touch At the moment, |
Identity
is the obvious choice, but we can pass arguments belowSelectT
.We should be able to even use arbitrary
ReaderT
stacks belowSelectT
.I'm not sure if there is a good way to generalize these instances.
I tried to use
MonadBaseControlIdentity
from https://hackage.haskell.org/package/monad-control-identity-0.2.0.0/docs/Control-Monad-Trans-Control-Identity.html#t:MonadBaseControlIdentity, but this leads to duplicate instances.And the only way to resolve these, was by adding newtype wrappers (atleast I think so).