polysemy-research / polysemy-zoo

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

Update to polysemy's new names #41

Closed Avi-D-coder closed 5 years ago

Avi-D-coder commented 5 years ago

This does not compile for me even with polysemy master. See This actually compiles.

3 of 4 makeSem_ produce errors like the following.

polysemy-zoo/src/Polysemy/Shift/Internal.hs:23:1: error:
    • Could not deduce: s ~ a
      from the context: Member (Shift ref a) r
        bound by the type signature for:
                   reset :: forall (ref :: * -> *) a (r :: [(* -> *) -> * -> *]).
                            Member (Shift ref a) r =>
                            Sem r a -> Sem r a
        at src/Polysemy/Shift/Internal.hs:(68,1)-(71,16)
      ‘s’ is a rigid type variable bound by
        an expression type signature:
          forall s. Shift ref s (Sem r) s
        at src/Polysemy/Shift/Internal.hs:23:1-16
      ‘a’ is a rigid type variable bound by
        the type signature for:
          reset :: forall (ref :: * -> *) a (r :: [(* -> *) -> * -> *]).
                   Member (Shift ref a) r =>
                   Sem r a -> Sem r a
        at src/Polysemy/Shift/Internal.hs:(68,1)-(71,16)
      Expected type: Shift ref s (Sem r) s
        Actual type: Shift ref a (Sem r) a
    • In the first argument of ‘Polysemy.Internal.send’, namely
        ‘(Reset x_asQf :: Shift ref s (Sem r) s)’
      In the expression:
        Polysemy.Internal.send (Reset x_asQf :: Shift ref s (Sem r) s)
      In an equation for ‘reset’:
          reset x_asQf
            = Polysemy.Internal.send (Reset x_asQf :: Shift ref s (Sem r) s)
    • Relevant bindings include
        x_asQf :: Sem r a (bound at src/Polysemy/Shift/Internal.hs:23:1)
        reset :: Sem r a -> Sem r a
          (bound at src/Polysemy/Shift/Internal.hs:23:1)
   |
23 | makeSem_ ''Shift

--  While building package polysemy-zoo-0.4.0.1 using:
      /home/user/.stack/setup-exe-cache/x86_64-linux-tinfo6/Cabal-simple_mPHDZzAJ_2.4.0.1_ghc-8.6.5 --builddir=.stack-work/dist/x86_64-linux-tinfo6/Cabal-2.4.0.1 build lib:polysemy-zoo --ghc-options " -fdiagnostics-color=always"

Is the plugin active? Running stack build --ghc-options='-fplugin=Polysemy.Plugin' leads to the same error.

KingoftheHomeless commented 5 years ago

Grr, yeah, I've had issues using makeSem_. It is incredibly fussy about how you name the type variables. I had massaged the type signatures to the point that it all compiled at the time, but perhaps something has changed?

I'll look into it, because I really, really want to keep the uses of makeSem_. Continuation effects are weird enough that the main methods need to be documented.

Avi-D-coder commented 5 years ago

Are names defined in zoo going to be changed to fit the new guidelines?

How do people feel about it?

KingoftheHomeless commented 5 years ago

I'm all for changing interpreter names to fit the current standard. I.e. instead of runAsyncFinal, you could do asyncToFinal/asyncToIOFinal. (Although the latter's perhaps not the best name, seeing as how the semantics of runAsyncFinal is like lowerAsync, and not asyncToIO?)

I could create a pull request for changing the interpreter names for Final and friends as well as the continuation effects.

KingoftheHomeless commented 5 years ago

I have (presumably) fixed the uses of makeSem_ and opened a pull request over at Avi-D-coder's with-polysemy-submodule branch https://github.com/Avi-D-coder/polysemy-zoo/pull/1.

It's really weird. I figure it must be a difference between 8.6.3 (or 8.6.4, which I use locally), and 8.6.5, but I don't see anything in the release notes that would hint at the change. On the plus side, this allows me to name the variables the way I originally wanted to, so that's nice.

isovector commented 5 years ago

@TheMatten mind syncing with people to figure out what's going on with makeSem_ here?

isovector commented 5 years ago

I'd like it if the names in this module rolled with the same convention as those in the main lib. I've been putting it off. Thanks for the PR!

TheMatten commented 5 years ago

@KingoftheHomeless @isovector it seems like reset uses name from type constructor (a) - use name from data constructor signature instead (s)

KingoftheHomeless commented 5 years ago

Previously, I had to use a when type annotating reset to get it to compile, but that's no longer the case. I have no idea why.

Have you tested with 8.6.3 (lts 13.0)? I can't do that since I'm on windows, and TH is broken on windows on 8.6.3.

TheMatten commented 5 years ago

Well, there was a change in logic that collects info about effect GADTs - maybe you were working with version of polysemy before this PR.

KingoftheHomeless commented 5 years ago

That may very well be it! In that case, we have nothing to worry about when it comes to compatibility.