polysemy-research / polysemy-zoo

:monkey::panda_face: Experimental, user-contributed effects and interpreters for polysemy
BSD 3-Clause "New" or "Revised" License
70 stars 20 forks source link

MTL absorbers are complex when they have superclasses #24

Closed adamConnerSax closed 5 years ago

adamConnerSax commented 5 years ago

This may be simpler than I think but I started to try and write the CanonicalEffect code for my PandocMonad class. PandocMonad has a superclass constraint of MonadError PandocError m that is, somewhere in Pandoc there is class (MonadError PandocError m) => PandocMonad m where ... and that seems to cause some weirdness:

-The ReifiedConstraint1 instance wouldn't compile, complaining about a functional dependency.

I haven't dug in yet so this is sort of preliminary but I wanted to signpost it here so I/we remember to document this and however we solve it, if it warrants explanation.

isovector commented 5 years ago

Maybe reflection can be of some inspiration:

https://github.com/ekmett/reflection/blob/49d1d6ad2355f4cb2061bff0c744feb8841cd6e1/examples/Constraints.hs#L75-L83

adamConnerSax commented 5 years ago

Okay. Figured that out, I think. Harder than the reflection example but that's likely because my brain frizzes a bit every time I see "Sub Dict" and it takes me a while to uncross the wires.

Anyway, solved it by (for the sake of documenting the process):

  1. Adding throwError_ and catchError_ to the Dict1 associated data type
  2. Using them to create the instance above.
  3. Filling the canonical dictionary bit of this with throw and catch from Polysemy.Error.

Which all makes sense.

But now life is full of Oprhan instances! GHC says:

Orphan instance: instance P.ReifiableConstraint1 PA.PandocMonad
    To avoid this
        move the instance declaration to the module of the class or of the type, or
        wrap the type with a newtype and declare the instance on the new type.
    |
195 | instance P.ReifiableConstraint1 PA.PandocMonad where
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...

src/Knit/Effect/PandocMonad.hs:222:1: warning: [-Worphans]
    Orphan instance:
      instance (Monad m, P.Reifies s' (P.Dict1 PA.PandocMonad m)) =>
               MonadError PA.PandocError (P.ConstrainedAction PA.PandocMonad m s')

222 | instance (Monad m
    | ^^^^^^^^^^^^^^^^^...

src/Knit/Effect/PandocMonad.hs:229:1: warning: [-Worphans]
    Orphan instance:
      instance (Monad m,
                MonadError
                  PA.PandocError (P.ConstrainedAction PA.PandocMonad m s'),
                P.Reifies s' (P.Dict1 PA.PandocMonad m)) =>
               PA.PandocMonad (P.ConstrainedAction PA.PandocMonad m s')

229 | instance (Monad m
    | ^^^^^^^^^^^^^^^^^...

src/Knit/Effect/PandocMonad.hs:262:1: warning: [-Worphans]
    Orphan instance:
      instance P.Members '[P.Error PA.PandocError, Pandoc] r =>
               P.IsCanonicalEffect PA.PandocMonad r

Should i re-define all the absorb types locally to avoid this? I could then still get a working absorbPandocMonad which would compose.

GHC is not wrong. I suppose we don't want different people making different "canonical" effects for, e.g., PandocMonad. I could imagine scrapping the whole CanonicalEffect class and just writing each "absorb" function separately. That's just sugar. But the other two?

Hmm. I'll keep thinking...

adamConnerSax commented 5 years ago

Here's a thought, which ties into another thing I've been thinking about.

We write each absorber in its own module, each exporting only the constraint-monomorphic abosrber. And we name/organize them like

It's a small amount of code replication, though we could use TH or something to write the boilerplate, which would genuinely be identical each time. Since we don't export the classes, no orphan instances! And it solves another problem which is that the module organization for anything not in that MTL module was going to be inconsistent with it. But now they could all be consistent. But it would move a bunch of code around now. And make it slightly harder to write new ones.

I'm working on it a little. Trying to make non-typeclass-driven using and absorb. Then each absorber module can make a wrapper, a dictionary and a reified instance, none of which are exported, re-use the zoo's harsh and magical using and export a constraint-monomorphic absorber. Might be a reasonable compromise.

Is it worth my making a PR so you can see what I have in mind? If we're going to re-organize modules, sooner is better, right?

Or maybe you have another idea for the orphans?

isovector commented 5 years ago

Sounds reasonable! I'd love to see a PR!

adamConnerSax commented 5 years ago

Closing since I found my way through. But we should keep an eye out for non-trivial examples of absorbers we can write as examples.