
Simon and othere Happy new year! When debugging Trac #8628 I wrote the following: main = do [libdir] <- getArgs ok <- runGhc (Just libdir) $ do dflags <- getSessionDynFlags -- (1) setSessionDynFlags dflags liftIO (setUnsafeGlobalDynFlags dflags) -- (2) setContext [IIDecl (simpleImportDecl pRELUDE_NAME)] -- (3) runDecls "data X = Y Int" runStmt "print True" -- (4) return () There are several odd things here 1. Why do I have to do this "getSessionDynFlags/setSessionDynFlags" thing. Seems bizarre. I just copied it from some other tests in ghc-api/. Is it necessary? If not, can we remove it from all tests? 2. Initially I didn't have that setUnsafeGlobalDynFlags call. But then I got T8628.exe: T8628.exe: panic! (the 'impossible' happened) (GHC version 7.7.20131228 for i386-unknown-mingw32): v_unsafeGlobalDynFlags: not initialised which is a particularly unhelpful message. It arose because I was using a GHC built with assertions on, and a warnPprTrace triggered. Since this could happen to anyone, would it make sense to make this part of runGhc and setSessionDynFlags? 3. Initially I didn't have that setContext call, and got a complaint that "Int is not in scope". I was expecting the Prelude to be implicitly in scope. But I'm not sure where to fix that. Possibly part of the setup in runGhc? 4. The runStmt should print something somewhere, but it doesn't. Why not? What do you think? Simon

On 02/01/14 07:06, Simon Peyton-Jones wrote:
Simon and othere
Happy new year!
When debugging Trac #8628 I wrote the following:
main
= do [libdir] <- getArgs
ok <- runGhc (Just libdir) $ do
dflags <- getSessionDynFlags -- (1)
setSessionDynFlags dflags
liftIO (setUnsafeGlobalDynFlags dflags) -- (2)
setContext [IIDecl (simpleImportDecl pRELUDE_NAME)] -- (3)
runDecls "data X = Y Int"
runStmt “print True” -- (4)
return ()
There are several odd things here
1.Why do I have to do this “getSessionDynFlags/setSessionDynFlags” thing. Seems bizarre. I just copied it from some other tests in ghc-api/. Is it necessary? If not, can we remove it from all tests?
It's a sensible question given the naming of the functions. The API is definitely clunky here, but there is a purpose to these calls. setSessionDynFlags loads the package database and does the necessary processing to make packages available. We don't do that automatically, because the client might want to add their own package flags to the DynFlags between the calls to getSessionDynFlags and setSessionDynFlags. Incidentally you can find out some of this stuff from the Haddock docs, e.g. look at the docs for setSessionDynFlags.
2.Initially I didn’t have that setUnsafeGlobalDynFlags call. But then I got
T8628.exe: T8628.exe: panic! (the 'impossible' happened)
(GHC version 7.7.20131228 for i386-unknown-mingw32):
v_unsafeGlobalDynFlags: not initialised
which is a particularly unhelpful message. It arose because I was using a GHC built with assertions on, and a warnPprTrace triggered. Since this could happen to anyone, would it make sense to make this part of runGhc and setSessionDynFlags?
I'm not all that familiar with the unsafeGlobalDynFlags stuff (that's Ian's invention), but from looking at the code it looks like you wouldn't need to call this if you were calling parseDynamicFlags. It should be safe to call parseDynamicFlags with an empty set of flags to parse.
3.Initially I didn’t have that setContext call, and got a complaint that “Int is not in scope”. I was expecting the Prelude to be implicitly in scope. But I’m not sure where to fix that. Possibly part of the setup in runGhc?
I think it's sensible to require a call to setContext to bring the Prelude into scope. The client might want a different context, and setContext isn't free, so we probably don't want to initialise a default context.
4.The runStmt should print something somewhere, but it doesn’t. Why not?
I've no idea! It does look like it should print something. Cheers, Simon
What do you think?
Simon

