summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon Peyton Jones <simonpj@microsoft.com>2019-08-30 13:43:24 +0100
committerSimon Peyton Jones <simonpj@microsoft.com>2019-09-20 10:50:21 +0100
commit0dad81ca5fd1f63bf8a3b6ad09787559e8bd05c0 (patch)
tree8759889c9a5bfe5f59dda0f809c2bfc1b8fab3f1
parent1755424806839d57a0c5672922a4b65b838f7d17 (diff)
downloadhaskell-0dad81ca5fd1f63bf8a3b6ad09787559e8bd05c0.tar.gz
Fix bogus type of case expressionwip/T17056
Issue #17056 revealed that we were sometimes building a case expression whose type field (in the Case constructor) was bogus. Consider a phantom type synonym type S a = Int and we want to form the case expression case x of K (a::*) -> (e :: S a) We must not make the type field of the Case constructor be (S a) because 'a' isn't in scope. We must instead expand the synonym. Changes in this patch: * Expand synonyms in the new function CoreUtils.mkSingleAltCase. * Use mkSingleAltCase in MkCore.wrapFloat, which was the proximate source of the bug (when called by exprIsConApp_maybe) * Use mkSingleAltCase elsewhere * Documentation CoreSyn new invariant (6) in Note [Case expression invariants] CoreSyn Note [Why does Case have a 'Type' field?] CoreUtils Note [Care with the type of a case expression] * I improved Core Lint's error reporting, which was pretty confusing in this case, because it didn't mention that the offending type was the return type of a case expression. * A little bit of cosmetic refactoring in CoreUtils
-rw-r--r--compiler/basicTypes/MkId.hs12
-rw-r--r--compiler/coreSyn/CoreLint.hs175
-rw-r--r--compiler/coreSyn/CorePrep.hs2
-rw-r--r--compiler/coreSyn/CoreSyn.hs142
-rw-r--r--compiler/coreSyn/CoreUtils.hs45
-rw-r--r--compiler/coreSyn/MkCore.hs123
-rw-r--r--compiler/deSugar/DsUtils.hs3
-rw-r--r--compiler/simplCore/SimplUtils.hs6
-rw-r--r--compiler/stranal/WwLib.hs12
-rw-r--r--compiler/types/Type.hs4
-rw-r--r--testsuite/tests/dependent/should_compile/all.T2
-rw-r--r--testsuite/tests/indexed-types/should_compile/T17056.hs47
-rw-r--r--testsuite/tests/indexed-types/should_compile/all.T1
13 files changed, 397 insertions, 177 deletions
diff --git a/compiler/basicTypes/MkId.hs b/compiler/basicTypes/MkId.hs
index 741b48e58b..bc7d0f57c9 100644
--- a/compiler/basicTypes/MkId.hs
+++ b/compiler/basicTypes/MkId.hs
@@ -48,7 +48,7 @@ import FamInstEnv
import Coercion
import TcType
import MkCore
-import CoreUtils ( exprType, mkCast )
+import CoreUtils ( mkCast, mkDefaultCase )
import CoreUnfold
import Literal
import TyCon
@@ -463,8 +463,8 @@ mkDictSelRhs clas val_index
rhs_body | new_tycon = unwrapNewTypeBody tycon (mkTyVarTys tyvars)
(Var dict_id)
- | otherwise = Case (Var dict_id) dict_id (idType the_arg_id)
- [(DataAlt data_con, arg_ids, varToCoreExpr the_arg_id)]
+ | otherwise = mkSingleAltCase (Var dict_id) dict_id (DataAlt data_con)
+ arg_ids (varToCoreExpr the_arg_id)
-- varToCoreExpr needed for equality superclass selectors
-- sel a b d = case x of { MkC _ (g:a~b) _ -> CO g }
@@ -987,7 +987,7 @@ wrapCo co rep_ty (unbox_rep, box_rep) -- co :: arg_ty ~ rep_ty
------------------------
seqUnboxer :: Unboxer
-seqUnboxer v = return ([v], \e -> Case (Var v) v (exprType e) [(DEFAULT, [], e)])
+seqUnboxer v = return ([v], mkDefaultCase (Var v) v)
unitUnboxer :: Unboxer
unitUnboxer v = return ([v], \e -> e)
@@ -1014,8 +1014,8 @@ dataConArgUnpack arg_ty
,( \ arg_id ->
do { rep_ids <- mapM newLocal rep_tys
; let unbox_fn body
- = Case (Var arg_id) arg_id (exprType body)
- [(DataAlt con, rep_ids, body)]
+ = mkSingleAltCase (Var arg_id) arg_id
+ (DataAlt con) rep_ids body
; return (rep_ids, unbox_fn) }
, Boxer $ \ subst ->
do { rep_ids <- mapM (newLocal . TcType.substTyUnchecked subst) rep_tys
diff --git a/compiler/coreSyn/CoreLint.hs b/compiler/coreSyn/CoreLint.hs
index 86943e2ba7..e0f4dda2aa 100644
--- a/compiler/coreSyn/CoreLint.hs
+++ b/compiler/coreSyn/CoreLint.hs
@@ -3,7 +3,8 @@
(c) The GRASP/AQUA Project, Glasgow University, 1993-1998
-A ``lint'' pass to check for Core correctness
+A ``lint'' pass to check for Core correctness.
+See Note [Core Lint guarantee].
-}
{-# LANGUAGE CPP #-}
@@ -78,6 +79,23 @@ import Pair
import qualified GHC.LanguageExtensions as LangExt
{-
+Note [Core Lint guarantee]
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+Core Lint is the type-checker for Core. Using it, we get the following guarantee:
+
+If all of:
+1. Core Lint passes,
+2. there are no unsafe coercions (i.e. UnsafeCoerceProv),
+3. all plugin-supplied coercions (i.e. PluginProv) are valid, and
+4. all case-matches are complete
+then running the compiled program will not seg-fault, assuming no bugs downstream
+(e.g. in the code generator). This guarantee is quite powerful, in that it allows us
+to decouple the safety of the resulting program from the type inference algorithm.
+
+However, do note point (4) above. Core Lint does not check for incomplete case-matches;
+see Note [Case expression invariants] in CoreSyn, invariant (4). As explained there,
+an incomplete case-match might slip by Core Lint and cause trouble at runtime.
+
Note [GHC Formalism]
~~~~~~~~~~~~~~~~~~~~
This file implements the type-checking algorithm for System FC, the "official"
@@ -392,6 +410,7 @@ interactiveInScope hsc_env
-- f :: [t] -> [t]
-- where t is a RuntimeUnk (see TcType)
+-- | Type-check a 'CoreProgram'. See Note [Core Lint guarantee].
lintCoreBindings :: DynFlags -> CoreToDo -> [Var] -> CoreProgram -> (Bag MsgDoc, Bag MsgDoc)
-- Returns (warnings, errors)
-- If you edit this function, you may need to update the GHC formalism
@@ -786,50 +805,8 @@ lintCoreExpr (Lam var expr)
do { body_ty <- lintCoreExpr expr
; return $ mkLamType var' body_ty }
-lintCoreExpr e@(Case scrut var alt_ty alts) =
- -- Check the scrutinee
- do { scrut_ty <- markAllJoinsBad $ lintCoreExpr scrut
- -- See Note [Join points are less general than the paper]
- -- in CoreSyn
-
- ; (alt_ty, _) <- lintInTy alt_ty
- ; (var_ty, _) <- lintInTy (idType var)
-
- -- We used to try to check whether a case expression with no
- -- alternatives was legitimate, but this didn't work.
- -- See Note [No alternatives lint check] for details.
-
- -- See Note [Rules for floating-point comparisons] in PrelRules
- ; let isLitPat (LitAlt _, _ , _) = True
- isLitPat _ = False
- ; checkL (not $ isFloatingTy scrut_ty && any isLitPat alts)
- (ptext (sLit $ "Lint warning: Scrutinising floating-point " ++
- "expression with literal pattern in case " ++
- "analysis (see #9238).")
- $$ text "scrut" <+> ppr scrut)
-
- ; case tyConAppTyCon_maybe (idType var) of
- Just tycon
- | debugIsOn
- , isAlgTyCon tycon
- , not (isAbstractTyCon tycon)
- , null (tyConDataCons tycon)
- , not (exprIsBottom scrut)
- -> pprTrace "Lint warning: case binder's type has no constructors" (ppr var <+> ppr (idType var))
- -- This can legitimately happen for type families
- $ return ()
- _otherwise -> return ()
-
- -- Don't use lintIdBndr on var, because unboxed tuple is legitimate
-
- ; subst <- getTCvSubst
- ; ensureEqTys var_ty scrut_ty (mkScrutMsg var var_ty scrut_ty subst)
-
- ; lintBinder CaseBind var $ \_ ->
- do { -- Check the alternatives
- mapM_ (lintCoreAlt scrut_ty alt_ty) alts
- ; checkCaseAlts e scrut_ty alts
- ; return alt_ty } }
+lintCoreExpr (Case scrut var alt_ty alts)
+ = lintCaseExpr scrut var alt_ty alts
-- This case can't happen; linting types in expressions gets routed through
-- lintCoreArgs
@@ -1095,6 +1072,60 @@ lintTyKind tyvar arg_ty
************************************************************************
-}
+lintCaseExpr :: CoreExpr -> Id -> Type -> [CoreAlt] -> LintM OutType
+lintCaseExpr scrut var alt_ty alts =
+ do { let e = Case scrut var alt_ty alts -- Just for error messages
+
+ -- Check the scrutinee
+ ; scrut_ty <- markAllJoinsBad $ lintCoreExpr scrut
+ -- See Note [Join points are less general than the paper]
+ -- in CoreSyn
+
+ ; (alt_ty, _) <- addLoc (CaseTy scrut) $
+ lintInTy alt_ty
+ ; (var_ty, _) <- addLoc (IdTy var) $
+ lintInTy (idType var)
+
+ -- We used to try to check whether a case expression with no
+ -- alternatives was legitimate, but this didn't work.
+ -- See Note [No alternatives lint check] for details.
+
+ -- Check that the scrutinee is not a floating-point type
+ -- if there are any literal alternatives
+ -- See CoreSyn Note [Case expression invariants] item (5)
+ -- See Note [Rules for floating-point comparisons] in PrelRules
+ ; let isLitPat (LitAlt _, _ , _) = True
+ isLitPat _ = False
+ ; checkL (not $ isFloatingTy scrut_ty && any isLitPat alts)
+ (ptext (sLit $ "Lint warning: Scrutinising floating-point " ++
+ "expression with literal pattern in case " ++
+ "analysis (see #9238).")
+ $$ text "scrut" <+> ppr scrut)
+
+ ; case tyConAppTyCon_maybe (idType var) of
+ Just tycon
+ | debugIsOn
+ , isAlgTyCon tycon
+ , not (isAbstractTyCon tycon)
+ , null (tyConDataCons tycon)
+ , not (exprIsBottom scrut)
+ -> pprTrace "Lint warning: case binder's type has no constructors" (ppr var <+> ppr (idType var))
+ -- This can legitimately happen for type families
+ $ return ()
+ _otherwise -> return ()
+
+ -- Don't use lintIdBndr on var, because unboxed tuple is legitimate
+
+ ; subst <- getTCvSubst
+ ; ensureEqTys var_ty scrut_ty (mkScrutMsg var var_ty scrut_ty subst)
+ -- See CoreSyn Note [Case expression invariants] item (7)
+
+ ; lintBinder CaseBind var $ \_ ->
+ do { -- Check the alternatives
+ mapM_ (lintCoreAlt scrut_ty alt_ty) alts
+ ; checkCaseAlts e scrut_ty alts
+ ; return alt_ty } }
+
checkCaseAlts :: CoreExpr -> OutType -> [CoreAlt] -> LintM ()
-- a) Check that the alts are non-empty
-- b1) Check that the DEFAULT comes first, if it exists
@@ -1106,7 +1137,10 @@ checkCaseAlts :: CoreExpr -> OutType -> [CoreAlt] -> LintM ()
checkCaseAlts e ty alts =
do { checkL (all non_deflt con_alts) (mkNonDefltMsg e)
+ -- See CoreSyn Note [Case expression invariants] item (2)
+
; checkL (increasing_tag con_alts) (mkNonIncreasingAltsMsg e)
+ -- See CoreSyn Note [Case expression invariants] item (3)
-- For types Int#, Word# with an infinite (well, large!) number of
-- possible values, there should usually be a DEFAULT case
@@ -1136,6 +1170,7 @@ lintAltExpr :: CoreExpr -> OutType -> LintM ()
lintAltExpr expr ann_ty
= do { actual_ty <- lintCoreExpr expr
; ensureEqTys actual_ty ann_ty (mkCaseAltMsg expr actual_ty ann_ty) }
+ -- See CoreSyn Note [Case expression invariants] item (6)
lintCoreAlt :: OutType -- Type of scrutinee
-> OutType -- Type of the alternative
@@ -1246,7 +1281,8 @@ lintIdBndr top_lvl bind_site id linterF
; checkL (not (isExternalName (Var.varName id)) || is_top_lvl)
(mkNonTopExternalNameMsg id)
- ; (ty, k) <- lintInTy (idType id)
+ ; (ty, k) <- addLoc (IdTy id) $
+ lintInTy (idType id)
-- See Note [Levity polymorphism invariants] in CoreSyn
; lintL (isJoinId id || not (isKindLevPoly k))
@@ -2180,6 +2216,9 @@ data LintLocInfo
| BodyOfLetRec [Id] -- One of the binders
| CaseAlt CoreAlt -- Case alternative
| CasePat CoreAlt -- The *pattern* of the case alternative
+ | CaseTy CoreExpr -- The type field of a case expression
+ -- with this scrutinee
+ | IdTy Id -- The type field of an Id binder
| AnExpr CoreExpr -- Some expression
| ImportedUnfolding SrcLoc -- Some imported unfolding (ToDo: say which)
| TopLevelBindings
@@ -2237,17 +2276,23 @@ addWarnL msg = LintM $ \ env (warns,errs) ->
addMsg :: LintEnv -> Bag MsgDoc -> MsgDoc -> Bag MsgDoc
addMsg env msgs msg
- = ASSERT( notNull locs )
+ = ASSERT( notNull loc_msgs )
msgs `snocBag` mk_msg msg
where
- locs = le_loc env
- (loc, cxt1) = dumpLoc (head locs)
- cxts = [snd (dumpLoc loc) | loc <- locs]
- context = ifPprDebug (vcat (reverse cxts) $$ cxt1 $$
- text "Substitution:" <+> ppr (le_subst env))
- cxt1
+ loc_msgs :: [(SrcLoc, SDoc)] -- Innermost first
+ loc_msgs = map dumpLoc (le_loc env)
+
+ cxt_doc = vcat $ reverse $ map snd loc_msgs
+ context = cxt_doc $$ whenPprDebug extra
+ extra = text "Substitution:" <+> ppr (le_subst env)
- mk_msg msg = mkLocMessage SevWarning (mkSrcSpan loc loc) (context $$ msg)
+ msg_span = case [ span | (loc,_) <- loc_msgs
+ , let span = srcLocSpan loc
+ , isGoodSrcSpan span ] of
+ [] -> noSrcSpan
+ (s:_) -> s
+ mk_msg msg = mkLocMessage SevWarning msg_span
+ (msg $$ context)
addLoc :: LintLocInfo -> LintM a -> LintM a
addLoc extra_loc m
@@ -2345,7 +2390,8 @@ lintTyCoVarInScope :: TyCoVar -> LintM ()
lintTyCoVarInScope var
= do { subst <- getTCvSubst
; lintL (var `isInScope` subst)
- (pprBndr LetBind var <+> text "is out of scope") }
+ (hang (text "The variable" <+> pprBndr LetBind var)
+ 2 (text "is out of scope")) }
ensureEqTys :: OutType -> OutType -> MsgDoc -> LintM ()
-- check ty2 is subtype of ty1 (ie, has same structure but usage
@@ -2375,19 +2421,19 @@ lintRole co r1 r2
dumpLoc :: LintLocInfo -> (SrcLoc, SDoc)
dumpLoc (RhsOf v)
- = (getSrcLoc v, brackets (text "RHS of" <+> pp_binders [v]))
+ = (getSrcLoc v, text "In the RHS of" <+> pp_binders [v])
dumpLoc (LambdaBodyOf b)
- = (getSrcLoc b, brackets (text "in body of lambda with binder" <+> pp_binder b))
+ = (getSrcLoc b, text "In the body of lambda with binder" <+> pp_binder b)
dumpLoc (UnfoldingOf b)
- = (getSrcLoc b, brackets (text "in the unfolding of" <+> pp_binder b))
+ = (getSrcLoc b, text "In the unfolding of" <+> pp_binder b)
dumpLoc (BodyOfLetRec [])
- = (noSrcLoc, brackets (text "In body of a letrec with no binders"))
+ = (noSrcLoc, text "In body of a letrec with no binders")
dumpLoc (BodyOfLetRec bs@(_:_))
- = ( getSrcLoc (head bs), brackets (text "in body of letrec with binders" <+> pp_binders bs))
+ = ( getSrcLoc (head bs), text "In the body of letrec with binders" <+> pp_binders bs)
dumpLoc (AnExpr e)
= (noSrcLoc, text "In the expression:" <+> ppr e)
@@ -2398,8 +2444,15 @@ dumpLoc (CaseAlt (con, args, _))
dumpLoc (CasePat (con, args, _))
= (noSrcLoc, text "In the pattern of a case alternative:" <+> parens (ppr con <+> pp_binders args))
+dumpLoc (CaseTy scrut)
+ = (noSrcLoc, hang (text "In the result-type of a case with scrutinee:")
+ 2 (ppr scrut))
+
+dumpLoc (IdTy b)
+ = (getSrcLoc b, text "In the type of a binder:" <+> ppr b)
+
dumpLoc (ImportedUnfolding locn)
- = (locn, brackets (text "in an imported unfolding"))
+ = (locn, text "In an imported unfolding")
dumpLoc TopLevelBindings
= (noSrcLoc, Outputable.empty)
dumpLoc (InType ty)
diff --git a/compiler/coreSyn/CorePrep.hs b/compiler/coreSyn/CorePrep.hs
index 6be5346ab5..9d4044cf57 100644
--- a/compiler/coreSyn/CorePrep.hs
+++ b/compiler/coreSyn/CorePrep.hs
@@ -1274,7 +1274,7 @@ wrapBinds :: Floats -> CpeBody -> CpeBody
wrapBinds (Floats _ binds) body
= foldrOL mk_bind body binds
where
- mk_bind (FloatCase bndr rhs _) body = Case rhs bndr (exprType body) [(DEFAULT, [], body)]
+ mk_bind (FloatCase bndr rhs _) body = mkDefaultCase rhs bndr body
mk_bind (FloatLet bind) body = Let bind body
mk_bind (FloatTick tickish) body = mkTick tickish body
diff --git a/compiler/coreSyn/CoreSyn.hs b/compiler/coreSyn/CoreSyn.hs
index f8fb9ef971..d94761b237 100644
--- a/compiler/coreSyn/CoreSyn.hs
+++ b/compiler/coreSyn/CoreSyn.hs
@@ -201,40 +201,7 @@ These data types are the heart of the compiler
-- The binder gets bound to the value of the scrutinee,
-- and the 'Type' must be that of all the case alternatives
--
--- #case_invariants#
--- This is one of the more complicated elements of the Core language,
--- and comes with a number of restrictions:
---
--- 1. The list of alternatives may be empty;
--- See Note [Empty case alternatives]
---
--- 2. The 'DEFAULT' case alternative must be first in the list,
--- if it occurs at all.
---
--- 3. The remaining cases are in order of increasing
--- tag (for 'DataAlts') or
--- lit (for 'LitAlts').
--- This makes finding the relevant constructor easy,
--- and makes comparison easier too.
---
--- 4. The list of alternatives must be exhaustive. An /exhaustive/ case
--- does not necessarily mention all constructors:
---
--- @
--- data Foo = Red | Green | Blue
--- ... case x of
--- Red -> True
--- other -> f (case x of
--- Green -> ...
--- Blue -> ... ) ...
--- @
---
--- The inner case does not need a @Red@ alternative, because @x@
--- can't be @Red@ at that program point.
---
--- 5. Floating-point values must not be scrutinised against literals.
--- See #9238 and Note [Rules for floating-point comparisons]
--- in PrelRules for rationale.
+-- IMPORTANT: see Note [Case expression invariants]
--
-- * Cast an expression to a particular type.
-- This is used to implement @newtype@s (a @newtype@ constructor or
@@ -247,6 +214,41 @@ These data types are the heart of the compiler
--
-- * A coercion
+{- Note [Why does Case have a 'Type' field?]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The obvious alternative is
+ exprType (Case scrut bndr alts)
+ | (_,_,rhs1):_ <- alts
+ = exprType rhs1
+
+But caching the type in the Case constructor
+ exprType (Case scrut bndr ty alts) = ty
+is better for at least three reasons:
+
+* It works when there are no alternatives (see case invarant 1 above)
+
+* It might be faster in deeply-nested situations.
+
+* It might not be quite the same as (exprType rhs) for one
+ of the RHSs in alts. Consider a phantom type synonym
+ type S a = Int
+ and we want to form the case expression
+ case x of { K (a::*) -> (e :: S a) }
+ Then exprType of the RHS is (S a), but we cannot make that be
+ the 'ty' in the Case constructor because 'a' is simply not in
+ scope there. Instead we must expand the synonym to Int before
+ putting it in the Case constructor. See CoreUtils.mkSingleAltCase.
+
+ So we'd have to do synonym expansion in exprType which would
+ be inefficient.
+
+* The type stored in the case is checked with lintInTy. This checks
+ (among other things) that it does not mention any variables that are
+ not in scope. If we did not have the type there, it would be a bit
+ harder for Core Lint to reject case blah of Ex x -> x where
+ data Ex = forall a. Ex a.
+-}
+
-- If you edit this type, you may need to update the GHC formalism
-- See Note [GHC Formalism] in coreSyn/CoreLint.hs
data Expr b
@@ -255,7 +257,8 @@ data Expr b
| App (Expr b) (Arg b)
| Lam b (Expr b)
| Let (Bind b) (Expr b)
- | Case (Expr b) b Type [Alt b] -- See #case_invariants#
+ | Case (Expr b) b Type [Alt b] -- See Note [Case expression invariants]
+ -- and Note [Why does Case have a 'Type' field?]
| Cast (Expr b) Coercion
| Tick (Tickish Id) (Expr b)
| Type Type
@@ -448,6 +451,71 @@ coreSyn/MkCore.
For discussion of some implications of the let/app invariant primops see
Note [Checking versus non-checking primops] in PrimOp.
+Note [Case expression invariants]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Case expressions are one of the more complicated elements of the Core
+language, and come with a number of invariants. All of them should be
+checked by Core Lint.
+
+1. The list of alternatives may be empty;
+ See Note [Empty case alternatives]
+
+2. The 'DEFAULT' case alternative must be first in the list,
+ if it occurs at all. Checked in CoreLint.checkCaseAlts.
+
+3. The remaining cases are in order of (strictly) increasing
+ tag (for 'DataAlts') or
+ lit (for 'LitAlts').
+ This makes finding the relevant constructor easy, and makes
+ comparison easier too. Checked in CoreLint.checkCaseAlts.
+
+4. The list of alternatives must be exhaustive. An /exhaustive/ case
+ does not necessarily mention all constructors:
+
+ @
+ data Foo = Red | Green | Blue
+ ... case x of
+ Red -> True
+ other -> f (case x of
+ Green -> ...
+ Blue -> ... ) ...
+ @
+
+ The inner case does not need a @Red@ alternative, because @x@
+ can't be @Red@ at that program point.
+
+ This is not checked by Core Lint -- it's very hard to do so.
+ E.g. suppose that inner case was floated out, thus:
+ let a = case x of
+ Green -> ...
+ Blue -> ... )
+ case x of
+ Red -> True
+ other -> f a
+ Now it's really hard to see that the Green/Blue case is
+ exhaustive. But it is.
+
+ If you have a case-expression that really /isn't/ exhaustive,
+ we may generate seg-faults. Consider the Green/Blue case
+ above. Since there are only two branches we may generate
+ code that tests for Green, and if not Green simply /assumes/
+ Blue (since, if the case is exhaustive, that's all that
+ remains). Of course, if it's not Blue and we start fetching
+ fields that should be in a Blue constructor, we may die
+ horribly. See also Note [Core Lint guarantee] in CoreLint.
+
+5. Floating-point values must not be scrutinised against literals.
+ See #9238 and Note [Rules for floating-point comparisons]
+ in PrelRules for rationale. Checked in lintCaseExpr;
+ see the call to isFloatingTy.
+
+6. The 'ty' field of (Case scrut bndr ty alts) is the type of the
+ /entire/ case expression. Checked in lintAltExpr.
+ See also Note [Why does Case have a 'Type' field?].
+
+7. The type of the scrutinee must be the same as the type
+ of the case binder, obviously. Checked in lintCaseExpr.
+
Note [CoreSyn type and coercion invariant]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
We allow a /non-recursive/, /non-top-level/ let to bind type and
@@ -1748,8 +1816,8 @@ ltAlt a1 a2 = (a1 `cmpAlt` a2) == LT
cmpAltCon :: AltCon -> AltCon -> Ordering
-- ^ Compares 'AltCon's within a single list of alternatives
--- DEFAULT comes out smallest, so that sorting by AltCon
--- puts alternatives in the order required by #case_invariants#
+-- DEFAULT comes out smallest, so that sorting by AltCon puts
+-- alternatives in the order required: see Note [Case expression invariants]
cmpAltCon DEFAULT DEFAULT = EQ
cmpAltCon DEFAULT _ = LT
diff --git a/compiler/coreSyn/CoreUtils.hs b/compiler/coreSyn/CoreUtils.hs
index d3ed00f783..95aae22b58 100644
--- a/compiler/coreSyn/CoreUtils.hs
+++ b/compiler/coreSyn/CoreUtils.hs
@@ -14,7 +14,7 @@ module CoreUtils (
mkCast,
mkTick, mkTicks, mkTickNoHNF, tickHNFArgs,
bindNonRec, needsCaseBinding,
- mkAltExpr,
+ mkAltExpr, mkDefaultCase, mkSingleAltCase,
-- * Taking expressions apart
findDefault, addDefault, findAlt, isDefaultAlt,
@@ -488,7 +488,7 @@ bindNonRec bndr rhs body
| needsCaseBinding (idType bndr) rhs = case_bind
| otherwise = let_bind
where
- case_bind = Case rhs bndr (exprType body) [(DEFAULT, [], body)]
+ case_bind = mkDefaultCase rhs bndr body
let_bind = Let (NonRec bndr rhs) body
-- | Tests whether we have to use a @case@ rather than @let@ binding for this expression
@@ -512,8 +512,45 @@ mkAltExpr (LitAlt lit) [] []
mkAltExpr (LitAlt _) _ _ = panic "mkAltExpr LitAlt"
mkAltExpr DEFAULT _ _ = panic "mkAltExpr DEFAULT"
-{- Note [Binding coercions]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~
+mkDefaultCase :: CoreExpr -> Id -> CoreExpr -> CoreExpr
+-- Make (case x of y { DEFAULT -> e }
+mkDefaultCase scrut case_bndr body
+ = Case scrut case_bndr (exprType body) [(DEFAULT, [], body)]
+
+mkSingleAltCase :: CoreExpr -> Id -> AltCon -> [Var] -> CoreExpr -> CoreExpr
+-- Use this function if possible, when building a case,
+-- because it ensures that the type on the Case itself
+-- doesn't mention variables bound by the case
+-- See Note [Care with the type of a case expression]
+mkSingleAltCase scrut case_bndr con bndrs body
+ = Case scrut case_bndr case_ty [(con,bndrs,body)]
+ where
+ body_ty = exprType body
+
+ case_ty -- See Note [Care with the type of a case expression]
+ | Just body_ty' <- occCheckExpand bndrs body_ty
+ = body_ty'
+
+ | otherwise
+ = pprPanic "mkSingleAltCase" (ppr scrut $$ ppr bndrs $$ ppr body_ty)
+
+{- Note [Care with the type of a case expression]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Consider a phantom type synonym
+ type S a = Int
+and we want to form the case expression
+ case x of K (a::*) -> (e :: S a)
+
+We must not make the type field of the case-expression (S a) because
+'a' isn't in scope. Hence the call to occCheckExpand. This caused
+issue #17056.
+
+NB: this situation can only arise with type synonyms, which can
+falsely "mention" type variables that aren't "really there", and which
+can be eliminated by expanding the synonym.
+
+Note [Binding coercions]
+~~~~~~~~~~~~~~~~~~~~~~~~
Consider binding a CoVar, c = e. Then, we must atisfy
Note [CoreSyn type and coercion invariant] in CoreSyn,
which allows only (Coercion co) on the RHS.
diff --git a/compiler/coreSyn/MkCore.hs b/compiler/coreSyn/MkCore.hs
index e2c881a1c4..b451e61a63 100644
--- a/compiler/coreSyn/MkCore.hs
+++ b/compiler/coreSyn/MkCore.hs
@@ -7,6 +7,7 @@ module MkCore (
mkCoreApp, mkCoreApps, mkCoreConApps,
mkCoreLams, mkWildCase, mkIfThenElse,
mkWildValBinder, mkWildEvBinder,
+ mkSingleAltCase,
sortQuantVars, castBottomExpr,
-- * Constructing boxed literals
@@ -57,7 +58,7 @@ import Id
import Var ( EvVar, setTyVarUnique )
import CoreSyn
-import CoreUtils ( exprType, needsCaseBinding, bindNonRec )
+import CoreUtils ( exprType, needsCaseBinding, mkSingleAltCase, bindNonRec )
import Literal
import HscTypes
@@ -111,29 +112,34 @@ mkCoreLet (NonRec bndr rhs) body -- See Note [CoreSyn let/app invariant]
mkCoreLet bind body
= Let bind body
+-- | Create a lambda where the given expression has a number of variables
+-- bound over it. The leftmost binder is that bound by the outermost
+-- lambda in the result
+mkCoreLams :: [CoreBndr] -> CoreExpr -> CoreExpr
+mkCoreLams = mkLams
+
-- | Bind a list of binding groups over an expression. The leftmost binding
-- group becomes the outermost group in the resulting expression
mkCoreLets :: [CoreBind] -> CoreExpr -> CoreExpr
mkCoreLets binds body = foldr mkCoreLet body binds
--- | Construct an expression which represents the application of one expression
--- paired with its type to an argument. The result is paired with its type. This
--- function is not exported and used in the definition of 'mkCoreApp' and
--- 'mkCoreApps'.
+-- | Construct an expression which represents the application of a number of
+-- expressions to that of a data constructor expression. The leftmost expression
+-- in the list is applied first
+mkCoreConApps :: DataCon -> [CoreExpr] -> CoreExpr
+mkCoreConApps con args = mkCoreApps (Var (dataConWorkId con)) args
+
+-- | Construct an expression which represents the application of a number of
+-- expressions to another. The leftmost expression in the list is applied first
-- Respects the let/app invariant by building a case expression where necessary
-- See CoreSyn Note [CoreSyn let/app invariant]
-mkCoreAppTyped :: SDoc -> (CoreExpr, Type) -> CoreExpr -> (CoreExpr, Type)
-mkCoreAppTyped _ (fun, fun_ty) (Type ty)
- = (App fun (Type ty), piResultTy fun_ty ty)
-mkCoreAppTyped _ (fun, fun_ty) (Coercion co)
- = (App fun (Coercion co), res_ty)
- where
- (_, res_ty) = splitFunTy fun_ty
-mkCoreAppTyped d (fun, fun_ty) arg
- = ASSERT2( isFunTy fun_ty, ppr fun $$ ppr arg $$ d )
- (mk_val_app fun arg arg_ty res_ty, res_ty)
+mkCoreApps :: CoreExpr -> [CoreExpr] -> CoreExpr
+mkCoreApps fun args
+ = fst $
+ foldl' (mkCoreAppTyped doc_string) (fun, fun_ty) args
where
- (arg_ty, res_ty) = splitFunTy fun_ty
+ doc_string = ppr fun_ty $$ ppr fun $$ ppr args
+ fun_ty = exprType fun
-- | Construct an expression which represents the application of one expression
-- to the other
@@ -143,47 +149,40 @@ mkCoreApp :: SDoc -> CoreExpr -> CoreExpr -> CoreExpr
mkCoreApp s fun arg
= fst $ mkCoreAppTyped s (fun, exprType fun) arg
--- | Construct an expression which represents the application of a number of
--- expressions to another. The leftmost expression in the list is applied first
+-- | Construct an expression which represents the application of one expression
+-- paired with its type to an argument. The result is paired with its type. This
+-- function is not exported and used in the definition of 'mkCoreApp' and
+-- 'mkCoreApps'.
-- Respects the let/app invariant by building a case expression where necessary
-- See CoreSyn Note [CoreSyn let/app invariant]
-mkCoreApps :: CoreExpr -> [CoreExpr] -> CoreExpr
-mkCoreApps fun args
- = fst $
- foldl' (mkCoreAppTyped doc_string) (fun, fun_ty) args
+mkCoreAppTyped :: SDoc -> (CoreExpr, Type) -> CoreExpr -> (CoreExpr, Type)
+mkCoreAppTyped _ (fun, fun_ty) (Type ty)
+ = (App fun (Type ty), piResultTy fun_ty ty)
+mkCoreAppTyped _ (fun, fun_ty) (Coercion co)
+ = (App fun (Coercion co), funResultTy fun_ty)
+mkCoreAppTyped d (fun, fun_ty) arg
+ = ASSERT2( isFunTy fun_ty, ppr fun $$ ppr arg $$ d )
+ (mkValApp fun arg arg_ty res_ty, res_ty)
where
- doc_string = ppr fun_ty $$ ppr fun $$ ppr args
- fun_ty = exprType fun
-
--- | Construct an expression which represents the application of a number of
--- expressions to that of a data constructor expression. The leftmost expression
--- in the list is applied first
-mkCoreConApps :: DataCon -> [CoreExpr] -> CoreExpr
-mkCoreConApps con args = mkCoreApps (Var (dataConWorkId con)) args
+ (arg_ty, res_ty) = splitFunTy fun_ty
-mk_val_app :: CoreExpr -> CoreExpr -> Type -> Type -> CoreExpr
+mkValApp :: CoreExpr -> CoreExpr -> Type -> Type -> CoreExpr
-- Build an application (e1 e2),
-- or a strict binding (case e2 of x -> e1 x)
-- using the latter when necessary to respect the let/app invariant
-- See Note [CoreSyn let/app invariant]
-mk_val_app fun arg arg_ty res_ty
+mkValApp fun arg arg_ty res_ty
| not (needsCaseBinding arg_ty arg)
= App fun arg -- The vastly common case
-
| otherwise
- = Case arg arg_id res_ty [(DEFAULT,[],App fun (Var arg_id))]
- where
- arg_id = mkWildValBinder arg_ty
- -- Lots of shadowing, but it doesn't matter,
- -- because 'fun ' should not have a free wild-id
- --
- -- This is Dangerous. But this is the only place we play this
- -- game, mk_val_app returns an expression that does not have
- -- a free wild-id. So the only thing that can go wrong
- -- is if you take apart this case expression, and pass a
- -- fragment of it as the fun part of a 'mk_val_app'.
+ = mkStrictApp fun arg arg_ty res_ty
+
+{- *********************************************************************
+* *
+ Building case expressions
+* *
+********************************************************************* -}
------------
mkWildEvBinder :: PredType -> EvVar
mkWildEvBinder pred = mkWildValBinder pred
@@ -197,10 +196,29 @@ mkWildValBinder ty = mkLocalIdOrCoVar wildCardName ty
mkWildCase :: CoreExpr -> Type -> Type -> [CoreAlt] -> CoreExpr
-- Make a case expression whose case binder is unused
--- The alts should not have any occurrences of WildId
+-- The alts and res_ty should not have any occurrences of WildId
mkWildCase scrut scrut_ty res_ty alts
= Case scrut (mkWildValBinder scrut_ty) res_ty alts
+mkStrictApp :: CoreExpr -> CoreExpr -> Type -> Type -> CoreExpr
+-- Build a strict application (case e2 of x -> e1 x)
+mkStrictApp fun arg arg_ty res_ty
+ = Case arg arg_id res_ty [(DEFAULT,[],App fun (Var arg_id))]
+ -- mkDefaultCase looks attractive here, and would be sound.
+ -- But it uses (exprType alt_rhs) to compute the result type,
+ -- whereas here we already know that the result type is res_ty
+ where
+ arg_id = mkWildValBinder arg_ty
+ -- Lots of shadowing, but it doesn't matter,
+ -- because 'fun' and 'res_ty' should not have a free wild-id
+ --
+ -- This is Dangerous. But this is the only place we play this
+ -- game, mkStrictApp returns an expression that does not have
+ -- a free wild-id. So the only way 'fun' could get a free wild-id
+ -- would be if you take apart this case expression (or some other
+ -- expression that uses mkWildValBinder, of which there are not
+ -- many), and pass a fragment of it as the fun part of a 'mkStrictApp'.
+
mkIfThenElse :: CoreExpr -> CoreExpr -> CoreExpr -> CoreExpr
mkIfThenElse guard then_expr else_expr
-- Not going to be refining, so okay to take the type of the "then" clause
@@ -219,17 +237,6 @@ castBottomExpr e res_ty
e_ty = exprType e
{-
-The functions from this point don't really do anything cleverer than
-their counterparts in CoreSyn, but they are here for consistency
--}
-
--- | Create a lambda where the given expression has a number of variables
--- bound over it. The leftmost binder is that bound by the outermost
--- lambda in the result
-mkCoreLams :: [CoreBndr] -> CoreExpr -> CoreExpr
-mkCoreLams = mkLams
-
-{-
************************************************************************
* *
\subsection{Making literals}
@@ -558,7 +565,7 @@ instance Outputable FloatBind where
wrapFloat :: FloatBind -> CoreExpr -> CoreExpr
wrapFloat (FloatLet defns) body = Let defns body
-wrapFloat (FloatCase e b con bs) body = Case e b (exprType body) [(con, bs, body)]
+wrapFloat (FloatCase e b con bs) body = mkSingleAltCase e b con bs body
-- | Applies the floats from right to left. That is @wrapFloats [b1, b2, …, bn]
-- u = let b1 in let b2 in … in let bn in u@
diff --git a/compiler/deSugar/DsUtils.hs b/compiler/deSugar/DsUtils.hs
index 7d39b4a3c6..14dab29b30 100644
--- a/compiler/deSugar/DsUtils.hs
+++ b/compiler/deSugar/DsUtils.hs
@@ -243,8 +243,7 @@ wrapBind new old body -- NB: this function must deal with term
| otherwise = Let (NonRec new (varToCoreExpr old)) body
seqVar :: Var -> CoreExpr -> CoreExpr
-seqVar var body = Case (Var var) var (exprType body)
- [(DEFAULT, [], body)]
+seqVar var body = mkDefaultCase (Var var) var body
mkCoLetMatchResult :: CoreBind -> MatchResult -> MatchResult
mkCoLetMatchResult bind = adjustMatchResult (mkCoreLet bind)
diff --git a/compiler/simplCore/SimplUtils.hs b/compiler/simplCore/SimplUtils.hs
index 63c216fce2..4eeb51ceaa 100644
--- a/compiler/simplCore/SimplUtils.hs
+++ b/compiler/simplCore/SimplUtils.hs
@@ -2212,8 +2212,10 @@ mkCase2 dflags scrut bndr alts_ty alts
; return (ex_tvs ++ arg_ids) }
mk_new_bndrs _ _ = return []
- re_sort :: [CoreAlt] -> [CoreAlt] -- Re-sort the alternatives to
- re_sort alts = sortBy cmpAlt alts -- preserve the #case_invariants#
+ re_sort :: [CoreAlt] -> [CoreAlt]
+ -- Sort the alternatives to re-establish
+ -- CoreSyn Note [Case expression invariants]
+ re_sort alts = sortBy cmpAlt alts
add_default :: [CoreAlt] -> [CoreAlt]
-- See Note [Literal cases]
diff --git a/compiler/stranal/WwLib.hs b/compiler/stranal/WwLib.hs
index f346324f4d..5e4d22857a 100644
--- a/compiler/stranal/WwLib.hs
+++ b/compiler/stranal/WwLib.hs
@@ -16,7 +16,7 @@ module WwLib ( mkWwBodies, mkWWstr, mkWorkerArgs
import GhcPrelude
import CoreSyn
-import CoreUtils ( exprType, mkCast )
+import CoreUtils ( exprType, mkCast, mkDefaultCase, mkSingleAltCase )
import Id
import IdInfo ( JoinArity )
import DataCon
@@ -1027,7 +1027,7 @@ mkWWcpr_help (data_con, inst_tys, arg_tys, co)
con_app = mkConApp2 data_con inst_tys [arg] `mkCast` mkSymCo co
; return ( True
- , \ wkr_call -> Case wkr_call arg (exprType con_app) [(DEFAULT, [], con_app)]
+ , \ wkr_call -> mkDefaultCase wkr_call arg con_app
, \ body -> mkUnpackCase body co work_uniq data_con [arg] (varToCoreExpr arg)
-- varToCoreExpr important here: arg can be a coercion
-- Lacking this caused #10658
@@ -1042,9 +1042,11 @@ mkWWcpr_help (data_con, inst_tys, arg_tys, co)
ubx_tup_ty = exprType ubx_tup_app
ubx_tup_app = mkCoreUbxTup (map fst arg_tys) (map varToCoreExpr args)
con_app = mkConApp2 data_con inst_tys args `mkCast` mkSymCo co
+ tup_con = tupleDataCon Unboxed (length arg_tys)
; return (True
- , \ wkr_call -> Case wkr_call wrap_wild (exprType con_app) [(DataAlt (tupleDataCon Unboxed (length arg_tys)), args, con_app)]
+ , \ wkr_call -> mkSingleAltCase wkr_call wrap_wild
+ (DataAlt tup_con) args con_app
, \ body -> mkUnpackCase body co work_uniq data_con args ubx_tup_app
, ubx_tup_ty ) }
@@ -1056,8 +1058,8 @@ mkUnpackCase :: CoreExpr -> Coercion -> Unique -> DataCon -> [Id] -> CoreExpr -
mkUnpackCase (Tick tickish e) co uniq con args body -- See Note [Profiling and unpacking]
= Tick tickish (mkUnpackCase e co uniq con args body)
mkUnpackCase scrut co uniq boxing_con unpk_args body
- = Case casted_scrut bndr (exprType body)
- [(DataAlt boxing_con, unpk_args, body)]
+ = mkSingleAltCase casted_scrut bndr
+ (DataAlt boxing_con) unpk_args body
where
casted_scrut = scrut `mkCast` co
bndr = mk_ww_local uniq (exprType casted_scrut, MarkedStrict)
diff --git a/compiler/types/Type.hs b/compiler/types/Type.hs
index 39a07c2fed..d8664eba62 100644
--- a/compiler/types/Type.hs
+++ b/compiler/types/Type.hs
@@ -2825,6 +2825,10 @@ occCheckExpand :: [Var] -> Type -> Maybe Type
-- of the given type variable. If the type is already syntactically
-- free of the variable, then the same type is returned.
occCheckExpand vs_to_avoid ty
+ | null vs_to_avoid -- Efficient shortcut
+ = Just ty -- Can happen, eg. CoreUtils.mkSingleAltCase
+
+ | otherwise
= go (mkVarSet vs_to_avoid, emptyVarEnv) ty
where
go :: (VarSet, VarEnv TyCoVar) -> Type -> Maybe Type
diff --git a/testsuite/tests/dependent/should_compile/all.T b/testsuite/tests/dependent/should_compile/all.T
index bbc32c8115..823244ebca 100644
--- a/testsuite/tests/dependent/should_compile/all.T
+++ b/testsuite/tests/dependent/should_compile/all.T
@@ -40,7 +40,7 @@ test('T12742', normal, compile, [''])
# (1) Use -fexternal-interpreter, or
# (2) Build the program twice: once with -dynamic, and then
# with -prof using -osuf to set a different object file suffix.
-test('T13910', [expect_broken_for(16537, ['optasm']), omit_ways(['profasm'])], compile, [''])
+test('T13910', [omit_ways(['profasm'])], compile, [''])
test('T13938', [req_th, extra_files(['T13938a.hs'])], makefile_test, ['T13938'])
test('T14556', normal, compile, [''])
test('T14720', normal, compile, [''])
diff --git a/testsuite/tests/indexed-types/should_compile/T17056.hs b/testsuite/tests/indexed-types/should_compile/T17056.hs
new file mode 100644
index 0000000000..d22d1dfcdc
--- /dev/null
+++ b/testsuite/tests/indexed-types/should_compile/T17056.hs
@@ -0,0 +1,47 @@
+-- This test tripped Core Lint by producing a
+-- case expression whose 'type' field included an
+-- out of scope variable because of a phantom type
+-- synonym
+
+{-# OPTIONS_GHC -O #-}
+
+{-# LANGUAGE AllowAmbiguousTypes #-}
+{-# LANGUAGE DataKinds #-}
+{-# LANGUAGE GADTs #-}
+{-# LANGUAGE PolyKinds #-}
+{-# LANGUAGE RankNTypes #-}
+{-# LANGUAGE ScopedTypeVariables #-}
+{-# LANGUAGE TypeApplications #-}
+{-# LANGUAGE TypeFamilies #-}
+{-# LANGUAGE TypeOperators #-}
+module Bug where
+
+import Data.Kind (Type)
+import Data.Type.Equality ((:~:)(..))
+
+type IsInt a = Int :~: a
+
+data Sing :: forall (b :: Type). IsInt b -> Type where
+ SRefl :: Sing Refl
+
+data SomeSing :: Type -> Type where
+ SomeSing :: Sing (x :: IsInt b) -> SomeSing b
+
+type WhyCastWith (e :: IsInt b) = b
+-- Type /synonym/
+-- WhyCastWith b (e :: IsInt b) = b
+
+type family Apply (e :: IsInt b) :: Type
+type instance Apply (e :: IsInt b) = WhyCastWith e
+
+-- axiom Apply (b :: *) (e :: IsInt b) ~ WhyCastWith e
+
+(~>:~:) :: forall (b :: Type) (r :: IsInt b).
+ Sing r
+ -> Apply r
+(~>:~:) SRefl = let w = w in w
+
+castWith :: forall b. IsInt b -> b
+castWith eq
+ = case (case eq of Refl -> SomeSing SRefl) :: SomeSing b of
+ SomeSing SRefl -> (~>:~:) @b SRefl
diff --git a/testsuite/tests/indexed-types/should_compile/all.T b/testsuite/tests/indexed-types/should_compile/all.T
index 7eeeda6d59..1ee797ca41 100644
--- a/testsuite/tests/indexed-types/should_compile/all.T
+++ b/testsuite/tests/indexed-types/should_compile/all.T
@@ -292,3 +292,4 @@ test('T16356_Compile2', normal, compile, [''])
test('T16632', normal, compile, ['-Wunused-type-patterns -fdiagnostics-show-caret'])
test('T16828', normal, compile, [''])
test('T17008b', normal, compile, [''])
+test('T17056', normal, compile, [''])