On 14/02/2012 02:49, David Terei wrote:
On 13 February 2012 14:41, Simon Marlow<[email protected]> wrote:
Right, but I think what you're missing is that we sometimes need to keep
modules in the context even when they can't currently be imported. There are
a couple of comments about this - one on setGHCContextFromGHCiState and one
in GhciMonad pointing to the relevant tickets.
OK that helps explain the complexity. I didn't realize how much GHC
will try to load a module, if I say :add E and no such module exists
ghci still puts it in the context and tries to load it.
Hmm, I think you might be mixing up :add with :module/import. The
former adds a *target*, while the latter add to the *context*. But
anyway, all of those commands reject a module if it doesn't exist:
Prelude> :l hello
[1 of 1] Compiling Main ( hello.hs, interpreted )
Ok, modules loaded: Main.
*Main> :add no_such_file
target `no_such_file' is not a module name or a source file
*Main> :module +Does.Not.Exist
<no location info>:
Could not find module `Does.Not.Exist'
It is not a module in the current program, or in any known package.
The import command had a bug where it wasn't doing this check, but that
was fixed (#5836).
I'm not really
sure how to fit -XSafe into this design right now. The existing
behaviour would seem to say I should put the unsafe import in the
context and keep repeating the failure so the user can fix it...
No, not at all. You should do the error checking up front and reject
the command if there's a problem.
The only time we remember imports that don't current exist is after a
:reload, where there was an error and some of the previous imports
aren't valid. They might become valid later, so we remember them in the
context. Users wanted this behaviour (there's a ticket about it,
pointed to by a comment in the code I think).
So there's an easy fix for your bug BTW. See the fix I made for #5836, you
just need to do something else there.
Yes I saw that fix but didn't think it would give me what I want as
its only for the 'import' branch. However it seems that's the
behaviour I want as :load should repeat the failure.
The real problem (and I think this might be underlying the confusion
here) is that addImportToContext does not call checkAdd, making it the
odd one out from addModuleToContext/setContext. It doesn't call
checkAdd because there's a bit of refactoring needed that I was too lazy
to do - checkAdd takes a String, but addImportToContext has a full
import declaration.
My fix for #5836 is really a hack, the right fix would be to refactor
checkAdd so that it is called by all the code paths. That would fix
your Safe Haskell bug too. I can do that if you like, or do you want to
do it?
OK I still don't
like the code but given the complexity of the ghci user interface its
implementing I don't see a clear way to do it better. I think the main
issue I have is this trick of calling checkAdd multiple times,
handling exceptions differently (that should be documented please).
Will do.
Having multiple data structures and moving between them seems a better
choice but thats just an instinct, I haven't thought through what it
would look like.
Thanks for the explanations though, sorry for bad mouthing your code :)
No problem :) It clearly needs some commentary/refactoring.
Cheers,
Simon
Cheers,
David
Cheers,
Simon
e.g the 'setContext' function takes two lists of imports, maps
'checkAdd' over them and saves the new context. It then calls
'setGHCContextFromGHCiState' which does basically (map over the input
with checkAdd) the same thing but calls into GHC at the end to set the
actual context.
What I want is one interface to this all. So a setContext method that
handles calling 'checkAdd' and 'addNotSubsumed' and can handle batch
or individual additions. Then for all the :module, :import commands I
just call this 'setContext'. Based on how I've seen the code used this
seems quite doable.
Cheers,
David.
+checkAdd :: Bool -> String -> GHCi (InteractiveImport)
checkAdd star mstr = do
dflags<- getDynFlags
case safeLanguageOn dflags of
@@ -1664,8 +1661,8 @@ checkAdd star mstr = do
s<- GHC.isModuleTrusted m
case s of
True -> return $ IIDecl (simpleImportDecl $
moduleName
m)
- False -> ghcError $ CmdLineError $ "can't import "
++
mstr
- ++ " as it isn't
trusted."
+ False -> ghcError $ CmdLineError $
+ "can't import " ++ mstr ++ " as it isn't trusted."
False | star -> do m<- wantInterpretedModule mstr
return $ IIModule m
@@ -1687,12 +1684,26 @@ checkAdd star mstr = do
--
setGHCContextFromGHCiState :: GHCi ()
setGHCContextFromGHCiState = do
- let ok (IIModule m) = checkAdd True (moduleNameString (moduleName
m))
- ok (IIDecl d) = checkAdd False (moduleNameString (unLoc
(ideclName d)))
st<- getGHCiState
- iidecls<- filterM (tryBool . ok) (transient_ctx st ++ remembered_ctx
st)
- setGHCContext iidecls
+ goodTran<- filterM (tryBool . ok) $ transient_ctx st
+ goodRemb<- filterM (tryBool . ok) $ remembered_ctx st
+ -- drop bad imports so we don't keep replaying it to the user!
+ modifyGHCiState $ \s -> s { transient_ctx = goodTran }
+ modifyGHCiState $ \s -> s { remembered_ctx = goodRemb }
+ setGHCContext (goodTran ++ goodRemb)
+ where
+ ok (IIModule m) = checkAdd True (moduleNameString (moduleName m))
+ ok (IIDecl d) = checkAdd False (moduleNameString (unLoc
(ideclName
d)))
+
+setContext :: [String] -> [String] -> GHCi ()
+setContext starred not_starred = do
+ is1<- mapM (checkAdd True) starred
+ is2<- mapM (checkAdd False) not_starred
+ let iss = foldr addNotSubsumed [] (is1++is2)
+ modifyGHCiState $ \st -> st { remembered_ctx = iss, transient_ctx
= []
}
+ -- delete the transient context
+ setGHCContextFromGHCiState
-- | Sets the GHC contexts to the given set of imports, adding a
Prelude
-- import if there isn't an explicit one already.
@@ -2745,7 +2756,7 @@ tryBool :: GHCi a -> GHCi Bool
tryBool m = do
r<- ghciTry m
case r of
- Left _ -> return False
+ Left e -> showException e>> return False
Right _ -> return True
--
----------------------------------------------------------------------------
_______________________________________________
Cvs-ghc mailing list
[email protected]
http://www.haskell.org/mailman/listinfo/cvs-ghc
_______________________________________________
Cvs-ghc mailing list
[email protected]
http://www.haskell.org/mailman/listinfo/cvs-ghc
