Fix recompilation detection when set of signatures to merge changes.

Summary:
Previously, we only checked to recompile if a signature we
previously depended on changed; however, if the -unit-id
settings changed, this could have resulted in more or less
signatures needing to be merged in; we weren't checking
for this case.

(Note that this logic is irrelevant for normal module imports,
which we also check using -unit-id, as we record each import
and redo it, forcing a recompile if the result changed.)
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>

Test Plan: validate

Reviewers: simonpj, austin, bgamari

Subscribers: thomie

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

compiler/basicTypes/Module.hs

@@ -34,6 +34,8 @@ module Module
         unitIdKey,
         IndefUnitId(..),
         IndefModule(..),
+        indefUnitIdToUnitId,
+        indefModuleToModule,
         InstalledUnitId(..),
         toInstalledUnitId,
         ShHoleSubst,
@@ -619,6 +621,20 @@ newIndefUnitId cid insts =
      fs = hashUnitId cid sorted_insts
      sorted_insts = sortBy (stableModuleNameCmp `on` fst) insts
 
+-- | Injects an 'IndefUnitId' (indefinite library which
+-- was on-the-fly instantiated) to a 'UnitId' (either
+-- an indefinite or definite library).
+indefUnitIdToUnitId :: DynFlags -> IndefUnitId -> UnitId
+indefUnitIdToUnitId dflags iuid =
+    -- NB: suppose that we want to compare the indefinite
+    -- unit id p[H=impl:H] against p+abcd (where p+abcd
+    -- happens to be the existing, installed version of
+    -- p[H=impl:H].  If we *only* wrap in p[H=impl:H]
+    -- IndefiniteUnitId, they won't compare equal; only
+    -- after improvement will the equality hold.
+    improveUnitId (getPackageConfigMap dflags) $
+        IndefiniteUnitId iuid
+
 data IndefModule = IndefModule {
         indefModuleUnitId :: IndefUnitId,
         indefModuleName   :: ModuleName
@@ -628,6 +644,12 @@ instance Outputable IndefModule where
   ppr (IndefModule uid m) =
     ppr uid <> char ':' <> ppr m
 
+-- | Injects an 'IndefModule' to 'Module' (see also
+-- 'indefUnitIdToUnitId'.
+indefModuleToModule :: DynFlags -> IndefModule -> Module
+indefModuleToModule dflags (IndefModule iuid mod_name) =
+    mkModule (indefUnitIdToUnitId dflags iuid) mod_name
+
 -- | An installed unit identifier identifies a library which has
 -- been installed to the package database.  These strings are
 -- provided to us via the @-this-unit-id@ flag.  The library

compiler/iface/MkIface.hs

@@ -107,6 +107,7 @@ import Fingerprint
 import Exception
 import UniqFM
 import UniqDFM
+import Packages
 
 import Control.Monad
 import Data.Function
@@ -1172,6 +1173,8 @@ checkVersions hsc_env mod_summary iface
 
        ; recomp <- checkFlagHash hsc_env iface
        ; if recompileRequired recomp then return (recomp, Nothing) else do {
+       ; recomp <- checkMergedSignatures mod_summary iface
+       ; if recompileRequired recomp then return (recomp, Nothing) else do {
        ; recomp <- checkHsig mod_summary iface
        ; if recompileRequired recomp then return (recomp, Nothing) else do {
        ; recomp <- checkDependencies hsc_env mod_summary iface
@@ -1193,7 +1196,7 @@ checkVersions hsc_env mod_summary iface
        ; updateEps_ $ \eps  -> eps { eps_is_boot = udfmToUfm mod_deps }
        ; recomp <- checkList [checkModUsage this_pkg u | u <- mi_usages iface]
        ; return (recomp, Just iface)
-    }}}}
+    }}}}}
   where
     this_pkg = thisPackage (hsc_dflags hsc_env)
     -- This is a bit of a hack really
@@ -1225,6 +1228,20 @@ checkFlagHash hsc_env iface = do
                      (text "  Module flags have changed")
                      old_hash new_hash
 
+-- Check that the set of signatures we are merging in match.
+-- If the -unit-id flags change, this can change too.
+checkMergedSignatures :: ModSummary -> ModIface -> IfG RecompileRequired
+checkMergedSignatures mod_summary iface = do
+    dflags <- getDynFlags
+    let old_merged = sort [ mod | UsageMergedRequirement{ usg_mod = mod } <- mi_usages iface ]
+        new_merged = case Map.lookup (ms_mod_name mod_summary)
+                                     (requirementContext (pkgState dflags)) of
+                        Nothing -> []
+                        Just r -> sort $ map (indefModuleToModule dflags) r
+    if old_merged == new_merged
+        then up_to_date (text "signatures to merge in unchanged" $$ ppr new_merged)
+        else return (RecompBecause "signatures to merge in changed")
+
 -- If the direct imports of this module are resolved to targets that
 -- are not among the dependencies of the previous interface file,
 -- then we definitely need to recompile.  This catches cases like

testsuite/driver/extra_files.py

