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

Simpler constraints #217

Merged
merged 27 commits into from
Sep 25, 2019
Merged

Simpler constraints #217

merged 27 commits into from
Sep 25, 2019

Conversation

robrix
Copy link
Contributor

@robrix robrix commented Sep 23, 2019

This PR:

  • Splits Member into separate Inject and Project classes, retaining the latter solely for interposition. This makes both into single-method classes, whose dictionaries are essentially passed around as newtypes, removing extra indirection around the method. I haven’t noticed any particular speedup from this change, however; send/inj is not a bottleneck as far as I can tell.
  • Defines a Has constraint synonym as the combination of Inject and Carrier, and uses it for all effect smart constructors.
  • Exports Has from all carrier modules, replacing the exports of Carrier and Member.
  • Applies all of the above to the docs.
  • Export Carrier again; it’s useful when handling effects on top of some other effectful context.
  • Fixes Requiring both Carrier & Member constraints is inconvenient #212.

Note that in contrast to the discussion in #212, this does not involve any changes to Carrier (i.e. using a type family instead of a functional dependency to provide the signature). Unfortunately, experimentation in diffused-effects demonstrated that while the type families approach has better ergonomics (no FlexibleInstances or UndecidableInstances extensions required for most carrier instances, and no sig parameter floating around purposelessly in Has constraints), it has unacceptable consequences for compile-times.

Specifically, it triggers such a severe coercion pile-up that having stacks as few as fifteen effects deep become completely unusable. (I left a stack of about 20 carriers going for about fifteen minutes before finally giving up and killing the ghc process.) This is an entirely normal sort of quantity for projects like semantic designed around effects, making it a show-stopper.

We can perhaps revisit that part of the equation later on, once the coercion pile-up bug is fixed (or rather, once it has been fixed for a few releases of ghc).

🎩 @lexi-lambda for the suggestion!

@robrix robrix added this to the 1.0 milestone Sep 23, 2019
tellLoop i = replicateM_ i (tell (Sum (1 :: Int)))

modLoop :: (Carrier sig m, Member (State (Sum Int)) sig) => Int -> m ()
modLoop :: Has (State (Sum Int)) sig m => Int -> 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.

sig is not particularly useful, but I’ll still take this over the previous idiom.

Choosing m1 <> Choosing m2 = Choosing (m1 <|> m2)

instance (Carrier sig m, Member Choose sig, Member Empty sig) => Monoid (Choosing m a) where
instance (Has Choose sig m, Has Empty sig m) => Monoid (Choosing m a) where
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that there’s no warning about redundant constraints coming from the multiple instances of Carrier sig m implied by the expansion of the constraint synonym.


-- | Construct a request for an effect to be interpreted by some handler later on.
send :: Has eff sig m => eff m a -> m a
send = eff . inj
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 moved this back out here because Has definitely doesn’t belong in Control.Effect.Sum.

@@ -45,7 +45,7 @@ newtype Handler eff m = Handler (forall x . eff m x -> m x)
runHandler :: (HFunctor eff, Functor m) => Handler eff m -> eff (ReaderC (Handler eff m) m) a -> m a
runHandler h@(Handler handler) = handler . hmap (runReader h)

instance (HFunctor eff, Carrier sig m, Member eff sig) => Carrier sig (InterposeC eff m) where
instance (HFunctor eff, Carrier sig m, Project eff sig) => Carrier sig (InterposeC eff m) where
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 is basically the only thing in the library using Project.

I want to 🔥 it so badly, but there’s no way @patrickt will let me 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However, see #220.

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.

Spectacular work, @robrix, and thank you @lexi-lambda for the inspiration!


```haskell
action :: (Member (State String) sig, Carrier sig m) => m ()
action :: Has (State String) sig m => m ()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really like this. I wish we could get rid of the sig parameter, but I think I grok, at least intuitively, why we can’t. I like that we have n rather than n+1 constraints for all functions of n effects now.

@@ -34,15 +34,15 @@ isSafeIn (i,j) qs = null (diags (i,j) `intersect` underThreat)
qs' = zip [1..length qs] qs
underThreat = qs' >>= diags

addOne :: (Carrier sig m, Alternative m) => Int -> Board -> m Board
addOne :: (Alternative m, Monad m) => Int -> Board -> m Board
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I forgot that Carrier implies Monad. Still, this is way more general now!

@robrix robrix merged commit ca036d0 into master Sep 25, 2019
@robrix robrix deleted the simpler-constraints branch September 25, 2019 15:29
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.

Requiring both Carrier & Member constraints is inconvenient
2 participants