Commit ed480981 authored by Ben Gamari's avatar Ben Gamari Committed by Ben Gamari
Browse files

InstEnv: Ensure that instance visibility check is lazy

Previously instIsVisible had completely broken the laziness of
lookupInstEnv' since it would examine is_dfun_name to check the name of
the defining module (to know whether it is an interactive module). This
resulted in the visibility check drawing in an interface file
unnecessarily. This contributed to the unnecessary regression in
compiler allocations reported in #12367.

Test Plan: Validate, check nofib changes

Reviewers: simonpj, ezyang, austin

Reviewed By: ezyang

Subscribers: thomie, ezyang

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

GHC Trac Issues: #12367
parent 627c767b
......@@ -651,13 +651,13 @@ look at it.
-}
tcIfaceInst :: IfaceClsInst -> IfL ClsInst
tcIfaceInst (IfaceClsInst { ifDFun = dfun_occ, ifOFlag = oflag
tcIfaceInst (IfaceClsInst { ifDFun = dfun_name, ifOFlag = oflag
, ifInstCls = cls, ifInstTys = mb_tcs
, ifInstOrph = orph })
= do { dfun <- forkM (text "Dict fun" <+> ppr dfun_occ) $
tcIfaceExtId dfun_occ
= do { dfun <- forkM (text "Dict fun" <+> ppr dfun_name) $
tcIfaceExtId dfun_name
; let mb_tcs' = map (fmap ifaceTyConName) mb_tcs
; return (mkImportedInstance cls mb_tcs' dfun oflag orph) }
; return (mkImportedInstance cls mb_tcs' dfun_name dfun oflag orph) }
tcIfaceFamInst :: IfaceFamInst -> IfL FamInst
tcIfaceFamInst (IfaceFamInst { ifFamInstFam = fam, ifFamInstTys = mb_tcs
......
......@@ -55,11 +55,25 @@ import Data.Maybe ( isJust, isNothing )
************************************************************************
-}
-- | A type-class instance. Note that there is some tricky laziness at work
-- here. See Note [ClsInst laziness and the rough-match fields] for more
-- details.
data ClsInst
= ClsInst { -- Used for "rough matching"; see Note [Rough-match field]
= ClsInst { -- Used for "rough matching"; see
-- Note [ClsInst laziness and the rough-match fields]
-- INVARIANT: is_tcs = roughMatchTcs is_tys
is_cls_nm :: Name -- Class name
, is_tcs :: [Maybe Name] -- Top of type args
is_cls_nm :: Name -- ^ Class name
, is_tcs :: [Maybe Name] -- ^ Top of type args
-- | @is_dfun_name = idName . is_dfun@.
--
-- We use 'is_dfun_name' for the visibility check,
-- 'instIsVisible', which needs to know the 'Module' which the
-- dictionary is defined in. However, we cannot use the 'Module'
-- attached to 'is_dfun' since doing so would mean we would
-- potentially pull in an entire interface file unnecessarily.
-- This was the cause of #12367.
, is_dfun_name :: Name
-- Used for "proper matching"; see Note [Proper-match fields]
, is_tvs :: [TyVar] -- Fresh template tyvars for full match
......@@ -95,6 +109,45 @@ isOverlappable i = hasOverlappableFlag (overlapMode (is_flag i))
isOverlapping i = hasOverlappingFlag (overlapMode (is_flag i))
isIncoherent i = hasIncoherentFlag (overlapMode (is_flag i))
{-
Note [ClsInst laziness and the rough-match fields]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Suppose we load 'instance A.C B.T' from A.hi, but suppose that the type B.T is
otherwise unused in the program. Then it's stupid to load B.hi, the data type
declaration for B.T -- and perhaps further instance declarations!
We avoid this as follows:
* is_cls_nm, is_tcs, is_dfun_name are all Names. We can poke them to our heart's
content.
* Proper-match fields. is_dfun, and its related fields is_tvs, is_cls, is_tys
contain TyVars, Class, Type, Class etc, and so are all lazy thunks. When we
poke any of these fields we'll typecheck the DFunId declaration, and hence
pull in interfaces that it refers to. See Note [Proper-match fields].
* Rough-match fields. During instance lookup, we use the is_cls_nm :: Name and
is_tcs :: [Maybe Name] fields to perform a "rough match", *without* poking
inside the DFunId. The rough-match fields allow us to say "definitely does not
match", based only on Names.
This laziness is very important; see Trac #12367. Try hard to avoid pulling on
the structured fields unless you really need the instance.
* Another place to watch is InstEnv.instIsVisible, which needs the module to
which the ClsInst belongs. We can get this from is_dfun_name.
* In is_tcs,
Nothing means that this type arg is a type variable
(Just n) means that this type arg is a
TyConApp with a type constructor of n.
This is always a real tycon, never a synonym!
(Two different synonyms might match, but two
different real tycons can't.)
NB: newtypes are not transparent, though!
-}
{-
Note [Template tyvars are fresh]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
......@@ -108,31 +161,12 @@ etc, and that requires the tyvars to be distinct.
The invariant is checked by the ASSERT in lookupInstEnv'.
Note [Rough-match field]
~~~~~~~~~~~~~~~~~~~~~~~~~~~
The is_cls_nm, is_tcs fields allow a "rough match" to be done
*without* poking inside the DFunId. Poking the DFunId forces
us to suck in all the type constructors etc it involves,
which is a total waste of time if it has no chance of matching
So the Name, [Maybe Name] fields allow us to say "definitely
does not match", based only on the Name.
In is_tcs,
Nothing means that this type arg is a type variable
(Just n) means that this type arg is a
TyConApp with a type constructor of n.
This is always a real tycon, never a synonym!
(Two different synonyms might match, but two
different real tycons can't.)
NB: newtypes are not transparent, though!
Note [Proper-match fields]
~~~~~~~~~~~~~~~~~~~~~~~~~
The is_tvs, is_cls, is_tys fields are simply cached values, pulled
out (lazily) from the dfun id. They are cached here simply so
that we don't need to decompose the DFunId each time we want
to match it. The hope is that the fast-match fields mean
to match it. The hope is that the rough-match fields mean
that we often never poke the proper-match fields.
However, note that:
......@@ -226,6 +260,7 @@ mkLocalInstance :: DFunId -> OverlapFlag
mkLocalInstance dfun oflag tvs cls tys
= ClsInst { is_flag = oflag, is_dfun = dfun
, is_tvs = tvs
, is_dfun_name = dfun_name
, is_cls = cls, is_cls_nm = cls_name
, is_tys = tys, is_tcs = roughMatchTcs tys
, is_orphan = orph
......@@ -257,19 +292,21 @@ mkLocalInstance dfun oflag tvs cls tys
choose_one nss = chooseOrphanAnchor (unionNameSets nss)
mkImportedInstance :: Name
-> [Maybe Name]
-> DFunId
-> OverlapFlag
-> IsOrphan
mkImportedInstance :: Name -- ^ the name of the class
-> [Maybe Name] -- ^ the types which the class was applied to
-> Name -- ^ the 'Name' of the dictionary binding
-> DFunId -- ^ the 'Id' of the dictionary.
-> OverlapFlag -- ^ may this instance overlap?
-> IsOrphan -- ^ is this instance an orphan?
-> ClsInst
-- Used for imported instances, where we get the rough-match stuff
-- from the interface file
-- The bound tyvars of the dfun are guaranteed fresh, because
-- the dfun has been typechecked out of the same interface file
mkImportedInstance cls_nm mb_tcs dfun oflag orphan
mkImportedInstance cls_nm mb_tcs dfun_name dfun oflag orphan
= ClsInst { is_flag = oflag, is_dfun = dfun
, is_tvs = tvs, is_tys = tys
, is_dfun_name = dfun_name
, is_cls_nm = cls_nm, is_cls = cls, is_tcs = mb_tcs
, is_orphan = orphan }
where
......@@ -397,7 +434,7 @@ instIsVisible vis_mods ispec
| IsOrphan <- is_orphan ispec = mod `elemModuleSet` vis_mods
| otherwise = True
where
mod = nameModule (idName (is_dfun ispec))
mod = nameModule $ is_dfun_name ispec
classInstances :: InstEnvs -> Class -> [ClsInst]
classInstances (InstEnvs { ie_global = pkg_ie, ie_local = home_ie, ie_visible = vis_mods }) cls
......
......@@ -5,12 +5,11 @@
Use :print or :force to determine these types
Relevant bindings include it :: a (bound at <interactive>:4:1)
These potential instances exist:
instance (Show b, Show a) => Show (Either a b)
-- Defined in ‘Data.Either’
instance Show Ordering -- Defined in ‘GHC.Show’
instance Show Integer -- Defined in ‘GHC.Show’
...plus 23 others
...plus 42 instances involving out-of-scope types
instance Show a => Show (Maybe a) -- Defined in ‘GHC.Show’
...plus 22 others
...plus 11 instances involving out-of-scope types
(use -fprint-potential-instances to see them all)
• In a stmt of an interactive GHCi command: print it
......@@ -20,11 +19,10 @@
Use :print or :force to determine these types
Relevant bindings include it :: a (bound at <interactive>:6:1)
These potential instances exist:
instance (Show b, Show a) => Show (Either a b)
-- Defined in ‘Data.Either’
instance Show Ordering -- Defined in ‘GHC.Show’
instance Show Integer -- Defined in ‘GHC.Show’
...plus 23 others
...plus 42 instances involving out-of-scope types
instance Show a => Show (Maybe a) -- Defined in ‘GHC.Show’
...plus 22 others
...plus 11 instances involving out-of-scope types
(use -fprint-potential-instances to see them all)
• In a stmt of an interactive GHCi command: print it
......@@ -62,11 +62,12 @@ test('T4029',
# 2016-05-23: 82 (amd64/Linux) Use -G1
# 2016-07-13: 92 (amd64/Linux) Changes to tidyType
stats_num_field('max_bytes_used',
[(wordsize(64), 25247216, 5)]),
[(wordsize(64), 22920616, 5)]),
# 2016-02-26: 24071720 (amd64/Linux) INITIAL
# 2016-04-21: 25542832 (amd64/Linux)
# 2016-05-23: 25247216 (amd64/Linux) Use -G1
# 2016-07-13: 27575416 (amd64/Linux) Changes to tidyType
# 2016-07-20: 22920616 (amd64/Linux) Fix laziness of instance matching
extra_hc_opts('+RTS -G1 -RTS' ),
],
ghci_script,
......
......@@ -3,11 +3,11 @@ T5095.hs:9:9: error:
• Overlapping instances for Eq a arising from a use of ‘==’
Matching instances:
instance [overlappable] Show a => Eq a -- Defined at T5095.hs:5:31
instance (Eq b, Eq a) => Eq (Either a b)
-- Defined in ‘Data.Either’
instance Eq Ordering -- Defined in ‘GHC.Classes’
...plus 24 others
...plus 36 instances involving out-of-scope types
instance Eq Integer
-- Defined in ‘integer-gmp-1.0.0.1:GHC.Integer.Type’
...plus 23 others
...plus three instances involving out-of-scope types
(use -fprint-potential-instances to see them all)
(The choice depends on the instantiation of ‘a’
To pick the first instance above, use IncoherentInstances
......
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