Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#8628 closed bug (fixed)

dynCompileExpr breaks repeated runDecls of the same name

Reported by: agibiansky Owned by:
Priority: normal Milestone:
Component: GHC API Version: 7.6.3
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case: ghc-api/T8628
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

Using dynCompileExpr with runDecls from InteractiveEval seems to be incredibly problematic when evaluating declarations for the same name. For instance, consider the following code:

  runDecls "data X = Y Int"
  gtry $ runStmt "print (Y 3)" RunToCompletion :: GhcMonad m => m (Either SomeException RunResult)
  runDecls "data X = Y Int deriving Show"
  runStmt "print (Y 8)" RunToCompletion

As expected, this prints Y 8 once (the first one fails because X is not an instance of Show).

However, if we insert an arbitrary dynCompileExpr, things begin to break:

  runDecls "data X = Y Int"
  gtry $ runStmt "print (Y 3)" RunToCompletion :: GhcMonad m => m (Either SomeException RunResult)
  runDecls "data X = Y Int deriving Show"
  _ <- dynCompileExpr "'x'"
  runStmt "print (Y 8)" RunToCompletion

We then get:

No instance for (GHC.Show.Show :Interactive.X)
  arising from a use of `System.IO.print'
Possible fix:
  add an instance declaration for (GHC.Show.Show :Interactive.X)

Which is clearly incorrect - we haven't done anything to change the data declaration.

I'm not sure what's going on, but I've tried to investigate into the source of dynCompileExpr. It loads Data.Dynamic via a setContext and then unloads it, and that may be the issue.

If we do the following (very similar to the dynCompileExpr source), things work fine:

  runDecls "data X = Y Int"
  gtry $ runStmt "print (Y 3)" RunToCompletion :: GhcMonad m => m (Either SomeException RunResult)
  runDecls "data X = Y Int deriving Show"

  let stmt = "let __expr = 'x'"
  Just (ids, hval, fixenv) <- withSession $ \hsc_env -> 
                          liftIO $ hscStmt hsc_env stmt
  vals <- liftIO (unsafeCoerce hval :: IO [Char])
  liftIO $ print vals -- thish prints "x", as expected

  runStmt "print (Y 8)" RunToCompletion

I have not tested with GHC API other than 7.6.3, and could not find a bug that was similar to this.

Change History (6)

comment:1 Changed 3 years ago by Simon Peyton Jones <simonpj@…>

In 5dffb4ac14b53362ebe9a67c5c6a01f9c9c25229/ghc:

Refactor the way shadowing in handled in GHCi

If you say
  ghci> import Foo( T )
  ghci> data T = MkT
  ghci> data T = XXX
then the second 'data T' should shadow the first.  But the qualified
Foo.T should still be available.  We really weren't handling this
correctly at all, resulting in Trac #8639 and #8628 among others

This patch:

* Add RdrName.extendGlobalRdrEnv, which does shadowing properly

* Change HscTypes.icExtendGblRdrEnv (was badly-named icPlusGblRdrEnv)
  to use the new function

* Change RnNames.extendGobalRdrEnvRn to use the new function

* Move gresFrom Avails into RdrName
* Better pprGlobalRdrEnv function in RdrName

comment:2 Changed 3 years ago by Simon Peyton Jones <simonpj@…>

comment:3 Changed 3 years ago by simonpj

  • Resolution set to fixed
  • Status changed from new to closed
  • Test Case set to ghc-api/T8628

Good example. Fixed!

Simon

comment:4 Changed 3 years ago by agibiansky

Simon,

The same thing happens with setContext instead of dynCompileExpr. Does the fix correct that issue as well?

(And do you know how I might work around this in the meantime? Is there something else I can use instead of setContext, or modify the setContext source somehow, to make it work?)

Andrew

comment:5 Changed 3 years ago by simonpj

I think so, but we won't know for sure until we try it. Maybe you can try?

I don't know what a good workaround might be. I'm expecting a release candidate for 7.8 any day now, so you can use that.

Simon

comment:6 Changed 3 years ago by agibiansky

Alright, thanks. Looking at the source it seems like the same issue, as it uses icPlusGblRdrEnv. I'll try it on 7.8 RC when it's out, I guess.

Thanks!

Note: See TracTickets for help on using tickets.