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

Improvements/bugfixes to signature reexport handling.



Summary:
A number of changes:

- Keep the TcGblEnv from typechecking the local signature
  around when we do merging.  In particular, we setup
  tcg_imports and tcg_rdr_env according to the local
  signature.  This improves our error output (for example,
  see bkpfail04) and also fixes a bug with reexporting
  modules in signatures (see bkpreex07)

- Fix a bug in thinning, where if we had signature A(module A),
  this previously would have *thinned out* all of the inherited
  signatures.  Now we treat every inherited signature as having
  come from an import like "import A", so a module A reexport
  will pick them up.

- Recompilation checking now keeps track of dependent source files
  of the source signature; previously we forgot to retain this
  info.

There's a manual update too.
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/D3133
parent 22dba98f
...@@ -420,7 +420,7 @@ hscTypecheck keep_rn mod_summary mb_rdr_module = do ...@@ -420,7 +420,7 @@ hscTypecheck keep_rn mod_summary mb_rdr_module = do
if hsc_src == HsigFile if hsc_src == HsigFile
then do (iface, _, _) <- liftIO $ hscSimpleIface hsc_env tc_result0 Nothing then do (iface, _, _) <- liftIO $ hscSimpleIface hsc_env tc_result0 Nothing
ioMsgMaybe $ ioMsgMaybe $
tcRnMergeSignatures hsc_env (tcg_top_loc tc_result0) hpm iface tcRnMergeSignatures hsc_env hpm tc_result0 iface
else return tc_result0 else return tc_result0
-- wrapper around tcRnModule to handle safe haskell extras -- wrapper around tcRnModule to handle safe haskell extras
......
...@@ -343,17 +343,18 @@ tcRnCheckUnitId hsc_env uid = ...@@ -343,17 +343,18 @@ tcRnCheckUnitId hsc_env uid =
-- | Top-level driver for signature merging (run after typechecking -- | Top-level driver for signature merging (run after typechecking
-- an @hsig@ file). -- an @hsig@ file).
tcRnMergeSignatures :: HscEnv -> RealSrcSpan -> HsParsedModule -> ModIface tcRnMergeSignatures :: HscEnv -> HsParsedModule -> TcGblEnv {- from local sig -} -> ModIface
-> IO (Messages, Maybe TcGblEnv) -> IO (Messages, Maybe TcGblEnv)
tcRnMergeSignatures hsc_env real_loc hsmod iface = tcRnMergeSignatures hsc_env hpm orig_tcg_env iface =
withTiming (pure dflags) withTiming (pure dflags)
(text "Signature merging" <+> brackets (ppr this_mod)) (text "Signature merging" <+> brackets (ppr this_mod))
(const ()) $ (const ()) $
initTc hsc_env HsigFile False this_mod real_loc $ initTc hsc_env HsigFile False this_mod real_loc $
mergeSignatures hsmod iface mergeSignatures hpm orig_tcg_env iface
where where
dflags = hsc_dflags hsc_env dflags = hsc_dflags hsc_env
this_mod = mi_module iface this_mod = mi_module iface
real_loc = tcg_top_loc orig_tcg_env
thinModIface :: [AvailInfo] -> ModIface -> ModIface thinModIface :: [AvailInfo] -> ModIface -> ModIface
thinModIface avails iface = thinModIface avails iface =
...@@ -484,8 +485,11 @@ merge_msg mod_name reqs = ...@@ -484,8 +485,11 @@ merge_msg mod_name reqs =
-- from 'requirementMerges' into this signature, producing -- from 'requirementMerges' into this signature, producing
-- a final 'TcGblEnv' that matches the local signature and -- a final 'TcGblEnv' that matches the local signature and
-- all required signatures. -- all required signatures.
mergeSignatures :: HsParsedModule -> ModIface -> TcRn TcGblEnv mergeSignatures :: HsParsedModule -> TcGblEnv -> ModIface -> TcRn TcGblEnv
mergeSignatures hsmod lcl_iface0 = do mergeSignatures
(HsParsedModule { hpm_module = L loc (HsModule { hsmodExports = mb_exports }),
hpm_src_files = src_files })
orig_tcg_env lcl_iface0 = setSrcSpan loc $ do
-- The lcl_iface0 is the ModIface for the local hsig -- The lcl_iface0 is the ModIface for the local hsig
-- file, which is guaranteed to exist, see -- file, which is guaranteed to exist, see
-- Note [Blank hsigs for all requirements] -- Note [Blank hsigs for all requirements]
...@@ -494,7 +498,6 @@ mergeSignatures hsmod lcl_iface0 = do ...@@ -494,7 +498,6 @@ mergeSignatures hsmod lcl_iface0 = do
tcg_env <- getGblEnv tcg_env <- getGblEnv
let outer_mod = tcg_mod tcg_env let outer_mod = tcg_mod tcg_env
inner_mod = tcg_semantic_mod tcg_env inner_mod = tcg_semantic_mod tcg_env
mb_exports = hsmodExports (unLoc (hpm_module hsmod))
mod_name = moduleName (tcg_mod tcg_env) mod_name = moduleName (tcg_mod tcg_env)
-- STEP 1: Figure out all of the external signature interfaces -- STEP 1: Figure out all of the external signature interfaces
...@@ -529,7 +532,16 @@ mergeSignatures hsmod lcl_iface0 = do ...@@ -529,7 +532,16 @@ mergeSignatures hsmod lcl_iface0 = do
as1 <- tcRnModExports insts ireq_iface as1 <- tcRnModExports insts ireq_iface
let inst_uid = fst (splitUnitIdInsts (IndefiniteUnitId iuid)) let inst_uid = fst (splitUnitIdInsts (IndefiniteUnitId iuid))
pkg = getInstalledPackageDetails dflags inst_uid pkg = getInstalledPackageDetails dflags inst_uid
rdr_env = mkGlobalRdrEnv (gresFromAvails Nothing as1) -- Setup the import spec correctly, so that when we apply
-- IEModuleContents we pick up EVERYTHING
ispec = ImpSpec
ImpDeclSpec{
is_mod = mod_name,
is_as = mod_name,
is_qual = False,
is_dloc = loc
} ImpAll
rdr_env = mkGlobalRdrEnv (gresFromAvails (Just ispec) as1)
(thinned_iface, as2) <- case mb_exports of (thinned_iface, as2) <- case mb_exports of
Just (L loc _) Just (L loc _)
| null (exposedModules pkg) -> setSrcSpan loc $ do | null (exposedModules pkg) -> setSrcSpan loc $ do
...@@ -572,7 +584,19 @@ mergeSignatures hsmod lcl_iface0 = do ...@@ -572,7 +584,19 @@ mergeSignatures hsmod lcl_iface0 = do
| otherwise = WarnSome $ map (\o -> (o, inheritedSigPvpWarning)) warn_occs | otherwise = WarnSome $ map (\o -> (o, inheritedSigPvpWarning)) warn_occs
-} -}
setGblEnv tcg_env { setGblEnv tcg_env {
tcg_rdr_env = rdr_env, -- The top-level GlobalRdrEnv is quite interesting. It consists
-- of two components:
-- 1. First, we reuse the GlobalRdrEnv of the local signature.
-- This is very useful, because it means that if we have
-- to print a message involving some entity that the local
-- signature imported, we'll qualify it accordingly.
-- 2. Second, we need to add all of the declarations we are
-- going to merge in (as they need to be in scope for the
-- final test of the export list.)
tcg_rdr_env = rdr_env `plusGlobalRdrEnv` tcg_rdr_env orig_tcg_env,
-- Inherit imports from the local signature, so that module
-- rexports are picked up correctly
tcg_imports = tcg_imports orig_tcg_env,
tcg_exports = exports, tcg_exports = exports,
tcg_dus = usesOnly (availsToNameSetWithSelectors exports), tcg_dus = usesOnly (availsToNameSetWithSelectors exports),
tcg_warns = warns tcg_warns = warns
...@@ -580,6 +604,7 @@ mergeSignatures hsmod lcl_iface0 = do ...@@ -580,6 +604,7 @@ mergeSignatures hsmod lcl_iface0 = do
tcg_env <- getGblEnv tcg_env <- getGblEnv
-- Make sure we didn't refer to anything that doesn't actually exist -- Make sure we didn't refer to anything that doesn't actually exist
-- pprTrace "mergeSignatures: exports_from_avail" (ppr exports) $ return ()
(mb_lies, _) <- exports_from_avail mb_exports rdr_env (mb_lies, _) <- exports_from_avail mb_exports rdr_env
(tcg_imports tcg_env) (tcg_semantic_mod tcg_env) (tcg_imports tcg_env) (tcg_semantic_mod tcg_env)
...@@ -746,6 +771,8 @@ mergeSignatures hsmod lcl_iface0 = do ...@@ -746,6 +771,8 @@ mergeSignatures hsmod lcl_iface0 = do
tcg_type_env = extendTypeEnvWithIds (tcg_type_env tcg_env) (map fst dfun_insts) tcg_type_env = extendTypeEnvWithIds (tcg_type_env tcg_env) (map fst dfun_insts)
} }
addDependentFiles src_files
return tcg_env return tcg_env
-- | Top-level driver for signature instantiation (run when compiling -- | Top-level driver for signature instantiation (run when compiling
......
...@@ -729,7 +729,7 @@ to ``hs-boot`` files, but with some slight changes: ...@@ -729,7 +729,7 @@ to ``hs-boot`` files, but with some slight changes:
- Import statements and scoping rules are exactly as in Haskell. - Import statements and scoping rules are exactly as in Haskell.
To mention a non-Prelude type or class, you must import it. To mention a non-Prelude type or class, you must import it.
- Unlike regular modules, the exports and defined entities of - Unlike regular modules, the defined entities of
a signature include not only those written in the local a signature include not only those written in the local
``hsig`` file, but also those from inherited signatures ``hsig`` file, but also those from inherited signatures
(as inferred from the :ghc-flag:`-package-id` flags). (as inferred from the :ghc-flag:`-package-id` flags).
...@@ -763,49 +763,67 @@ to ``hs-boot`` files, but with some slight changes: ...@@ -763,49 +763,67 @@ to ``hs-boot`` files, but with some slight changes:
f :: T -> T f :: T -> T
g :: T g :: T
- The export list of a signature applies the final export list - If no export list is provided for a signature, the exports of
of a signature after merging inherited signatures; in particular, it a signature are all of its defined entities merged with the
may refer to entities which are not declared in the body of the exports of all inherited signatures.
local ``hsig`` file. The set of entities that are required by a
signature is defined exclusively by its exports; if an entity
is not mentioned in the export list, it is not required. This means
that a library author can provide an omnibus signature containing the
type of every function someone might want to use, while a client thins
down the exports to the ones they actually require. For example,
supposing that you have inherited a signature for strings, you might
write a local signature of this form, listing only the entities
that you need::
signature Str (Str, empty, append, concat) where If you want to reexport an entity from a signature, you must
-- empty also include a ``module SigName`` export, so that all of the
entities defined in the signature are exported. For example,
the following module exports both ``f`` and ``Int`` from
``Prelude``::
A few caveats apply here. First, it is illegal to export an entity signature A(module A, Int) where
which refers to a locally defined type which itself is not exported import Prelude (Int)
(GHC will report an error in this case). Second, signatures which f :: Int
come from dependencies which expose modules cannot be thinned in this
way (after all, the dependency itself may need the entity); these
requirements are unconditionally exported, but are associated with
a warning discouraging their use by a module. To use an entity
defined by such a signature, add its declaration to your local
``hsig`` file.
- A signature can reexport an entity brought into scope by an import. Reexports merge with local declarations; thus, the signature above
In this case, we indicate that any implementation of the module would successfully merge with::
must export this very same entity. For example, this signature
must be implemented by a module which itself reexports ``Int``::
signature A (Int) where signature A where
import Prelude (Int) data Int
The only permissible implementation of such a signature is a module
which reexports precisely the same entity::
-- can be implemented by: module A (f, Int) where
module A (Int) where
import Prelude (Int) import Prelude (Int)
f = 2 :: Int
Conversely, any entity requested by a signature can be provided Conversely, any entity requested by a signature can be provided
by a reexport from the implementing module. This is different from by a reexport from the implementing module. This is different from
``hs-boot`` files, which require every entity to be defined ``hs-boot`` files, which require every entity to be defined
locally in the implementing module. locally in the implementing module.
- GHC has experimental support for *signature thinning*, which is used
when a signature has an explicit export list without a module export of the
signature itself. In this case, the export list applies to the final export
list *after* merging, in particular, you may refer to entities which are not
declared in the body of the local ``hsig`` file.
The semantics in this case is that the set of required entities is defined
exclusively by its exports; if an entity is not mentioned in the export list,
it is not required. The motivation behind this feature is to allow a library
author to provide an omnibus signature containing the type of every function
someone might want to use, while a client thins down the exports to the ones
they actually require. For example, supposing that you have inherited a
signature for strings, you might write a local signature of this form, listing
only the entities that you need::
signature Str (Str, empty, append, concat) where
-- empty
A few caveats apply here. First, it is illegal to export an entity
which refers to a locally defined type which itself is not exported
(GHC will report an error in this case). Second, signatures which
come from dependencies which expose modules cannot be thinned in this
way (after all, the dependency itself may need the entity); these
requirements are unconditionally exported. Finally, any module
reexports must refer to modules imported by the local signature
(even if an inherited signature exported the module).
We may change the syntax and semantics of this feature in the future.
- The declarations and types from signatures of dependencies - The declarations and types from signatures of dependencies
that will be merged in are not in scope when type checking that will be merged in are not in scope when type checking
an ``hsig`` file. To refer to any such type, you must an ``hsig`` file. To refer to any such type, you must
......
...@@ -2,8 +2,8 @@ ...@@ -2,8 +2,8 @@
q/H.hsig:2:1: error: q/H.hsig:2:1: error:
• Identifier ‘x’ has conflicting definitions in the module • Identifier ‘x’ has conflicting definitions in the module
and its hsig file and its hsig file
Main module: x :: ghc-prim-0.5.0.0:GHC.Types.Int Main module: x :: Int
Hsig file: x :: ghc-prim-0.5.0.0:GHC.Types.Bool Hsig file: x :: Bool
The two types are different The two types are different
• while merging the signatures from: • while merging the signatures from:
• z-bkpcabal01-z-p-0.1.0.0[H=<H>]:H • z-bkpcabal01-z-p-0.1.0.0[H=<H>]:H
......
...@@ -5,3 +5,7 @@ test('bkpreex04', normal, backpack_typecheck, ['']) ...@@ -5,3 +5,7 @@ test('bkpreex04', normal, backpack_typecheck, [''])
# These signatures are behaving badly and the renamer gets confused # These signatures are behaving badly and the renamer gets confused
test('bkpreex05', expect_broken(0), backpack_typecheck, ['']) test('bkpreex05', expect_broken(0), backpack_typecheck, [''])
test('bkpreex06', normal, backpack_typecheck, ['']) test('bkpreex06', normal, backpack_typecheck, [''])
test('bkpreex07', normal, backpack_typecheck, [''])
test('bkpreex08', normal, backpack_typecheck, [''])
test('bkpreex09', normal, backpack_typecheck, [''])
test('bkpreex10', normal, backpack_typecheck, [''])
unit p where
signature A(module Prelude) where
import Prelude
[1 of 1] Processing p
[1 of 1] Compiling A[sig] ( p/A.hsig, nothing )
unit q where
module B where
f = 2 :: Int
unit p2 where
dependency q
signature A(f) where
import B
unit p where
dependency q
dependency p2[A=<A>]
signature A(module A, module Prelude) where
import Prelude
f :: Int
module M where
import B
import A
g = f
[1 of 3] Processing q
Instantiating q
[1 of 1] Compiling B ( q/B.hs, nothing )
[2 of 3] Processing p2
[1 of 1] Compiling A[sig] ( p2/A.hsig, nothing )
[3 of 3] Processing p
[1 of 2] Compiling A[sig] ( p/A.hsig, nothing )
[2 of 2] Compiling M ( p/M.hs, nothing )
unit p where
signature A where
f :: Bool
unit q where
dependency p[A=<A>]
signature A(module A) where
h :: Bool
module M where
import A
g = f && h
[1 of 2] Processing p
[1 of 1] Compiling A[sig] ( p/A.hsig, nothing )
[2 of 2] Processing q
[1 of 2] Compiling A[sig] ( q/A.hsig, nothing )
[2 of 2] Compiling M ( q/M.hs, nothing )
{-# LANGUAGE ConstraintKinds #-}
unit p where
signature A(module Data.Typeable) where
import Data.Typeable
unit q where
dependency p[A=<A>]
signature A(module A) where
module M where
import A
type X = Typeable
[1 of 2] Processing p
[1 of 1] Compiling A[sig] ( p/A.hsig, nothing )
[2 of 2] Processing q
[1 of 2] Compiling A[sig] ( q/A.hsig, nothing )
[2 of 2] Compiling M ( q/M.hs, nothing )
...@@ -8,8 +8,8 @@ ...@@ -8,8 +8,8 @@
bkpfail04.bkp:7:9: error: bkpfail04.bkp:7:9: error:
• Type constructor ‘A’ has conflicting definitions in the module • Type constructor ‘A’ has conflicting definitions in the module
and its hsig file and its hsig file
Main module: data A = A {foo :: GHC.Types.Int} Main module: data A = A {foo :: Int}
Hsig file: data A = A {bar :: GHC.Types.Bool} Hsig file: data A = A {bar :: Bool}
The constructors do not match: The constructors do not match:
The record label lists for ‘A’ differ The record label lists for ‘A’ differ
The types for ‘A’ differ The types for ‘A’ differ
......
...@@ -7,9 +7,9 @@ bkpfail42.bkp:9:9: error: ...@@ -7,9 +7,9 @@ bkpfail42.bkp:9:9: error:
• Type constructor ‘F’ has conflicting definitions in the module • Type constructor ‘F’ has conflicting definitions in the module
and its hsig file and its hsig file
Main module: type family F a :: * Main module: type family F a :: *
where [a] F a = GHC.Types.Int where [a] F a = Int
Hsig file: type family F a :: * Hsig file: type family F a :: *
where [a] F a = GHC.Types.Bool where [a] F a = Bool
• while merging the signatures from: • while merging the signatures from:
• p[A=<A>]:A • p[A=<A>]:A
• ...and the local signature for A • ...and the local signature for A
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