On Thu, Jan 02, 2014 at 03:10:24PM +0000, Simon Marlow wrote:
On 02/01/14 07:06, Simon Peyton-Jones wrote:
Happy new year!
And to you :-)
runStmt “print True” -- (4)
4.The runStmt should print something somewhere, but it doesn’t. Why not?
I've no idea! It does look like it should print something.
Is this with a statically linked or dynamically linked GHC? Does doing runStmt "hFlush stdout" afterwards make it appear? Thanks Ian

| Is this with a statically linked or dynamically linked GHC? I don't know. How would I find out? (It's the one built by validate.) You are asking about GHC, but I guess there's also the question of whether the test program itself is statically or dynamically linked. I don't know that either. I just said ~/5builds/HEAD-2/inplace/bin/ghc-stage2 -o T8628 T8628.hs -package ghc Why would static/dynamic linking make a difference? That's very confusing! | Does doing | runStmt "hFlush stdout" | afterwards make it appear? Yes, it does. Again, that's very confusing. Shouldn't we automatically do a hFlush, so that output is not silently discarded? Simon | -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Ian | Lynagh | Sent: 02 January 2014 18:29 | To: ghc-devs@haskell.org | Subject: Re: GHC Api | | On Thu, Jan 02, 2014 at 03:10:24PM +0000, Simon Marlow wrote: | > On 02/01/14 07:06, Simon Peyton-Jones wrote: | > > | > >Happy new year! | | And to you :-) | | > > runStmt “print True” -- (4) | > | > >4.The runStmt should print something somewhere, but it doesn’t. Why | not? | > | > I've no idea! It does look like it should print something. | | Is this with a statically linked or dynamically linked GHC? | | Does doing | runStmt "hFlush stdout" | afterwards make it appear? | | | Thanks | Ian | | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs

On Fri, Jan 03, 2014 at 10:19:02AM +0000, Simon Peyton-Jones wrote:
| Is this with a statically linked or dynamically linked GHC?
I don't know. How would I find out? (It's the one built by validate.)
You are asking about GHC, but I guess there's also the question of whether the test program itself is statically or dynamically linked.
Oh, yes, sorry, I was thinking this was in ghci for some reason. You're right that it's the test program we need to know about.
Why would static/dynamic linking make a difference? That's very confusing!
With dynamic linking, there will be one shared copy of base, and in particular one shared stdout buffer. The runtime will flush that buffer when the program exits. With static linking, you'll be loading a second copy of base in which the statement is evalauted, and that base will have a separate stdout buffer. GHCi flushes this when appropriate by calling flushInterpBuffers. Thanks Ian

