-
Notifications
You must be signed in to change notification settings - Fork 52
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 IORef-based State carrier. #422
Conversation
This was copied straight from `fused-effects-exceptions`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good!
Can we add tests? I know it's a bit awkward given the whole IO
thing, but I'd rather not have this untested if possible. It would also pave the way toward testing its interactions with nondeterministic carriers like EmptyC
, ErrorC
, and ChooseC
, which would be good to pin down properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some API doc stuff left now!
src/Control/Carrier/State/IORef.hs
Outdated
|
||
{- | A carrier for the 'State' effect. It uses an 'IORef' internally to handle its state, and thus admits a 'MonadUnliftIO' instance. Because the state operations are performed impurely, this carrier will not lose state effects even with nefarious uses of 'liftWith'. | ||
|
||
Unlike the other carriers for 'State', this carrier's effects will not backtrack when run in conjuction with 'NonDet' effects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👨🏻🍳💋
(Note also that this PR has been correctly retargeted to merge into |
@robrix I think this is ready for another look. I agree we should test this, but propose to do this in a separate PR, as generalizing the existing machinery to either Identity or IO is somewhat fraught, and I agree that the existing tests are those that exercise paths like lifting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful 👍🏻
This was copied straight from
fused-effects-exceptions
.