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

Needs to be updated to account for remote-json's use of natural-transformation #40

Open
ecaustin opened this issue Jan 15, 2016 · 11 comments

Comments

@ecaustin
Copy link
Contributor

This remote-json commit broke compilation for hermit-shell:
ku-fpg/remote-json@ed3597c

There may be other bugs past this one, but for sure the definition of the remote session needs to be updated:

session :: JSONRPC.Session
session = JSONRPC.session
        $ (if debug 
           then traceSessionAPI "HERMIT-remote-json"
           else id)
        $ sendr

The use of id doesn't properly maintain the scope of the quantified variable introduced by the natural transformation type (~>).

I'm assuming the appropriate fix here is to add a natural identity transformation to natural-transformation and make hermit-shell's reliance on that library explicit?

@andygill
Copy link
Member

Nice find. What (GHC) version fails?

@ecaustin
Copy link
Contributor Author

It for sure fails with GHC 7.10.3 and 7.10.2.

I can draw up a smaller example to test with GHC 7.8.x since HERMIT doesn't support that version of GHC anymore.

The only reason it would not fail would be a change to the typechecker between versions that broke the semantics of identity, right?

That'd be bad...

@andygill
Copy link
Member

I'm actively heading towards remote-json, which is going to use the remote-monad package. And I'm happy for you to make any changes need to remote-json, or direct me to do so. But the code works in 7.10.3 for me. I'll add a travis instance of 8.X, and see what happens.

@ecaustin
Copy link
Contributor Author

Do you have a local commit you haven't pushed, or are you using an older version of any of the relevant libraries?

Here's the exact error I get:

[20 of 55] Compiling HERMIT.GHCI.Client ( src/HERMIT/GHCI/Client.hs, dist/build/HERMIT/GHCI/Client.o )

src/HERMIT/GHCI/Client.hs:34:17:
    Couldn't match type ‘forall x1. SessionAPI x1 -> IO x1’
                   with ‘SessionAPI x -> IO x’
    Expected type: SessionAPI
                   natural-transformation-0.3:Control.Natural.~> IO
                   -> SessionAPI x -> IO x
      Actual type: SessionAPI
                   natural-transformation-0.3:Control.Natural.~> IO
                   -> SessionAPI natural-transformation-0.3:Control.Natural.~> IO
    In the expression: id
    In the expression:
      (if debug then traceSessionAPI "HERMIT-remote-json" else id)

That's with a fresh checkout of natural-transformation, remote-json, and hermit-shell and GHC 7.10.3. I can include my entire package list if you think the issue might be with an old version of one of the compat libraries or something.

@andygill
Copy link
Member

No local changes. A local build of only remote-json though. Yes, what versions of the packages are you using.

@ecaustin
Copy link
Contributor Author

/home/e/setup/remote-json/.cabal-sandbox/x86_64-linux-ghc-7.10.3-packages.conf.d
   aeson-0.9.0.1
   attoparsec-0.13.0.1
   data-default-class-0.0.1
   dlist-0.7.1.2
   exceptions-0.8.1
   hashable-1.2.4.0
   mtl-2.2.1
   natural-transformation-0.2
   primitive-0.6.1.0
   remote-json-0.2
   scientific-0.3.4.4
   stm-2.4.4.1
   syb-0.6
   text-1.2.2.0
   transformers-compat-0.4.0.4
   unordered-containers-0.2.5.1
   vector-0.11.0.0

There's my package list dump.
This VM is less than a day old so everything should be the most recent version.

I can confirm I'm still having the issue with a local build of remote-json and using natural-transformation from Hackage.

@andygill
Copy link
Member

Can you try build just remote-json, in a local sandbox?

@ecaustin
Copy link
Contributor Author

Sorry, I misunderstood what you were asking for. I've updated my previous comment.

@ecaustin
Copy link
Contributor Author

I've identified a simpler test case that only requires natural-transformation:
ku-fpg/natural-transformation#9

@andygill
Copy link
Member

I've reproduced the error (on lilybank) and working on it.

src/HERMIT/GHCI/Client.hs:34:17:
    Couldn't match type ‘forall x1. SessionAPI x1 -> IO x1’
                   with ‘SessionAPI x -> IO x’
    Expected type: SessionAPI
                   natural-transformation-0.3:Control.Natural.~> IO
                   -> SessionAPI x -> IO x
      Actual type: SessionAPI
                   natural-transformation-0.3:Control.Natural.~> IO
                   -> SessionAPI natural-transformation-0.3:Control.Natural.~> IO
    In the expression: id
    In the expression:
      (if debug then traceSessionAPI "HERMIT-remote-json" else id)

andygill pushed a commit that referenced this issue Jan 20, 2016
andygill pushed a commit that referenced this issue Jan 25, 2016
@andygill
Copy link
Member

We might be able to avoid this if we use

Nat f g -> Nat f g

rather than

(forall a . f a -> g a) -> (forall a . f a -> g a)

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

No branches or pull requests

2 participants