Fix recompilation detection when set of signatures to merge changes.
authorEdward Z. Yang <ezyang@cs.stanford.edu>
Mon, 12 Dec 2016 03:42:29 +0000 (19:42 -0800)
committerEdward Z. Yang <ezyang@cs.stanford.edu>
Tue, 13 Dec 2016 07:11:08 +0000 (23:11 -0800)
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

15 files changed:
compiler/basicTypes/Module.hs
compiler/iface/MkIface.hs
testsuite/driver/extra_files.py
testsuite/tests/backpack/cabal/bkpcabal03/.gitignore [new file with mode: 0644]
testsuite/tests/backpack/cabal/bkpcabal03/Makefile [new file with mode: 0644]
testsuite/tests/backpack/cabal/bkpcabal03/Mod.hs [new file with mode: 0644]
testsuite/tests/backpack/cabal/bkpcabal03/Setup.hs [new file with mode: 0644]
testsuite/tests/backpack/cabal/bkpcabal03/all.T [new file with mode: 0644]
testsuite/tests/backpack/cabal/bkpcabal03/asig1/A.hsig [new file with mode: 0644]
testsuite/tests/backpack/cabal/bkpcabal03/asig1/asig1.cabal [new file with mode: 0644]
testsuite/tests/backpack/cabal/bkpcabal03/asig2/A.hsig [new file with mode: 0644]
testsuite/tests/backpack/cabal/bkpcabal03/asig2/asig2.cabal [new file with mode: 0644]
testsuite/tests/backpack/cabal/bkpcabal03/bkpcabal03.cabal.in1 [new file with mode: 0644]
testsuite/tests/backpack/cabal/bkpcabal03/bkpcabal03.cabal.in2 [new file with mode: 0644]
testsuite/tests/backpack/cabal/bkpcabal03/bkpcabal03.stderr [new file with mode: 0644]

index 75ff67d..e076580 100644 (file)
@@ -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
index 4d45efd..bdc9f0f 100644 (file)
@@ -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
index fcb6ea0..f151d75 100644 (file)
@@ -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'],
diff --git a/testsuite/tests/backpack/cabal/bkpcabal03/.gitignore b/testsuite/tests/backpack/cabal/bkpcabal03/.gitignore
new file mode 100644 (file)
index 0000000..11927ba
--- /dev/null
@@ -0,0 +1 @@
+bkpcabal03.cabal
diff --git a/testsuite/tests/backpack/cabal/bkpcabal03/Makefile b/testsuite/tests/backpack/cabal/bkpcabal03/Makefile
new file mode 100644 (file)
index 0000000..d5c39e4
--- /dev/null
@@ -0,0 +1,36 @@
+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)
diff --git a/testsuite/tests/backpack/cabal/bkpcabal03/Mod.hs b/testsuite/tests/backpack/cabal/bkpcabal03/Mod.hs
new file mode 100644 (file)
index 0000000..a0773b1
--- /dev/null
@@ -0,0 +1,4 @@
+module Foo where
+import A
+h :: Int
+h = g
diff --git a/testsuite/tests/backpack/cabal/bkpcabal03/Setup.hs b/testsuite/tests/backpack/cabal/bkpcabal03/Setup.hs
new file mode 100644 (file)
index 0000000..9a994af
--- /dev/null
@@ -0,0 +1,2 @@
+import Distribution.Simple
+main = defaultMain
diff --git a/testsuite/tests/backpack/cabal/bkpcabal03/all.T b/testsuite/tests/backpack/cabal/bkpcabal03/all.T
new file mode 100644 (file)
index 0000000..7a5ad44
--- /dev/null
@@ -0,0 +1,9 @@
+if config.cleanup:
+   cleanup = 'CLEANUP=1'
+else:
+   cleanup = 'CLEANUP=0'
+
+test('bkpcabal03',
+     normal,
+     run_command,
+     ['$MAKE -s --no-print-directory bkpcabal03 ' + cleanup])
diff --git a/testsuite/tests/backpack/cabal/bkpcabal03/asig1/A.hsig b/testsuite/tests/backpack/cabal/bkpcabal03/asig1/A.hsig
new file mode 100644 (file)
index 0000000..538d024
--- /dev/null
@@ -0,0 +1,2 @@
+signature A where
+f :: Int
diff --git a/testsuite/tests/backpack/cabal/bkpcabal03/asig1/asig1.cabal b/testsuite/tests/backpack/cabal/bkpcabal03/asig1/asig1.cabal
new file mode 100644 (file)
index 0000000..f45c925
--- /dev/null
@@ -0,0 +1,12 @@
+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
diff --git a/testsuite/tests/backpack/cabal/bkpcabal03/asig2/A.hsig b/testsuite/tests/backpack/cabal/bkpcabal03/asig2/A.hsig
new file mode 100644 (file)
index 0000000..57eb763
--- /dev/null
@@ -0,0 +1,2 @@
+signature A where
+g :: Int
diff --git a/testsuite/tests/backpack/cabal/bkpcabal03/asig2/asig2.cabal b/testsuite/tests/backpack/cabal/bkpcabal03/asig2/asig2.cabal
new file mode 100644 (file)
index 0000000..1d802d8
--- /dev/null
@@ -0,0 +1,12 @@
+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
diff --git a/testsuite/tests/backpack/cabal/bkpcabal03/bkpcabal03.cabal.in1 b/testsuite/tests/backpack/cabal/bkpcabal03/bkpcabal03.cabal.in1
new file mode 100644 (file)
index 0000000..689a7c8
--- /dev/null
@@ -0,0 +1,12 @@
+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
diff --git a/testsuite/tests/backpack/cabal/bkpcabal03/bkpcabal03.cabal.in2 b/testsuite/tests/backpack/cabal/bkpcabal03/bkpcabal03.cabal.in2
new file mode 100644 (file)
index 0000000..a25c7fe
--- /dev/null
@@ -0,0 +1,12 @@
+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
diff --git a/testsuite/tests/backpack/cabal/bkpcabal03/bkpcabal03.stderr b/testsuite/tests/backpack/cabal/bkpcabal03/bkpcabal03.stderr
new file mode 100644 (file)
index 0000000..f4bd94f
--- /dev/null
@@ -0,0 +1,2 @@
+
+Mod.hs:4:5: error: Variable not in scope: g :: Int