Skip to content
Snippets Groups Projects
Commit 1cf5c755 authored by Simon Peyton Jones's avatar Simon Peyton Jones Committed by Ian Lynagh
Browse files

Fix a grevious error in InstEnv: Trac #5095

An claimed short-cut optimisation was actually an error.
The optimisation was this: when looking up (C a b), where
'a' and 'b' are type variables, we were returning [] immediately
if the instance environment had no instances of form (C a b).
Why? Because the thing being looked up definitely won't match
(C Int Bool), say.

BUT it will *unify* with (C Int Bool) and we care very much
about things it might unify with.  If we neglect them we may
silently allow incoherent instance selection, and that is
exactly what happened in #5095.

The fix is easy: remove the "optimisation".
parent e6c4bc3d
No related branches found
No related tags found
No related merge requests found
...@@ -350,14 +350,11 @@ or, to put it another way, we have ...@@ -350,14 +350,11 @@ or, to put it another way, we have
--------------------------------------------------- ---------------------------------------------------
type InstEnv = UniqFM ClsInstEnv -- Maps Class to instances for that class type InstEnv = UniqFM ClsInstEnv -- Maps Class to instances for that class
data ClsInstEnv newtype ClsInstEnv
= ClsIE [Instance] -- The instances for a particular class, in any order = ClsIE [Instance] -- The instances for a particular class, in any order
Bool -- True <=> there is an instance of form C a b c
-- If *not* then the common case of looking up
-- (C a b c) can fail immediately
instance Outputable ClsInstEnv where instance Outputable ClsInstEnv where
ppr (ClsIE is b) = ptext (sLit "ClsIE") <+> ppr b <+> pprInstances is ppr (ClsIE is) = pprInstances is
-- INVARIANTS: -- INVARIANTS:
-- * The is_tvs are distinct in each Instance -- * The is_tvs are distinct in each Instance
...@@ -372,26 +369,24 @@ emptyInstEnv :: InstEnv ...@@ -372,26 +369,24 @@ emptyInstEnv :: InstEnv
emptyInstEnv = emptyUFM emptyInstEnv = emptyUFM
instEnvElts :: InstEnv -> [Instance] instEnvElts :: InstEnv -> [Instance]
instEnvElts ie = [elt | ClsIE elts _ <- eltsUFM ie, elt <- elts] instEnvElts ie = [elt | ClsIE elts <- eltsUFM ie, elt <- elts]
classInstances :: (InstEnv,InstEnv) -> Class -> [Instance] classInstances :: (InstEnv,InstEnv) -> Class -> [Instance]
classInstances (pkg_ie, home_ie) cls classInstances (pkg_ie, home_ie) cls
= get home_ie ++ get pkg_ie = get home_ie ++ get pkg_ie
where where
get env = case lookupUFM env cls of get env = case lookupUFM env cls of
Just (ClsIE insts _) -> insts Just (ClsIE insts) -> insts
Nothing -> [] Nothing -> []
extendInstEnvList :: InstEnv -> [Instance] -> InstEnv extendInstEnvList :: InstEnv -> [Instance] -> InstEnv
extendInstEnvList inst_env ispecs = foldl extendInstEnv inst_env ispecs extendInstEnvList inst_env ispecs = foldl extendInstEnv inst_env ispecs
extendInstEnv :: InstEnv -> Instance -> InstEnv extendInstEnv :: InstEnv -> Instance -> InstEnv
extendInstEnv inst_env ins_item@(Instance { is_cls = cls_nm, is_tcs = mb_tcs }) extendInstEnv inst_env ins_item@(Instance { is_cls = cls_nm })
= addToUFM_C add inst_env cls_nm (ClsIE [ins_item] ins_tyvar) = addToUFM_C add inst_env cls_nm (ClsIE [ins_item])
where where
add (ClsIE cur_insts cur_tyvar) _ = ClsIE (ins_item : cur_insts) add (ClsIE cur_insts) _ = ClsIE (ins_item : cur_insts)
(ins_tyvar || cur_tyvar)
ins_tyvar = not (any isJust mb_tcs)
\end{code} \end{code}
...@@ -442,7 +437,7 @@ lookupInstEnv :: (InstEnv, InstEnv) -- External and home package inst-env ...@@ -442,7 +437,7 @@ lookupInstEnv :: (InstEnv, InstEnv) -- External and home package inst-env
-- Then which we choose would depend on the way in which 'a' -- Then which we choose would depend on the way in which 'a'
-- is instantiated. So we report that Foo [b] is a match (mapping b->a) -- is instantiated. So we report that Foo [b] is a match (mapping b->a)
-- but Foo [Int] is a unifier. This gives the caller a better chance of -- but Foo [Int] is a unifier. This gives the caller a better chance of
-- giving a suitable error messagen -- giving a suitable error message
lookupInstEnv (pkg_ie, home_ie) cls tys lookupInstEnv (pkg_ie, home_ie) cls tys
= (safe_matches, all_unifs, safe_fail) = (safe_matches, all_unifs, safe_fail)
...@@ -494,22 +489,9 @@ lookupInstEnv (pkg_ie, home_ie) cls tys ...@@ -494,22 +489,9 @@ lookupInstEnv (pkg_ie, home_ie) cls tys
-------------- --------------
lookup env = case lookupUFM env cls of lookup env = case lookupUFM env cls of
Nothing -> ([],[]) -- No instances for this class Nothing -> ([],[]) -- No instances for this class
Just (ClsIE insts has_tv_insts) Just (ClsIE insts) -> find [] [] insts
| all_tvs && not has_tv_insts
-> ([],[]) -- Short cut for common case
-- The thing we are looking up is of form (C a b c), and
-- the ClsIE has no instances of that form, so don't bother to search
| otherwise
-> find [] [] insts
-------------- --------------
lookup_tv :: TvSubst -> TyVar -> Either TyVar Type
-- See Note [InstTypes: instantiating types]
lookup_tv subst tv = case lookupTyVar subst tv of
Just ty -> Right ty
Nothing -> Left tv
find ms us [] = (ms, us) find ms us [] = (ms, us)
find ms us (item@(Instance { is_tcs = mb_tcs, is_tvs = tpl_tvs, find ms us (item@(Instance { is_tcs = mb_tcs, is_tvs = tpl_tvs,
is_tys = tpl_tys, is_flag = oflag, is_tys = tpl_tys, is_flag = oflag,
...@@ -541,6 +523,13 @@ lookupInstEnv (pkg_ie, home_ie) cls tys ...@@ -541,6 +523,13 @@ lookupInstEnv (pkg_ie, home_ie) cls tys
Just _ -> find ms (item:us) rest Just _ -> find ms (item:us) rest
Nothing -> find ms us rest Nothing -> find ms us rest
----------------
lookup_tv :: TvSubst -> TyVar -> Either TyVar Type
-- See Note [InstTypes: instantiating types]
lookup_tv subst tv = case lookupTyVar subst tv of
Just ty -> Right ty
Nothing -> Left tv
--------------- ---------------
--------------- ---------------
insert_overlapping :: InstMatch -> [InstMatch] -> [InstMatch] insert_overlapping :: InstMatch -> [InstMatch] -> [InstMatch]
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment