[commit: ghc] master: Fix #15594 (--abi-hash with Backpack sometimes fails) (13ff0b7)
git at git.haskell.org
git at git.haskell.org
Mon Nov 12 03:40:02 UTC 2018
Repository : ssh://git@git.haskell.org/ghc
On branch : master
Link : http://ghc.haskell.org/trac/ghc/changeset/13ff0b7ced097286e0d7b054f050871effe07f86/ghc
>---------------------------------------------------------------
commit 13ff0b7ced097286e0d7b054f050871effe07f86
Author: Edward Z. Yang <ezyang at fb.com>
Date: Sun Nov 11 22:39:29 2018 -0500
Fix #15594 (--abi-hash with Backpack sometimes fails)
Summary:
For holes, its necessary to "see through" the instantiation
of the hole to get accurate family instance dependencies.
For example, if B imports <A>, and <A> is instantiated with
F, we must grab and include all of the dep_finsts from
F to have an accurate transitive dep_finsts list.
However, we MUST NOT do this for regular modules.
First, for efficiency reasons, doing this
bloats the the dep_finsts list, because we *already* had
those modules in the list (it wasn't a hole module, after
all). But there's a second, more important correctness
consideration: we perform module renaming when running
--abi-hash. In this case, GHC's contract to the user is that
it will NOT go and read out interfaces of any dependencies
(https://github.com/haskell/cabal/issues/3633); the point of
--abi-hash is just to get a hash of the on-disk interfaces
for this *specific* package. If we go off and tug on the
interface for /everything/ in dep_finsts, we're gonna have a
bad time. (It's safe to do do this for hole modules, though,
because the hmap for --abi-hash is always trivial, so the
interface we request is local. Though, maybe we ought
not to do it in this case either...)
Signed-off-by: Edward Z. Yang <ezyang at fb.com>
Test Plan: validate
Reviewers: alexbiehl, goldfire, bgamari
Subscribers: ppk, shlevy, rwbarton, carter
GHC Trac Issues: #15594
Differential Revision: https://phabricator.haskell.org/D5123
>---------------------------------------------------------------
13ff0b7ced097286e0d7b054f050871effe07f86
compiler/backpack/RnModIface.hs | 32 ++++++++++++++++++++--
.../backpack/cabal/{bkpcabal07 => T15594}/Makefile | 10 +++----
.../backpack/cabal/{T14304 => T15594}/Setup.hs | 0
.../cabal/{T14304/indef => T15594}/Sig.hsig | 3 +-
testsuite/tests/backpack/cabal/T15594/Stuff.hs | 10 +++++++
testsuite/tests/backpack/cabal/T15594/all.T | 9 ++++++
testsuite/tests/backpack/cabal/T15594/pkg.cabal | 19 +++++++++++++
testsuite/tests/backpack/cabal/T15594/src/Lib.hs | 7 +++++
8 files changed, 80 insertions(+), 10 deletions(-)
diff --git a/compiler/backpack/RnModIface.hs b/compiler/backpack/RnModIface.hs
index 3ae01d7..896303b 100644
--- a/compiler/backpack/RnModIface.hs
+++ b/compiler/backpack/RnModIface.hs
@@ -138,10 +138,36 @@ rnDepModules sel deps = do
-- in these dependencies.
fmap (nubSort . concat) . T.forM (sel deps) $ \mod -> do
dflags <- getDynFlags
+ -- For holes, its necessary to "see through" the instantiation
+ -- of the hole to get accurate family instance dependencies.
+ -- For example, if B imports <A>, and <A> is instantiated with
+ -- F, we must grab and include all of the dep_finsts from
+ -- F to have an accurate transitive dep_finsts list.
+ --
+ -- However, we MUST NOT do this for regular modules.
+ -- First, for efficiency reasons, doing this
+ -- bloats the the dep_finsts list, because we *already* had
+ -- those modules in the list (it wasn't a hole module, after
+ -- all). But there's a second, more important correctness
+ -- consideration: we perform module renaming when running
+ -- --abi-hash. In this case, GHC's contract to the user is that
+ -- it will NOT go and read out interfaces of any dependencies
+ -- (https://github.com/haskell/cabal/issues/3633); the point of
+ -- --abi-hash is just to get a hash of the on-disk interfaces
+ -- for this *specific* package. If we go off and tug on the
+ -- interface for /everything/ in dep_finsts, we're gonna have a
+ -- bad time. (It's safe to do do this for hole modules, though,
+ -- because the hmap for --abi-hash is always trivial, so the
+ -- interface we request is local. Though, maybe we ought
+ -- not to do it in this case either...)
+ --
+ -- This mistake was bug #15594.
let mod' = renameHoleModule dflags hmap mod
- iface <- liftIO . initIfaceCheck (text "rnDepModule") hsc_env
- $ loadSysInterface (text "rnDepModule") mod'
- return (mod' : sel (mi_deps iface))
+ if isHoleModule mod
+ then do iface <- liftIO . initIfaceCheck (text "rnDepModule") hsc_env
+ $ loadSysInterface (text "rnDepModule") mod'
+ return (mod' : sel (mi_deps iface))
+ else return [mod']
{-
************************************************************************
diff --git a/testsuite/tests/backpack/cabal/bkpcabal07/Makefile b/testsuite/tests/backpack/cabal/T15594/Makefile
similarity index 73%
copy from testsuite/tests/backpack/cabal/bkpcabal07/Makefile
copy to testsuite/tests/backpack/cabal/T15594/Makefile
index 346ac32..57ef67f 100644
--- a/testsuite/tests/backpack/cabal/bkpcabal07/Makefile
+++ b/testsuite/tests/backpack/cabal/T15594/Makefile
@@ -1,19 +1,17 @@
-TOP=../../../..
+TOP=/home/ezyang/Dev/ghc-known-nat/testsuite
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'
-# This test checks that instantiating an indefinite package
-# with a wired in package works.
-
-bkpcabal07: clean
- $(MAKE) -s --no-print-directory clean
+T15594: clean
'$(GHC_PKG)' init tmp.d
'$(TEST_HC)' $(TEST_HC_OPTS) -v0 --make Setup
$(CONFIGURE)
$(SETUP) build
+ $(SETUP) copy
+ $(SETUP) register
ifneq "$(CLEANUP)" ""
$(MAKE) -s --no-print-directory clean
endif
diff --git a/testsuite/tests/backpack/cabal/T14304/Setup.hs b/testsuite/tests/backpack/cabal/T15594/Setup.hs
similarity index 100%
copy from testsuite/tests/backpack/cabal/T14304/Setup.hs
copy to testsuite/tests/backpack/cabal/T15594/Setup.hs
diff --git a/testsuite/tests/backpack/cabal/T14304/indef/Sig.hsig b/testsuite/tests/backpack/cabal/T15594/Sig.hsig
similarity index 57%
copy from testsuite/tests/backpack/cabal/T14304/indef/Sig.hsig
copy to testsuite/tests/backpack/cabal/T15594/Sig.hsig
index a37b190..1342a7b 100644
--- a/testsuite/tests/backpack/cabal/T14304/indef/Sig.hsig
+++ b/testsuite/tests/backpack/cabal/T15594/Sig.hsig
@@ -1,2 +1,3 @@
signature Sig where
-data B
+
+foo :: String
diff --git a/testsuite/tests/backpack/cabal/T15594/Stuff.hs b/testsuite/tests/backpack/cabal/T15594/Stuff.hs
new file mode 100644
index 0000000..053949b
--- /dev/null
+++ b/testsuite/tests/backpack/cabal/T15594/Stuff.hs
@@ -0,0 +1,10 @@
+{-# LANGUAGE TypeFamilies #-}
+module Stuff where
+
+data family T a
+
+data instance T Int = T Int
+
+test :: String
+test =
+ "test"
diff --git a/testsuite/tests/backpack/cabal/T15594/all.T b/testsuite/tests/backpack/cabal/T15594/all.T
new file mode 100644
index 0000000..1978865
--- /dev/null
+++ b/testsuite/tests/backpack/cabal/T15594/all.T
@@ -0,0 +1,9 @@
+if config.cleanup:
+ cleanup = 'CLEANUP=1'
+else:
+ cleanup = 'CLEANUP=0'
+
+test('T15594',
+ extra_files(['Setup.hs', 'Stuff.hs', 'Sig.hsig', 'pkg.cabal', 'src']),
+ run_command,
+ ['$MAKE -s --no-print-directory T15594 ' + cleanup])
diff --git a/testsuite/tests/backpack/cabal/T15594/pkg.cabal b/testsuite/tests/backpack/cabal/T15594/pkg.cabal
new file mode 100644
index 0000000..cf6fdda
--- /dev/null
+++ b/testsuite/tests/backpack/cabal/T15594/pkg.cabal
@@ -0,0 +1,19 @@
+cabal-version: 2.0
+name: backpack-trans
+version: 0.1.0.0
+license: BSD3
+author: Alex Biehl
+maintainer: alex.biehl at target.com
+build-type: Simple
+
+library indef
+ signatures: Sig
+ exposed-modules: Stuff
+ build-depends: base
+ default-language: Haskell2010
+
+library
+ exposed-modules: Lib
+ build-depends: base, indef
+ default-language: Haskell2010
+ hs-source-dirs: src
diff --git a/testsuite/tests/backpack/cabal/T15594/src/Lib.hs b/testsuite/tests/backpack/cabal/T15594/src/Lib.hs
new file mode 100644
index 0000000..73cc27c
--- /dev/null
+++ b/testsuite/tests/backpack/cabal/T15594/src/Lib.hs
@@ -0,0 +1,7 @@
+module Lib where
+
+import Stuff
+
+doSomeStuff :: String
+doSomeStuff =
+ test
More information about the ghc-commits
mailing list