Commit 8fa2cdb1 authored by Edward Z. Yang's avatar Edward Z. Yang
Browse files

Track dep_finsts in exports hash, as it affects downstream deps.



Summary:
I also added some more comments about the orphan and family instance
hashing business.

Fixes #12723.
Signed-off-by: default avatarEdward Z. Yang <ezyang@cs.stanford.edu>

Test Plan: validate

Reviewers: bgamari, austin

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D2607

GHC Trac Issues: #12723
parent cf5eec3e
......@@ -569,6 +569,7 @@ addFingerprints hsc_env mb_old_fingerprint iface0 new_decls
-- tracked by the usage on the ABI hash of package modules that we import.
let orph_mods
= filter (/= this_mod) -- Note [Do not update EPS with your own hi-boot]
-- TODO: the line below is not correct, see #12733
. filter ((== this_pkg) . moduleUnitId)
$ dep_orphs sorted_deps
dep_orphan_hashes <- getOrphanHashes hsc_env orph_mods
......@@ -592,11 +593,41 @@ addFingerprints hsc_env mb_old_fingerprint iface0 new_decls
orphan_hash,
dep_orphan_hashes,
dep_pkgs (mi_deps iface0),
-- See Note [Export hash depends on non-orphan family instances]
dep_finsts (mi_deps iface0),
-- dep_pkgs: see "Package Version Changes" on
-- wiki/Commentary/Compiler/RecompilationAvoidance
mi_trust iface0)
-- Make sure change of Safe Haskell mode causes recomp.
-- Note [Export hash depends on non-orphan family instances]
-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
-- Suppose we have:
--
-- module A where
-- type instance F Int = Bool
--
-- module B where
-- import A
--
-- module C where
-- import B
--
-- The family instance consistency check for C depends on the dep_finsts of
-- B. If we rename module A to A2, when the dep_finsts of B changes, we need
-- to make sure that C gets rebuilt. Effectively, the dep_finsts are part of
-- the exports of B, because C always considers them when checking
-- consistency.
--
-- A full discussion is in #12723.
--
-- We do NOT need to hash dep_orphs, because this is implied by
-- dep_orphan_hashes, and we do not need to hash ordinary class instances,
-- because there is no eager consistency check as there is with type families
-- (also we didn't store it anywhere!)
--
-- put the declarations in a canonical order, sorted by OccName
let sorted_decls = Map.elems $ Map.fromList $
[(getOccName d, e) | e@(_, d) <- decls_w_hashes]
......@@ -664,6 +695,36 @@ addFingerprints hsc_env mb_old_fingerprint iface0 new_decls
fix_fn = mi_fix_fn iface0
ann_fn = mkIfaceAnnCache (mi_anns iface0)
-- | Retrieve the orphan hashes 'mi_orphan_hash' for a list of modules
-- (in particular, the orphan modules which are transitively imported by the
-- current module).
--
-- Q: Why do we need the hash at all, doesn't the list of transitively
-- imported orphan modules suffice?
--
-- A: If one of our transitive imports adds a new orphan instance, our
-- export hash must change so that modules which import us rebuild. If we just
-- hashed the [Module], the hash would not change even when a new instance was
-- added to a module that already had an orphan instance.
--
-- Q: Why don't we just hash the orphan hashes of our direct dependencies?
-- Why the full transitive closure?
--
-- A: Suppose we have these modules:
--
-- module A where
-- instance Show (a -> b) where
-- module B where
-- import A -- **
-- module C where
-- import A
-- import B
--
-- Whether or not we add or remove the import to A in B affects the
-- orphan hash of B. But it shouldn't really affect the orphan hash
-- of C. If we hashed only direct dependencies, there would be no
-- way to tell that the net effect was a wash, and we'd be forced
-- to recompile C and everything else.
getOrphanHashes :: HscEnv -> [Module] -> IO [Fingerprint]
getOrphanHashes hsc_env mods = do
eps <- hscEPS hsc_env
......
......@@ -2258,10 +2258,13 @@ data Dependencies
-- instance orphans as they are anyway included in
-- 'dep_finsts'. But then be careful about code
-- which relies on dep_orphs having the complete list!)
-- This does NOT include us, unlike 'imp_orphs'.
, dep_finsts :: [Module]
-- ^ Modules that contain family instances (whether the
-- instances are from the home or an external package)
-- ^ Transitive closure of depended upon modules which
-- contain family instances (whether home or external).
-- This is used by 'checkFamInstConsistency'. This
-- does NOT include us, unlike 'imp_finsts'.
}
deriving( Eq )
-- Equality used only for old/new comparison in MkIface.addFingerprints
......
......@@ -350,6 +350,11 @@ calculateAvails dflags iface mod_safe' want_boot =
-- Compute new transitive dependencies
--
-- 'dep_orphs' and 'dep_finsts' do NOT include the imported module
-- itself, but we DO need to include this module in 'imp_orphs' and
-- 'imp_finsts' if it defines an orphan or instance family; thus the
-- orph_iface/has_iface tests.
orphans | orph_iface = ASSERT( not (imp_mod `elem` dep_orphs deps) )
imp_mod : dep_orphs deps
......
......@@ -169,8 +169,19 @@ See Note [Unique Determinism] in Unique
listToSet :: [ModulePair] -> ModulePairSet
listToSet l = Set.fromList l
-- | Check family instance consistency, given:
--
-- 1. The list of all modules transitively imported by us
-- which define a family instance (these are the ones
-- we have to check for consistency), and
--
-- 2. The list of modules which we directly imported
-- (these specify the sets of family instance defining
-- modules which are already known to be consistent).
--
-- See Note [Checking family instance consistency] for more
-- details.
checkFamInstConsistency :: [Module] -> [Module] -> TcM ()
-- See Note [Checking family instance consistency]
checkFamInstConsistency famInstMods directlyImpMods
= do { dflags <- getDynFlags
; (eps, hpt) <- getEpsAndHpt
......
......@@ -480,6 +480,7 @@ extra_src_files = {
'recomp010': ['Main.hs', 'X1.hs', 'X2.hs'],
'recomp011': ['Main.hs'],
'recomp015': ['Generate.hs'],
'recomp016': ['A.hs', 'A2.hs', 'C.hs', 'D.hs', 'E.hs'],
'record_upd': ['Main.hs'],
'rename.prog001': ['Rn037Help.hs', 'rn037.hs'],
'rename.prog002': ['Rn037Help.hs', 'rnfail037.hs'],
......
{-# LANGUAGE TypeFamilies #-}
module A where
type family F a
type instance F Int = Bool
{-# LANGUAGE TypeFamilies #-}
module A2 where
type family F a
type instance F Int = Bool
module E where
import D
import B
TOP=../../..
include $(TOP)/mk/boilerplate.mk
include $(TOP)/mk/test.mk
# Recompilation tests
clean:
rm -f *.o *.hi
# bug #12723
recomp016: clean
echo 'module B (module A) where import A' > B.hs
echo 'first run'
'$(TEST_HC)' $(TEST_HC_OPTS) --make E.hs
sleep 1
echo 'module B (module A2) where import A2' > B.hs
echo 'second run'
'$(TEST_HC)' $(TEST_HC_OPTS) --make E.hs
# Test for #12723, a recompilation bug
test('recomp016',
[ clean_cmd('$MAKE -s clean') ],
run_command,
['$MAKE -s --no-print-directory recomp016'])
first run
[1 of 5] Compiling A ( A.hs, A.o )
[2 of 5] Compiling B ( B.hs, B.o )
[3 of 5] Compiling C ( C.hs, C.o )
[4 of 5] Compiling D ( D.hs, D.o )
[5 of 5] Compiling E ( E.hs, E.o )
second run
[1 of 5] Compiling A2 ( A2.hs, A2.o )
[2 of 5] Compiling B ( B.hs, B.o )
[3 of 5] Compiling C ( C.hs, C.o ) [B changed]
[4 of 5] Compiling D ( D.hs, D.o ) [C changed]
[5 of 5] Compiling E ( E.hs, E.o ) [B changed]
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment