Commit 65b185d4 authored by Simon Peyton Jones's avatar Simon Peyton Jones
Browse files

Be less aggressive about fragile-context warrnings

In the implementation of WarnSimplifiableClassConstraints, be
less aggressive about reporting a problem. We were complaining
about a "fragile" case that in fact was not fragile.

See Note [Simplifiable given constraints] in TcValidity.

This fixes Trac #13526.
parent 2ab7f626
......@@ -41,7 +41,7 @@ import HsSyn -- HsType
import TcRnMonad -- TcType, amongst others
import TcEnv ( tcGetInstEnvs )
import FunDeps
import InstEnv ( ClsInst, lookupInstEnv, isOverlappable )
import InstEnv ( InstMatch, lookupInstEnv )
import FamInstEnv ( isDominatedBy, injectiveBranches,
InjectivityCheckResult(..) )
import FamInst ( makeInjectivityErrors )
......@@ -810,7 +810,8 @@ check_class_pred env dflags ctxt pred cls tys
| otherwise
= do { check_arity
; check_simplifiable_class_constraint
; warn_simp <- woptM Opt_WarnSimplifiableClassConstraints
; when warn_simp check_simplifiable_class_constraint
; checkTcM arg_tys_ok (predTyVarErr env pred) }
where
check_arity = checkTc (classArity cls == length tys)
......@@ -833,25 +834,22 @@ check_class_pred env dflags ctxt pred cls tys
| DataTyCtxt {} <- ctxt -- Don't do this check for the "stupid theta"
= return () -- of a data type declaration
| otherwise
= do { instEnvs <- tcGetInstEnvs
; let (matches, _, _) = lookupInstEnv False instEnvs cls tys
bad_matches = [ inst | (inst,_) <- matches
, not (isOverlappable inst) ]
; warnIf (Reason Opt_WarnSimplifiableClassConstraints)
(not (null bad_matches))
(simplifiable_constraint_warn bad_matches) }
simplifiable_constraint_warn :: [ClsInst] -> SDoc
simplifiable_constraint_warn (match : _)
= do { envs <- tcGetInstEnvs
; case lookupInstEnv False envs cls tys of
([m], [], _) -> addWarnTc (Reason Opt_WarnSimplifiableClassConstraints)
(simplifiable_constraint_warn m)
_ -> return () }
simplifiable_constraint_warn :: InstMatch -> SDoc
simplifiable_constraint_warn (match, _)
= vcat [ hang (text "The constraint" <+> quotes (ppr (tidyType env pred)))
2 (text "matches an instance declaration")
, ppr match
, hang (text "This makes type inference for inner bindings fragile;")
2 (text "either use MonoLocalBinds, or simplify it using the instance") ]
simplifiable_constraint_warn [] = pprPanic "check_class_pred" (ppr pred)
{- Note [Simplifiable given constraints]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A type signature like
f :: Eq [(a,b)] => a -> b
is very fragile, for reasons described at length in TcInteract
......@@ -862,9 +860,27 @@ fragility. But if we /infer/ the type of a local let-binding, things
can go wrong (Trac #11948 is an example, discussed in the Note).
So this warning is switched on only if we have NoMonoLocalBinds; in
that case the warning discourages uses from writing simplifiable class
constraints, at least unless the top-level instance is explicitly
declared as OVERLAPPABLE.
that case the warning discourages users from writing simplifiable
class constraints.
The warning only fires if the constraint in the signature
matches the top-level instances in only one way, and with no
unifiers -- that is, under the same circumstances that
TcInteract.matchInstEnv fires an interaction with the top
level instances. For example (Trac #13526), consider
instance {-# OVERLAPPABLE #-} Eq (T a) where ...
instance Eq (T Char) where ..
f :: Eq (T a) => ...
We don't want to complain about this, even though the context
(Eq (T a)) matches an instance, because the user may be
deliberately deferring the choice so that the Eq (T Char)
has a chance to fire when 'f' is called. And the fragility
only matters when there's a risk that the instance might
fire instead of the local 'given'; and there is no such
risk in this case. Just use the same rules as for instance
firing!
-}
-------------------------
......
......@@ -7,7 +7,9 @@ Dependent packages: [base-4.10.0.0, ghc-prim-0.5.0.0,
integer-gmp-1.0.0.1]
SomethingShowable.hs:5:1: warning: [-Wsimplifiable-class-constraints (in -Wdefault)]
The constraint ‘Show Bool’ matches an instance declaration
instance Show Bool -- Defined in ‘GHC.Show’
This makes type inference for inner bindings fragile;
either use MonoLocalBinds, or simplify it using the instance
• The constraint ‘Show Bool’ matches an instance declaration
instance Show Bool -- Defined in ‘GHC.Show’
This makes type inference for inner bindings fragile;
either use MonoLocalBinds, or simplify it using the instance
• When checking the inferred type
somethingShowable :: Show Bool => Bool -> String
{-# LANGUAGE FlexibleContexts, MultiParamTypeClasses, FlexibleInstances #-}
module T13526 where
class C a where
op :: a -> a
instance {-# OVERLAPPING #-} C [Char] where
op x = x
instance C a => C [a] where
op (x:xs) = [op x]
instance C a => C (Maybe a) where
op x = error "urk"
-- We should get no complaint
foo :: C [a] => a -> [a]
foo x = op [x]
bar :: C (Maybe a) => a -> Maybe a
bar x = op (Just x)
T13526.hs:21:8: warning: [-Wsimplifiable-class-constraints (in -Wdefault)]
• The constraint ‘C (Maybe a)’ matches an instance declaration
instance C a => C (Maybe a) -- Defined at T13526.hs:14:10
This makes type inference for inner bindings fragile;
either use MonoLocalBinds, or simplify it using the instance
• In the type signature: bar :: C (Maybe a) => a -> Maybe a
......@@ -553,4 +553,5 @@ test('T13490', normal, compile, [''])
test('T13474', normal, compile, [''])
test('T13524', expect_broken(13524), compile, [''])
test('T13509', normal, compile, [''])
test('T13526', normal, compile, [''])
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