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

Base instances #206

Merged
merged 18 commits into from
Oct 6, 2019
Merged

Base instances #206

merged 18 commits into from
Oct 6, 2019

Conversation

robrix
Copy link
Contributor

@robrix robrix commented Sep 2, 2019

@robrix robrix changed the base branch from separate-carrier-and-effect-modules to master September 23, 2019 00:32
@robrix robrix changed the base branch from master to to-the-left-to-the-left September 28, 2019 20:12
@robrix robrix mentioned this pull request Sep 28, 2019
5 tasks
@robrix robrix changed the base branch from to-the-left-to-the-left to master September 29, 2019 16:10
Copy link
Contributor Author

@robrix robrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready for (early) review, but not yet ready to merge.

In addition to the discussion below, I’d like to better answer the question of which instances should we provide for types in base?

@@ -0,0 +1,19 @@
{-# LANGUAGE ConstraintKinds #-}
module Control.Carrier
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need .hs-boot files for Control.Carrier and Control.Carrier.Class to break the cycles in the module graph.

import Control.Effect.NonDet (NonDet)
import Control.Effect.Reader (Reader(..))
import Control.Effect.Sum ((:+:)(..))
import Control.Effect.Writer (Writer(..))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The payoff: we can import all of these effect modules right where we define the Carrier class.



instance Carrier Empty Maybe where
eff Empty = Nothing
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And that in turn allows us to implement Carrier instances for these effects without them being orphaned.


instance Carrier (Error e) (Either e) where
eff (Throw e) = Left e
eff (Catch m h k) = either (k <=< h) k m
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I called this branch base-instances both because they’re instances for types exported from basePrelude, even!—and because, like PureC and LiftC, they’re usable as the base of an effectful context. The signature does not mention :+:, since they’re not monad transformers.

Copy link
Collaborator

@patrickt patrickt Oct 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking my understanding: I could have an effect stack State s :+: Error e and discharge it into an Either s a with just a runState?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, exactly.


instance Carrier (Reader r) ((->) r) where
eff (Ask k) r = k r r
eff (Local f m k) r = k (m (f r)) r
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows you to use these instances anywhere expecting the type in question:

id' :: a -> a
id' = ask

Well, almost anywhere. I can’t figure out how to make this instance available to e.g. Control.Effect.Reader, and I suspect it’s due to some problem with the .hs-boot files. But this function compiles in this module, at least.

( Carrier(..)
) where

import Control.Effect.Class
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an effort to fix the aforementioned problem where Carrier (Reader r) ((->) r) isn’t available in e.g. Control.Effect.Reader, I attempted to duplicate the imports and instances here in the .hs-boot file, but it was not successful.

@@ -11,7 +11,7 @@ module Control.Effect.Choose
, Choosing(..)
) where

import Control.Carrier
import {-# SOURCE #-} Control.Carrier
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These directives indicate that it’s Control.Carrier that gets a .hs-boot file, and not, say, this module.

@robrix robrix marked this pull request as ready for review September 29, 2019 21:12
Copy link
Collaborator

@patrickt patrickt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though an example of how to use these would be nice (good for the documentation, too!)


instance Carrier (Error e) (Either e) where
eff (Throw e) = Left e
eff (Catch m h k) = either (k <=< h) k m
Copy link
Collaborator

@patrickt patrickt Oct 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking my understanding: I could have an effect stack State s :+: Error e and discharge it into an Either s a with just a runState?

@patrickt patrickt merged commit 86cd3c4 into master Oct 6, 2019
patrickt added a commit that referenced this pull request Oct 6, 2019
In keeping with #206, this offers a solo `Carrier` instance for
`NonEmpty` lists over the `Choose` effects. Because `NonEmpty` is now
in `base`, this doesn't add new dependencies, and nicely rounds out
the carrier definition of `[]` for `NonDet` and `Maybe` for `Empty`.
@robrix
Copy link
Contributor Author

robrix commented Oct 7, 2019

Oh, I wasn’t quite ready to merge this I don’t think! No worries, we’ll tidy things up in a follow-up PR.

robrix added a commit that referenced this pull request Oct 9, 2019
@robrix robrix deleted the base-instances branch October 9, 2019 04:35
@robrix robrix mentioned this pull request Oct 27, 2019
2 tasks
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

Successfully merging this pull request may close these issues.

Define (terminal) Carrier instances for common monads?
2 participants