@@ -156,6 +156,7 @@ extra_src_files = {
   'boolFormula': ['TestBoolFormula.hs'],
   'bkpcabal01': ['p', 'q', 'impl', 'bkpcabal01.cabal', 'Setup.hs', 'Main.hs'],
   'bkpcabal02': ['p', 'q', 'bkpcabal02.cabal', 'Setup.hs'],
+  'bkpcabal03': ['asig1', 'asig2', 'bkpcabal03.cabal.in1', 'bkpcabal03.cabal.in2', 'Setup.hs', 'Mod.hs'],
   'break001': ['../Test2.hs'],
   'break002': ['../Test2.hs'],
   'break003': ['../Test3.hs'],

testsuite/tests/backpack/cabal/bkpcabal03/.gitignore

+bkpcabal03.cabal

testsuite/tests/backpack/cabal/bkpcabal03/Makefile

+TOP=../../../..
+include $(TOP)/mk/boilerplate.mk
+include $(TOP)/mk/test.mk
+
+SETUP='$(PWD)/Setup' -v0
+CONFIGURE=$(SETUP) configure $(CABAL_MINIMAL_BUILD) --with-ghc='$(TEST_HC)' --ghc-options='$(TEST_HC_OPTS)' --package-db='$(PWD)/tmp.d' --prefix='$(PWD)/inst'
+
+bkpcabal03: clean
+	$(MAKE) -s --no-print-directory clean
+	'$(GHC_PKG)' init tmp.d
+	'$(TEST_HC)' -v0 --make Setup
+	cp bkpcabal03.cabal.in1 bkpcabal03.cabal
+	# typecheck asig1
+	(cd asig1; $(CONFIGURE) --cid "asig1" asig1)
+	(cd asig1; $(SETUP) build)
+	(cd asig1; $(SETUP) copy)
+	(cd asig1; $(SETUP) register)
+	# typecheck asig2
+	(cd asig2; $(CONFIGURE) --cid "asig2" asig2)
+	(cd asig2; $(SETUP) build)
+	(cd asig2; $(SETUP) copy)
+	(cd asig2; $(SETUP) register)
+	# typecheck top-level
+	$(CONFIGURE) --cid "toplevel" bkpcabal03
+	! $(SETUP) build
+	# modify mixins
+	cp bkpcabal03.cabal.in2 bkpcabal03.cabal
+	# retypecheck top-level
+	$(CONFIGURE) --cid "toplevel" bkpcabal03
+	$(SETUP) build
+ifneq "$(CLEANUP)" ""
+	$(MAKE) -s --no-print-directory clean
+endif
+
+clean :
+	$(RM) -r tmp.d inst dist Setup$(exeext)

testsuite/tests/backpack/cabal/bkpcabal03/Mod.hs

+module Foo where
+import A
+h :: Int
+h = g

testsuite/tests/backpack/cabal/bkpcabal03/Setup.hs

+import Distribution.Simple
+main = defaultMain

testsuite/tests/backpack/cabal/bkpcabal03/all.T

+if config.cleanup:
+   cleanup = 'CLEANUP=1'
+else:
+   cleanup = 'CLEANUP=0'
+
+test('bkpcabal03',
+     normal,
+     run_command,
+     ['$MAKE -s --no-print-directory bkpcabal03 ' + cleanup])

testsuite/tests/backpack/cabal/bkpcabal03/asig1/A.hsig

+signature A where
+f :: Int

testsuite/tests/backpack/cabal/bkpcabal03/asig1/asig1.cabal

+name:                asig1
+version:             0.1.0.0
+license:             BSD3
+author:              Edward Z. Yang
+maintainer:          ezyang@cs.stanford.edu
+build-type:          Simple
+cabal-version:       >=1.25
+
+library
+  build-depends: base
+  signatures: A
+  default-language: Haskell2010

testsuite/tests/backpack/cabal/bkpcabal03/asig2/A.hsig

+signature A where
+g :: Int

testsuite/tests/backpack/cabal/bkpcabal03/asig2/asig2.cabal

+name:                asig2
+version:             0.1.0.0
+license:             BSD3
+author:              Edward Z. Yang
+maintainer:          ezyang@cs.stanford.edu
+build-type:          Simple
+cabal-version:       >=1.25
+
+library
+  build-depends: base
+  signatures: A
+  default-language: Haskell2010

testsuite/tests/backpack/cabal/bkpcabal03/bkpcabal03.cabal.in1

+name:                bkpcabal03
+version:             0.1.0.0
+license:             BSD3
+author:              Edward Z. Yang
+maintainer:          ezyang@cs.stanford.edu
+build-type:          Simple
+cabal-version:       >=1.25
+
+library
+  build-depends: asig1, base
+  exposed-modules: Mod
+  default-language: Haskell2010

testsuite/tests/backpack/cabal/bkpcabal03/bkpcabal03.cabal.in2

+name:                bkpcabal03
+version:             0.1.0.0
+license:             BSD3
+author:              Edward Z. Yang
+maintainer:          ezyang@cs.stanford.edu
+build-type:          Simple
+cabal-version:       >=1.25
+
+library
+  build-depends: asig1, asig2, base
+  exposed-modules: Mod
+  default-language: Haskell2010

testsuite/tests/backpack/cabal/bkpcabal03/bkpcabal03.stderr

+
+Mod.hs:4:5: error: Variable not in scope: g :: Int