diff options
author | Phuong Trinh <lolotp@fb.com> | 2019-05-31 11:46:39 +0100 |
---|---|---|
committer | Ray Shih <rayshih@fb.com> | 2020-07-27 09:45:14 -0700 |
commit | 0c70208868f43e44f11b84de051fa393aacf6270 (patch) | |
tree | 7e7b4734d11932637ae2ceb88b31bd15b6f98694 | |
parent | 9c93a63fe700f6c98c6dbaa6977946571abfef2e (diff) | |
download | haskell-0c70208868f43e44f11b84de051fa393aacf6270.tar.gz |
Fix #16511: changes in interface dependencies should trigger recompilation
If the union of dependencies of imported modules change, the mi_deps
field of the interface files should change as well. Because of that, we
need to check for changes in this in recompilation checker which we are
not doing right now. This adds a checks for that. For more details,
please
see https://gitlab.haskell.org/ghc/ghc/issues/16511.
(cherry picked from commit bb53d08f573a476a7da2ea0701b29d5d7f89cc8c)
-rw-r--r-- | compiler/iface/MkIface.hs | 63 | ||||
-rw-r--r-- | compiler/main/GhcMake.hs | 22 | ||||
-rw-r--r-- | compiler/main/HscTypes.hs | 26 |
3 files changed, 87 insertions, 24 deletions
diff --git a/compiler/iface/MkIface.hs b/compiler/iface/MkIface.hs index bc575497f4..0ae3e4489f 100644 --- a/compiler/iface/MkIface.hs +++ b/compiler/iface/MkIface.hs @@ -115,6 +115,7 @@ import Control.Monad import Data.Function import Data.List import qualified Data.Map as Map +import qualified Data.Set as Set import Data.Ord import Data.IORef import System.Directory @@ -1486,10 +1487,22 @@ checkMergedSignatures mod_summary iface = do -- - a new home module has been added that shadows a package module -- See bug #1372. -- +-- In addition, we also checks if the union of dependencies of the imported +-- modules has any difference to the existing set of dependencies. We would need +-- to recompile in that case also since the `mi_deps` field of ModIface needs +-- to be updated to match that information. See bug #16511. +-- -- Returns (RecompBecause <textual reason>) if recompilation is required. checkDependencies :: HscEnv -> ModSummary -> ModIface -> IfG RecompileRequired checkDependencies hsc_env summary iface - = checkList (map dep_missing (ms_imps summary ++ ms_srcimps summary)) + = do + unseen_old_deps_ref <- liftIO $ newIORef old_deps + anyRecomp $ + [checkList (map dep_missing (ms_imps summary ++ ms_srcimps summary))] + ++ map + (checkForNewHomeDependency unseen_old_deps_ref) + (ms_home_imps summary) + ++ [checkIfAllOldHomeDependenciesAreSeen unseen_old_deps_ref] where prev_dep_mods = dep_mods (mi_deps iface) prev_dep_plgn = dep_plgins (mi_deps iface) @@ -1522,6 +1535,54 @@ checkDependencies hsc_env summary iface where pkg = moduleUnitId mod _otherwise -> return (RecompBecause reason) + old_deps = Set.fromList $ map fst $ filter (not . snd) prev_dep_mods + isOldHomeDeps = flip Set.member old_deps + checkForNewHomeDependency unseen_old_deps_ref (L _ mname) = do + let + mod = mkModule this_pkg mname + str_mname = moduleNameString mname + reason = str_mname ++ " changed" + -- We only want to look at home modules to check if any new home dependency + -- pops in and thus here, skip modules that are not home. Checking + -- membership in old home dependencies suffice because the `dep_missing` + -- check already verified that all imported home modules are present there + if not (isOldHomeDeps mname) then + return UpToDate + else + needInterface mod $ \imported_iface -> do + let mnames = mname:(map fst $ filter (not . snd) $ + dep_mods $ mi_deps imported_iface) + -- we also need to keep track of old dependencies that are no + -- longer present among our imported modules as their existence + -- also mean recompilation is needed + liftIO $ modifyIORef' unseen_old_deps_ref $ \unseen_old_deps -> + foldl' (flip Set.delete) unseen_old_deps mnames + case listToMaybe (filter (not . isOldHomeDeps) mnames) of + Nothing -> return UpToDate + Just new_dep_mname -> do + traceHiDiffs $ + text "imported home module " <> quotes (ppr mod) <> + text " has a new dependency " <> quotes (ppr new_dep_mname) + return $ RecompBecause reason + + checkIfAllOldHomeDependenciesAreSeen unseen_old_deps_ref = do + unseen_old_deps <- liftIO $ readIORef unseen_old_deps_ref + if not (null unseen_old_deps) + then do + let missing_dep = Set.elemAt 0 unseen_old_deps + traceHiDiffs $ + text "missing old home dependency " <> quotes (ppr missing_dep) + return $ RecompBecause "missing old depedency" + else return UpToDate + +anyRecomp :: [IfG RecompileRequired] -> IfG RecompileRequired +anyRecomp [] = return UpToDate +anyRecomp (x:xs) = do + recomp_reqd <- x + case recomp_reqd of + UpToDate -> anyRecomp xs + _ -> return recomp_reqd + needInterface :: Module -> (ModIface -> IfG RecompileRequired) -> IfG RecompileRequired needInterface mod continue diff --git a/compiler/main/GhcMake.hs b/compiler/main/GhcMake.hs index 8ed692ce54..b7f95bc8d8 100644 --- a/compiler/main/GhcMake.hs +++ b/compiler/main/GhcMake.hs @@ -2188,28 +2188,6 @@ msDeps s = concat [ [(m,IsBoot), (m,NotBoot)] | m <- ms_home_srcimps s ] ++ [ (m,NotBoot) | m <- ms_home_imps s ] -home_imps :: [(Maybe FastString, Located ModuleName)] -> [Located ModuleName] -home_imps imps = [ lmodname | (mb_pkg, lmodname) <- imps, - isLocal mb_pkg ] - where isLocal Nothing = True - isLocal (Just pkg) | pkg == fsLit "this" = True -- "this" is special - isLocal _ = False - -ms_home_allimps :: ModSummary -> [ModuleName] -ms_home_allimps ms = map unLoc (ms_home_srcimps ms ++ ms_home_imps ms) - --- | Like 'ms_home_imps', but for SOURCE imports. -ms_home_srcimps :: ModSummary -> [Located ModuleName] -ms_home_srcimps = home_imps . ms_srcimps - --- | All of the (possibly) home module imports from a --- 'ModSummary'; that is to say, each of these module names --- could be a home import if an appropriately named file --- existed. (This is in contrast to package qualified --- imports, which are guaranteed not to be home imports.) -ms_home_imps :: ModSummary -> [Located ModuleName] -ms_home_imps = home_imps . ms_imps - ----------------------------------------------------------------------------- -- Summarising modules diff --git a/compiler/main/HscTypes.hs b/compiler/main/HscTypes.hs index 3a4581fdc5..b70fb8ac95 100644 --- a/compiler/main/HscTypes.hs +++ b/compiler/main/HscTypes.hs @@ -32,7 +32,9 @@ module HscTypes ( ForeignSrcLang(..), phaseForeignLanguage, - ModSummary(..), ms_imps, ms_installed_mod, ms_mod_name, showModMsg, isBootSummary, + ModSummary(..), ms_imps, ms_installed_mod, ms_mod_name, ms_home_imps, + home_imps, ms_home_allimps, ms_home_srcimps, showModMsg, isBootSummary, + msHsFilePath, msHiFilePath, msObjFilePath, SourceModified(..), isTemplateHaskellOrQQNonBoot, moduleRetainsAllBindings, @@ -2803,6 +2805,28 @@ ms_imps ms = where mk_additional_import mod_nm = (Nothing, noLoc mod_nm) +home_imps :: [(Maybe FastString, Located ModuleName)] -> [Located ModuleName] +home_imps imps = [ lmodname | (mb_pkg, lmodname) <- imps, + isLocal mb_pkg ] + where isLocal Nothing = True + isLocal (Just pkg) | pkg == fsLit "this" = True -- "this" is special + isLocal _ = False + +ms_home_allimps :: ModSummary -> [ModuleName] +ms_home_allimps ms = map unLoc (ms_home_srcimps ms ++ ms_home_imps ms) + +-- | Like 'ms_home_imps', but for SOURCE imports. +ms_home_srcimps :: ModSummary -> [Located ModuleName] +ms_home_srcimps = home_imps . ms_srcimps + +-- | All of the (possibly) home module imports from a +-- 'ModSummary'; that is to say, each of these module names +-- could be a home import if an appropriately named file +-- existed. (This is in contrast to package qualified +-- imports, which are guaranteed not to be home imports.) +ms_home_imps :: ModSummary -> [Located ModuleName] +ms_home_imps = home_imps . ms_imps + -- The ModLocation contains both the original source filename and the -- filename of the cleaned-up source file after all preprocessing has been -- done. The point is that the summariser will have to cpp/unlit/whatever |