DanielG / ghc-mod

Happy Haskell Hacking for editors. DEPRECATED
Other
677 stars 175 forks source link

ghc-mod 5.3.0.0/master: Cannot add module to context: not interpreted #554

Closed lierdakil closed 9 years ago

lierdakil commented 9 years ago

If I build project prior to running ghc-mod, I get the following error:

$ ghc-mod type pandoc-crossref.hs 13 11
EXCEPTION: types:
           Cannot add module References.Accessors to context: not interpreted

On clean project, this is not the case:

$ cabal clean
cleaning...
$ ~/github/ghc-mod/.cabal-sandbox/bin/ghc-mod type pandoc-crossref.hs 13 11
13 8 13 20 "(Maybe Format -> Pandoc -> IO Pandoc) -> IO ()"
13 8 13 23 "IO ()"
13 1 13 23 "IO ()"

You can find sources to reproduce this with at https://github.com/lierdakil/pandoc-crossref, although I suspect any built import will do.

DanielG commented 9 years ago

GHC/Cabal version + ghc-mod debug in both cases please :3

lierdakil commented 9 years ago
[:~] $ ghc --version
The Glorious Glasgow Haskell Compilation System, version 7.10.2
[:~] $ cabal --version
cabal-install version 1.22.6.0
using version 1.22.4.0 of the Cabal library 

