summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPhuong Trinh <lolotp@fb.com>2019-05-31 11:46:39 +0100
committerRay Shih <rayshih@fb.com>2020-07-27 09:45:14 -0700
commit0c70208868f43e44f11b84de051fa393aacf6270 (patch)
tree7e7b4734d11932637ae2ceb88b31bd15b6f98694
parent9c93a63fe700f6c98c6dbaa6977946571abfef2e (diff)
downloadhaskell-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.hs63
-rw-r--r--compiler/main/GhcMake.hs22
-rw-r--r--compiler/main/HscTypes.hs26
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