| setSessionDynFlags loads the package database and does the necessary | processing to make packages available. We don't do that automatically, | because the client might want to add their own package flags to the | DynFlags between the calls to getSessionDynFlags and setSessionDynFlags. So it would be *OK* for runGhc to call setSessionDynFlags; but it might be a bit inefficient in the case you describe where the user adds their own package flags (which is uncommon). Correct? In that case, couldn't runGhc do the package initialisation thing, and we can perhaps provide a super-efficient variant of runGhc that doesn't do so for the reason you state? That would make the common case simple. | I'm not all that familiar with the unsafeGlobalDynFlags stuff (that's | Ian's invention), but from looking at the code it looks like you | wouldn't need to call this if you were calling parseDynamicFlags. It | should be safe to call parseDynamicFlags with an empty set of flags to | parse. True but weird. The point is that, instead of parsing a string, runGhc creates a fresh empty DynFlags (in inigGhcMonad actually). Since this is an alternative to parsing a string, it should set the static thing too, just as the string-parsing route does (in parseDynamicFlagsFull, as you point out). I'll do this unless you or Ian object. | I think it's sensible to require a call to setContext to bring the | Prelude into scope. The client might want a different context, and | setContext isn't free, so we probably don't want to initialise a default | context. This is very similar to the first point above. Maybe runGhc can do common thing (initialise packages, import Prelude), with a variant that doesn't? What do others think? Simon | -----Original Message----- | From: Simon Marlow [mailto:marlowsd@gmail.com] | Sent: 02 January 2014 15:10 | To: Simon Peyton-Jones | Cc: ghc-devs | Subject: Re: GHC Api | | On 02/01/14 07:06, Simon Peyton-Jones wrote: | > Simon and othere | > | > Happy new year! | > | > When debugging Trac #8628 I wrote the following: | > | > main | > | > = do [libdir] <- getArgs | > | > ok <- runGhc (Just libdir) $ do | > | > dflags <- getSessionDynFlags -- (1) | > | > setSessionDynFlags dflags | > | > liftIO (setUnsafeGlobalDynFlags dflags) -- (2) | > | > setContext [IIDecl (simpleImportDecl pRELUDE_NAME)] -- (3) | > | > runDecls "data X = Y Int" | > | > runStmt "print True" -- (4) | > | > return () | > | > There are several odd things here | > | > 1.Why do I have to do this "getSessionDynFlags/setSessionDynFlags" | > thing. Seems bizarre. I just copied it from some other tests in | > ghc-api/. Is it necessary? If not, can we remove it from all tests? | | It's a sensible question given the naming of the functions. The API is | definitely clunky here, but there is a purpose to these calls. | setSessionDynFlags loads the package database and does the necessary | processing to make packages available. We don't do that automatically, | because the client might want to add their own package flags to the | DynFlags between the calls to getSessionDynFlags and setSessionDynFlags. | Incidentally you can find out some of this stuff from the Haddock docs, | e.g. look at the docs for setSessionDynFlags. | | > 2.Initially I didn't have that setUnsafeGlobalDynFlags call. But then | > I got | > | > T8628.exe: T8628.exe: panic! (the 'impossible' happened) | > | > (GHC version 7.7.20131228 for i386-unknown-mingw32): | > | > v_unsafeGlobalDynFlags: not initialised | > | > which is a particularly unhelpful message. It arose because I was | > using a GHC built with assertions on, and a warnPprTrace triggered. | > Since this could happen to anyone, would it make sense to make this | > part of runGhc and setSessionDynFlags? | | I'm not all that familiar with the unsafeGlobalDynFlags stuff (that's | Ian's invention), but from looking at the code it looks like you | wouldn't need to call this if you were calling parseDynamicFlags. It | should be safe to call parseDynamicFlags with an empty set of flags to | parse. | | > 3.Initially I didn't have that setContext call, and got a complaint | > that "Int is not in scope". I was expecting the Prelude to be | > implicitly in scope. But I'm not sure where to fix that. Possibly | > part of the setup in runGhc? | | I think it's sensible to require a call to setContext to bring the | Prelude into scope. The client might want a different context, and | setContext isn't free, so we probably don't want to initialise a default | context. | | > 4.The runStmt should print something somewhere, but it doesn't. Why | not? | | I've no idea! It does look like it should print something. | | Cheers, | Simon | | > What do you think? | > | > Simon | >

