Closed turion closed 2 years ago
This looks good, but it needs a changelog entry
Weird, CI is not triggered...
Weird, CI is not triggered...
I think our travis credits ran out. I am preparing a PR to switch to GitHub actions.
Can you rebase on top of master to get CI changes?
Can you rebase on top of master to get CI changes?
Done :) I think it's waiting for you to trigger the CI.
I'm expecting some older versions to fail, but I think it's easier to test that on CI than to install all the old versions of GHC.
I have triggered CI.
Getting the CPP right is really painful. Is there a good trick to run the whole CI locally somehow? Possibly using nix?
Pinging @emilypi @gwils @ekmett for review.
Does MonadAccum
have laws? If so, I would like to see them documented. Do we know whether these instances obey them?
I skimmed the paper linked in Control.Monad.Accum
, and it looks it does not mention acccumulation monads. Why is it linked?
Does MonadAccum have laws? If so, I would like to see them documented.
I thought about some laws, also see this discussion if you're interested: https://github.com/fused-effects/fused-effects/pull/391#discussion_r642113261
It's not clear to me how to generalise them optimally to mtl
. Possibly we can phrase them by how add
and look
interact:
add
changes the environment for look
lhs == rhs
where
lhs = add w >> look
rhs = do
w' <- look
add w
return $ w' <> w
look
ing twice yields the same(==) <$> look <*> look == return true
add
ing is monoidaladd w >> add w' == add (w <> w')
add mempty == return
Do we know whether these instances obey them?
For AccumT
this is fairly simple to check (if I haven't done a typo in the above).
I skimmed the paper linked in Control.Monad.Accum, and it looks it does not mention acccumulation monads. Why is it linked?
That paper introduces transformers in general, not this specific one which was discovered later.
Thank you for putting this patch together. I don't have any problem with the code, but I'm not really sold on adding MonadAccum
to mtl
. Maybe @chessai can help me.
Where did Accum
come from? Does we know of anyone using it for anything? It looks like an awkward point in the design space. Does this have some helpful special property that I've missed?
I would prefer to use an update monad in many cases instead of this, or otherwise use State
or Writer
directly.
Where did Accum come from?
AccumT
was introduced in transformers
here: https://hub.darcs.net/ross/transformers/issue/24 It was known and used before that.
Does we know of anyone using it for anything?
I've used it myself in a few places in the past, even in production. Some reasons why it's not so common as other transformers might be:
mtl
(and other effect libraries like fused-effects
) yet, so using it is currently a maintability risk.The second one is kind of a self-fulfilling prophecy. mtl
doesn't support Accum
, so noone uses it, so noone tries to add it. In fact I came to create this PR precisely because I used it in my code, and then at some point couldn't work with my transformer stack anymore because AccumT
was poorly supported, both in lacking instances and the MonadAccum
class. (@chessai has improved the situation with https://github.com/haskell/mtl/pull/80 since.)
It looks like an awkward point in the design space. Does this have some helpful special property that I've missed? I would prefer to use an update monad in many cases instead of this, or otherwise use State or Writer directly.
AccumT
sits in between WriterT
and StateT
, and has advantages and disadvantages over both.
Monad | AccumT Advantage |
AccumT Disadvantage |
---|---|---|
WriterT |
Can read the log (as a monadic action) | WriterT is functorial in the log type, AccumT not |
StateT |
Extra guarantee that old logs are not discarded | StateT is more flexible |
Typical use cases for me are counters, or handing out unique keys/ids/tokens.
I wasn't familiar with update monads, so not sure how those relate. To me it seems that type AccumT w m a = UpdateT w w m a
, where the monoid acts on the state by multiplication. So maybe AccumT
is a special case of UpdateT
. Nevertheless I believe that UpdateT
doesn't even exist in any popular library. But AccumT
is part of one of the most popular and used Haskell libraries.
Should mtl
support it? I believe yes. The github readme says:
MTL is a collection of monad classes, extending the transformers package
So if mtl
extends transformers
, it should ideally have a class for every transformer.
Superceded by #109
See #86