ghc-mod debug (note it's literally identical for both cases): https://gist.github.com/lierdakil/e4338fd7478ab9f0c65f

DanielG commented 9 years ago

My guess is that ghc loads the modules' build outputs from the dist directory instead of interpreting the modules for some reason. I dunno this will need a closer look.

DanielG commented 9 years ago

This doesn't seem to be a problem when working on ghc-mod itself and there I definitely have compiled modules lying around on the search path all the time :/

lierdakil commented 9 years ago

I'm a little pressed for time right now, but I'll see if I can investigate this further.

DanielG commented 9 years ago

Right so mi_globals is Nothing which is why this error is being thrown.

DanielG commented 9 years ago

That's initialized by:

     -- We only fill in mi_globals if the module was compiled to byte
     -- code.  Otherwise, the compiler may not have retained all the
     -- top-level bindings and they won't be in the TypeEnv (see
     -- Desugar.addExportFlagsAndRules).  The mi_globals field is used
     -- by GHCi to decide whether the module has its full top-level
     -- scope available. (#5534)
     maybeGlobalRdrEnv :: GlobalRdrEnv -> Maybe GlobalRdrEnv
     maybeGlobalRdrEnv rdr_env
         | targetRetainsAllBindings (hscTarget dflags) = Just rdr_env
         | otherwise                                   = Nothing
DanielG commented 9 years ago

targetRetainsAllBindings is how keeping all vs. only exported binders is decided.

-- | Does this target retain *all* top-level bindings for a module,
-- rather than just the exported bindings, in the TypeEnv and compiled
-- code (if any)?  In interpreted mode we do this, so that GHCi can
-- call functions inside a module.  In HscNothing mode we also do it,
-- so that Haddock can get access to the GlobalRdrEnv for a module
-- after typechecking it.
targetRetainsAllBindings :: HscTarget -> Bool
targetRetainsAllBindings HscInterpreted = True
targetRetainsAllBindings HscNothing     = True
targetRetainsAllBindings _              = False

so wtf? We use one of the ones that returns True

lierdakil commented 9 years ago

So... my guess would be that compiled module is loaded as package in withContext, but is not pulled in as Target, hence this? Or maybe it's coerced into HscAsm, since it's compiled?

lierdakil commented 9 years ago

Might try bisecting to find out what's going on.

DanielG commented 9 years ago

I'm pretty sure you'll just arrive at that one big cabal-helper commit if you do that :p

DanielG commented 9 years ago

Maybe it has to do with some of cabal's inplace package registration stuff, but it really shouldn't as cabal-helper strips out all the inplace dependencies.

DanielG commented 9 years ago

Kay so I'm going to try adding a print in MkIface and see if that record field is actually Nothing or not.

DanielG commented 9 years ago

Of course everything blows up completely with this custom ghc .. gah. I'll have another look tomorrow.

DanielG commented 9 years ago

I basically just added a print $ isNothing $ maybeGlobalRdrEnv rdr_env in MkIface.hs:323 btw.

lierdakil commented 9 years ago

Could it be ghc 7.10 - specific, and not some immediate problem with ghc-mod itself? I'm pretty sure that it happens because withContext tries to load modules as IIModule, without setting them as targets. If I change IIModule to IIDecl . simpleImportDecl, then this error goes away (although all types get full qualification, which is not desired, but that could be rectified by bringing imports in scope properly and not with simpleImportDecl, I think)

lierdakil commented 9 years ago

Note: same issue was encountered in #515.

DanielG commented 9 years ago

Nah I was getting that with 7.8 as well I think.

DanielG commented 9 years ago

Hmm with the setting those modules as targets you might be on to something actually. Can you try what happens if you just add those in loadTargets?

DanielG commented 9 years ago

Okay finally got this to work together and it looks like the bit of code where my print is in doesn't even get invoked!

DanielG commented 9 years ago

If I pass -g-XTemplateHaskell it works of course. Must be related to HscNothing then. Do you use TemplateHaskell anywhere without telling cabal in other-extensions?

DanielG commented 9 years ago

@kazu-yamamoto any idea what might be happening here, you've investigated HscNothing weirdness before.

DanielG commented 9 years ago

I think I'll try constructing a testcase next this seems too weird to not be a bug. I mean HscNothing means that we don't even generate bytecode so the fact that mkIface isn't being called makes sense since we're not creating one, not even in memory. But that still leaves the question how does this work when there aren't any object files lying around.

DanielG commented 9 years ago

Ah the Binary instance for ModIface initializes mi_globals to Nothing too. So my guess now is that HscNothing is just not meant to be used like this. It only worked previously because we didn't bother with putting the build output directories on the search path but now cabal is doing that for us. I suppose it might be time to face the music and go HscInterpreted all the way.

rkarp commented 9 years ago

I've tried setting both setModeSimple = setModeIntelligent and setModeSimple = setModeIntelligent, the error remains the same. Looks like HscNothing isn't the problem here.

rkarp commented 9 years ago

I've narrowed it down somewhat. If I delete the .o and .hi files in dist/build/testproject/testproject-tmp, the type command works again, at least until I run cabal build and they are generated again.

DanielG commented 9 years ago

Have you tried -g-XTemplateHaskell too?

rkarp commented 9 years ago

It works if I pass that option, so that's another workaround.

DanielG commented 9 years ago

Very strange ...

DanielG commented 9 years ago

I'm working on a testcase to reproduce this but with the code below it just doesn't happen :/

I'm also digging through the GHC sources trying to find out when it would skip mkIface in favor of just loading the interface file off disk, so far I've identified this as possibly being a place where this might be happening.

   e <- genericHscCompileGetFrontendResult
            always_do_basic_recompilation_check
            m_tc_result mHscMessage
            hsc_env summary source_modified mb_old_iface (mod_index, nmods)

   case e of
       Left iface ->
           do details <- genModDetails hsc_env iface
              MASSERT(isJust maybe_old_linkable)
              return (HomeModInfo{ hm_details  = details,
                                   hm_iface    = iface,
                                   hm_linkable = maybe_old_linkable })

       Right (tc_result, mb_old_hash) ->
           -- run the compiler
           case hsc_lang of
               HscInterpreted ->
                   case ms_hsc_src summary of
                   t | isHsBootOrSig t ->
                       do (iface, _changed, details) <- hscSimpleIface hsc_env tc_result mb_old_hash
                          return (HomeModInfo{ hm_details  = details,
                                               hm_iface    = iface,
                                               hm_linkable = maybe_old_linkable })
                   _ -> do guts0 <- hscDesugar hsc_env summary tc_result
                           guts <- hscSimplify hsc_env guts0
                           (iface, _changed, details, cgguts) <- hscNormalIface hsc_env guts mb_old_hash
                           (hasStub, comp_bc, modBreaks) <- hscInteractive hsc_env cgguts summary
                           [...]
               HscNothing ->
                   do (iface, changed, details) <- hscSimpleIface hsc_env tc_result mb_old_hash
                   [...]

In the Left case GHC just re-uses the iface it got from genericHscCompileGetFrontendResult instead of passing it through the hsc_lang specific code again.

But since this seems to only be happening with HscNothing (thoug I've gotten conflicting reports on that) it might actually be happening further down.

The differences between the HscInterpreted code and HscNothing are that the former uses hscNormalIface while the later uses hscSimpleIface to get the interface for the module so the difference is somewhere in there probably.

Testcase:

{-# LANGUAGE ScopedTypeVariables #-}
module Main where

import GHC
import GHC.Paths (libdir)
import DynFlags
import PprTyThing
import Panic
import Outputable
import Pretty
import MonadUtils

import Control.Monad

main :: IO ()
main =
  defaultErrorHandler defaultFatalMessager defaultFlushOut $
    runGhc (Just libdir) $
      doStuff "A.hs" "A"

doStuff :: String -> String -> Ghc ()
doStuff targetFile targetModule = do
    dflags <- getSessionDynFlags
    _ <- setSessionDynFlags dflags {
        ghcLink   = NoLink
      , hscTarget = HscNothing
      , optLevel  = 0
      , ghcMode   = CompManager
      }
    target <- guessTarget targetFile Nothing
    setTargets [target]
    _ <- load LoadAllTargets

    setContext [IIModule $ mkModuleName targetModule]
    ty <- exprType "foo"
    liftIO $ putStrLn $ showOneLine dflags $ pprTypeForUser ty

    modSum <- getModSummary $ mkModuleName targetModule
    p <- parseModule modSum
    t <- typecheckModule p
    d <- desugarModule t

    return ()

showOneLine :: DynFlags -> SDoc -> String
showOneLine dflag = showDocWith dflag OneLineMode . withStyle dflag styleUnqualified

styleUnqualified :: PprStyle
styleUnqualified = mkUserStyle neverQualify AllTheWay

showDocWith dflags mode = Pretty.showDoc mode (pprCols dflags)

withStyle = withPprStyleDoc

--    hpt <- hsc_HPT <$> getSession
--    (hmi :: HomeModInfo) <- lookupUFM hpt targetModule

A.hs

module A where
foo = 123
DanielG commented 9 years ago

hscSimpleIface does eventually still call mkIface_ which is what I was print-debugging before and determined that it's not being called in the case when we're failing so the problem is not in there. Back to looking at the Left case of the case i mentioned above.

DanielG commented 9 years ago

Huzza! After another round of recompile-ghc-with-print statements I can now safely say that the above mentioned Left branch is called only when build outputs are present in dist/ so that's our culprit.

Specifically we end up here in genericHscCompileGetFrontendResult.

DanielG commented 9 years ago

Turns out I'm a moron and forgot to actually add support for passing options to the testcase so the -idist/build option I was passing on the command line wasn't even being applied xD. I still can't seem to get it to exhibit this problem though, just need to keep digging I guess.

I did find some useful dump/tracing options though: ./GhcTestcase -no-user-package-db -v5 -i -i. -idist/build/ -ddump-if-trace -ddump-hi-diffs -ddump-hi

{-# LANGUAGE ScopedTypeVariables #-}
module Main where

import GHC
import GHC.Paths (libdir)
import DynFlags
import PprTyThing
import Panic
import Outputable
import Pretty
import MonadUtils

import Control.Monad
import Control.Applicative
import System.Environment

main :: IO ()
main = do
  args <- getArgs
  defaultErrorHandler defaultFatalMessager defaultFlushOut $
    runGhc (Just libdir) $
      doStuff "A.hs" "A" args

doStuff :: String -> String -> [String] -> Ghc ()
doStuff targetFile targetModule args = do
    dflags0 <- getSessionDynFlags
    let dflags1 = dflags0 {
        ghcLink   = NoLink
      , hscTarget = HscNothing
      , optLevel  = 0
      , ghcMode   = CompManager
      }

    dflags2 <- addCmdOpts args dflags1

    _ <- setSessionDynFlags dflags2

    target <- guessTarget targetFile Nothing
    setTargets [target]
    _ <- load LoadAllTargets

    setContext [IIModule $ mkModuleName targetModule]
    ty <- exprType "foo"
    liftIO $ putStrLn $ showOneLine dflags2 $ pprTypeForUser ty

    modSum <- getModSummary $ mkModuleName targetModule
    p <- parseModule modSum
    t <- typecheckModule p
    d <- desugarModule t

    return ()

-- | Parse command line ghc options and add them to the 'DynFlags' passed
addCmdOpts :: GhcMonad m => [String] -> DynFlags -> m DynFlags
addCmdOpts cmdOpts df =
    fst3 <$> parseDynamicFlags df (map noLoc cmdOpts)
  where
    fst3 (a,_,_) = a

showOneLine :: DynFlags -> SDoc -> String
showOneLine dflag = showDocWith dflag OneLineMode . withStyle dflag styleUnqualified

styleUnqualified :: PprStyle
styleUnqualified = mkUserStyle neverQualify AllTheWay

showDocWith dflags mode = Pretty.showDoc mode (pprCols dflags)

withStyle = withPprStyleDoc
DanielG commented 9 years ago

So in checkOldIface:

        when src_changed $
            traceHiDiffs (nest 4 $ text "Source file changed or recompilation check turned off")

        case src_changed of
            -- If the source has changed and we're in interactive mode,
            -- avoid reading an interface; just return the one we might
            -- have been supplied with.
            True | not (isObjectTarget $ hscTarget dflags) ->
                return (MustCompile, maybe_iface)

            [...]

From the trace output of the testcase I can tell that src_change = True given that hscTarget = HscNothing not . isObjectTarget should return True so we should be hitting that first case. So obviously the interface isn't being read/created here but rather just passed through :/

Need to go back and find out where that's coming from.

DanielG commented 9 years ago

Starting from loadModule mb_old_iface/maybe_iface seems to be Nothing.

loadModule :: (TypecheckedMod mod, GhcMonad m) => mod -> m mod
loadModule tcm = do
   [...]
   mod_info <- liftIO $ compileOne' (Just tcg) Nothing
                                    hsc_env ms 1 1 Nothing mb_linkable
                                    source_modified
   [...]
DanielG commented 9 years ago

Ah upsweep_mod in GhcMake passes in old interfaces. I think GhcMake is what we're using by setting CompManager but dunno.

DanielG commented 9 years ago

Alright so upsweep_mod gets the interface it passes down to compileOne from hm_iface I think the home module table should only have anything in it when we already compiled something, so since we're compiling from scratch that shouldn't be where it's coming from.

DanielG commented 9 years ago

@lierdakil did you ever investigate that thing with using IIDecl further? I might be going down the completely wrong path here..

Either way there is a simple way to work around this it turns out. I have control of the output dir paths in cabal-helper so I could just add a flag that redirects all the output/obj search paths to a non-existent directory then ghc can't possibly try to load anything off disk. I'd like to avoid that if at all possible though because we might want to add output generation support at some point in the future and this just seems like an ugly hack.

DanielG commented 9 years ago

If I just don't setTarget a module that is a transitive dependency of another that I am loading then I get: "Cannot add module B to context: not a home module". So I don't think it's because we're not setting a module as a target.

DanielG commented 9 years ago

readIface/readBinIface seems to be where the interfaces are read off disk. The only relevant place where this is called is in checkOldIface from getIface. This only ever happens if isObjectTarget is True though, which is not the case for either of our hscTargets

DanielG commented 9 years ago

So I have two remaining questions that need answering:

1) When, how and why does GHC load ifaces off disk instead of calling mkIface_ 2) Why does my testcase not trigger this

DanielG commented 9 years ago

Some more print debugging tells me that mb_old_iface in compileOne' is always Nothing both with the testcase and when running the offending ghc-mod command on pandoc-crossref.

DanielG commented 9 years ago

In the testcase src_changed seems to be True and we end up in the isObjectTarget case whereas with ghc-mod we end up in the checkVersions case

        case src_changed of
            -- If the source has changed and we're in interactive mode,
            -- avoid reading an interface; just return the one we might
            -- have been supplied with.
            True | not (isObjectTarget $ hscTarget dflags) -> do
                liftIO $ hPutStrLn stderr $ show ("not (isObjectTarget $ hscTarget dflags)", isNothing maybe_iface)
                return (MustCompile, maybe_iface)

            -- Try and read the old interface for the current module
            -- from the .hi file left from the last time we compiled it
            True -> do
                liftIO $ hPutStrLn stderr $ show ("return (MustCompile, maybe_iface')")
                maybe_iface' <- getIface
                return (MustCompile, maybe_iface')

            False -> do
                maybe_iface' <- getIface
                case maybe_iface' of
                    -- We can't retrieve the iface

                    Nothing    -> do
                           liftIO $ hPutStrLn stderr $ show ("return (MustCompile, Nothing)", "We can't retrieve the iface")
                           return (MustCompile, Nothing)

                    -- We have got the old iface; check its versions
                    -- even in the SourceUnmodifiedAndStable case we
                    -- should check versions because some packages
                    -- might have changed or gone away.
                    Just iface -> do
                           liftIO $ hPutStrLn stderr $ show ("checkVersions hsc_env mod_summary iface")
                           checkVersions hsc_env mod_summary iface
DanielG commented 9 years ago

Interresting so it boils down to why does the testcase think the source was modified since the binaries were written. If I do touch **/*.hs in pandoc-crossref after cabal building everything it works of course.

so for (1) I can answer the how and when just not the why. I don't understand why it would load interfaces off disk with HscNothing it seems to me from some comments in the code that that's exactly what shouldn't be happening but I might be missing something. I could understand if that's just how HscNothing is "supposed to work" but it doesn't seem like it.

for (2) I can at least refine the question: why does ghc thing the sources where modified since the binaries were built when they weren't.

DanielG commented 9 years ago

src_changed is true in the testcase because src_modified = SourceModified. For ghc-mod it's SourceUnmodifiedAndStable

DanielG commented 9 years ago

I think I'm just going to switch ghc-mod to HscInterpreted for all commands that need it. There were some things that didn't work with that but those might have been ghc bugs anyways so this might just be fine now. If there is anything that's completely broken with that we can still work around it by removing the build directory from the search path completely (for the affected versions of ghc) and patch it upstream.

DanielG commented 9 years ago

FFFFFFFFFFFFFFFFFFFFFF I finally found the global switch that turns off object code loading >_<

targetAllowObjCode in Target, guessTarget sets that to True unless the string has a * prefix like in ghci's :module command.

DanielG commented 9 years ago

Damn setting that to False it still doesn't fix the issue. I'm trying to force the test case to exhibit this problem still. Got as far as having it notice the object files on disk but it refuses to load the interfaces.

Turns out you need to add -outputdir, -odir, -hidir and friends to get this to work -.-

DanielG commented 9 years ago

AHA! I wasn't passing exactly the same flags to ghc as cabal build was so ghc recompiled stuff instead of loading it from disk. Now the testcase works!

DanielG commented 9 years ago

Failing testcase: https://gist.github.com/DanielG/f1b92066b910a19ba707

Changing targetAllowObjCode to False in the testcase fixes the problem like it should tough. So the problem must be in ghc-mod somewhere.