On 03/01/14 13:46, Simon Peyton-Jones wrote:
| setSessionDynFlags loads the package database and does the necessary | processing to make packages available. We don't do that automatically, | because the client might want to add their own package flags to the | DynFlags between the calls to getSessionDynFlags and setSessionDynFlags.
So it would be *OK* for runGhc to call setSessionDynFlags; but it might be a bit inefficient in the case you describe where the user adds their own package flags (which is uncommon). Correct?
In that case, couldn't runGhc do the package initialisation thing, and we can perhaps provide a super-efficient variant of runGhc that doesn't do so for the reason you state? That would make the common case simple.
| I'm not all that familiar with the unsafeGlobalDynFlags stuff (that's | Ian's invention), but from looking at the code it looks like you | wouldn't need to call this if you were calling parseDynamicFlags. It | should be safe to call parseDynamicFlags with an empty set of flags to | parse.
True but weird. The point is that, instead of parsing a string, runGhc creates a fresh empty DynFlags (in inigGhcMonad actually). Since this is an alternative to parsing a string, it should set the static thing too, just as the string-parsing route does (in parseDynamicFlagsFull, as you point out).
I haven't looked into this in detail, but clients that need to parse command line flags will be doing *both* runGhc and parseDynamicFlags, so it's not really an alternative to parsing a string. Still, perhaps it would be fine to call setUnsafeGlobalDynFlags twice.
I'll do this unless you or Ian object.
| I think it's sensible to require a call to setContext to bring the | Prelude into scope. The client might want a different context, and | setContext isn't free, so we probably don't want to initialise a default | context.
This is very similar to the first point above. Maybe runGhc can do common thing (initialise packages, import Prelude), with a variant that doesn't?
What do others think?
We could certainly add another function to the API that packages up runGhc, setSessionDynFlags and setContext. It's not clear to me that this should be what runGhc does, though; it seems less likely to cause problems if we just add a new function to the API and let people migrate slowly. In the case of setContext, there are clients that don't need an interactive context at all: Haddock, for example. I do think that if we're going to package up "common" stuff we should look at what really is common, by surveying a few existing clients on Hackage. Cheers, Simon
Simon
| -----Original Message----- | From: Simon Marlow [mailto:marlowsd@gmail.com] | Sent: 02 January 2014 15:10 | To: Simon Peyton-Jones | Cc: ghc-devs | Subject: Re: GHC Api | | On 02/01/14 07:06, Simon Peyton-Jones wrote: | > Simon and othere | > | > Happy new year! | > | > When debugging Trac #8628 I wrote the following: | > | > main | > | > = do [libdir] <- getArgs | > | > ok <- runGhc (Just libdir) $ do | > | > dflags <- getSessionDynFlags -- (1) | > | > setSessionDynFlags dflags | > | > liftIO (setUnsafeGlobalDynFlags dflags) -- (2) | > | > setContext [IIDecl (simpleImportDecl pRELUDE_NAME)] -- (3) | > | > runDecls "data X = Y Int" | > | > runStmt "print True" -- (4) | > | > return () | > | > There are several odd things here | > | > 1.Why do I have to do this "getSessionDynFlags/setSessionDynFlags" | > thing. Seems bizarre. I just copied it from some other tests in | > ghc-api/. Is it necessary? If not, can we remove it from all tests? | | It's a sensible question given the naming of the functions. The API is | definitely clunky here, but there is a purpose to these calls. | setSessionDynFlags loads the package database and does the necessary | processing to make packages available. We don't do that automatically, | because the client might want to add their own package flags to the | DynFlags between the calls to getSessionDynFlags and setSessionDynFlags. | Incidentally you can find out some of this stuff from the Haddock docs, | e.g. look at the docs for setSessionDynFlags. | | > 2.Initially I didn't have that setUnsafeGlobalDynFlags call. But then | > I got | > | > T8628.exe: T8628.exe: panic! (the 'impossible' happened) | > | > (GHC version 7.7.20131228 for i386-unknown-mingw32): | > | > v_unsafeGlobalDynFlags: not initialised | > | > which is a particularly unhelpful message. It arose because I was | > using a GHC built with assertions on, and a warnPprTrace triggered. | > Since this could happen to anyone, would it make sense to make this | > part of runGhc and setSessionDynFlags? | | I'm not all that familiar with the unsafeGlobalDynFlags stuff (that's | Ian's invention), but from looking at the code it looks like you | wouldn't need to call this if you were calling parseDynamicFlags. It | should be safe to call parseDynamicFlags with an empty set of flags to | parse. | | > 3.Initially I didn't have that setContext call, and got a complaint | > that "Int is not in scope". I was expecting the Prelude to be | > implicitly in scope. But I'm not sure where to fix that. Possibly | > part of the setup in runGhc? | | I think it's sensible to require a call to setContext to bring the | Prelude into scope. The client might want a different context, and | setContext isn't free, so we probably don't want to initialise a default | context. | | > 4.The runStmt should print something somewhere, but it doesn't. Why | not? | | I've no idea! It does look like it should print something. | | Cheers, | Simon | | > What do you think? | > | > Simon | >
participants (3)
-
Ian Lynagh
-
Simon Marlow
-
Simon Peyton-Jones