GHC Api

[Simon Marlow]

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 at 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
> | >
>