Improve the handling of deriving, in error cases

I'd been too ambitious with error handling for 'deriving', and got it
entirely wrong.  This fixes it.  See extensive
	Note [Exotic derived instance contexts]
in TcSimplify.  (Most of the extra lines are comments!)

diff --git a/compiler/typecheck/TcDeriv.lhs b/compiler/typecheck/TcDeriv.lhs
index 6934eb1..b973ec4 100644 (file)
--- a/compiler/typecheck/TcDeriv.lhs
+++ b/compiler/typecheck/TcDeriv.lhs
@@ -866,11 +866,11 @@ solveDerivEqns overlap_flag orig_eqns
     gen_soln :: DerivEqn -> TcM [PredType]
     gen_soln (loc, orig, _, tyvars, clas, inst_ty, deriv_rhs)
       = setSrcSpan loc $
+       addErrCtxt (derivInstCtxt clas [inst_ty]) $ 
        do { theta <- tcSimplifyDeriv orig tyvars deriv_rhs
-          ; addErrCtxt (derivInstCtxt theta clas [inst_ty]) $ 
-       do { checkNoErrs (checkValidInstance tyvars theta clas [inst_ty])
-               -- See Note [Deriving context]
-               -- If this fails, don't continue
+               -- checkValidInstance tyvars theta clas [inst_ty]
+               -- Not necessary; see Note [Exotic derived instance contexts]
+               --                in TcSimplify
 
                  -- Check for a bizarre corner case, when the derived instance decl should
                  -- have form  instance C a b => D (T a) where ...
@@ -884,7 +884,7 @@ solveDerivEqns overlap_flag orig_eqns
                -- Claim: the result instance declaration is guaranteed valid
                -- Hence no need to call:
                --   checkValidInstance tyvars theta clas inst_tys
-          ; return (sortLe (<=) theta) } }     -- Canonicalise before returning the solution
+          ; return (sortLe (<=) theta) }       -- Canonicalise before returning the solution
 
     ------------------------------------------------------------------
     mk_inst_spec :: DerivEqn -> DerivSoln -> Instance
@@ -903,25 +903,6 @@ extendLocalInstEnv dfuns thing_inside
       ; setGblEnv env' thing_inside }
 \end{code}
 
-Note [Deriving context]
-~~~~~~~~~~~~~~~~~~~~~~~
-With -fglasgow-exts, we allow things like (C Int a) in the simplified
-context for a derived instance declaration, because at a use of this
-instance, we might know that a=Bool, and have an instance for (C Int
-Bool)
-
-We nevertheless insist that each predicate meets the termination
-conditions. If not, the deriving mechanism generates larger and larger
-constraints.  Example:
-  data Succ a = S a
-  data Seq a = Cons a (Seq (Succ a)) | Nil deriving Show
-
-Note the lack of a Show instance for Succ.  First we'll generate
-  instance (Show (Succ a), Show a) => Show (Seq a)
-and then
-  instance (Show (Succ (Succ a)), Show (Succ a), Show a) => Show (Seq a)
-and so on.  Instead we want to complain of no instance for (Show (Succ a)).
-  
 
 %************************************************************************
 %*                                                                     *
@@ -1137,10 +1118,8 @@ derivingThingErr clas tys ty why
 standaloneCtxt :: LHsType Name -> SDoc
 standaloneCtxt ty = ptext SLIT("In the stand-alone deriving instance for") <+> quotes (ppr ty)
 
-derivInstCtxt theta clas inst_tys
-  = hang (ptext SLIT("In the derived instance:"))
-        2 (pprThetaArrow theta <+> pprClassPred clas inst_tys)
--- Used for the ...Thetas variants; all top level
+derivInstCtxt clas inst_tys
+  = ptext SLIT("When deriving the instance for") <+> parens (pprClassPred clas inst_tys)
 
 badDerivedPred pred
   = vcat [ptext SLIT("Can't derive instances where the instance context mentions"),

diff --git a/compiler/typecheck/TcSimplify.lhs b/compiler/typecheck/TcSimplify.lhs
index 32fe6cf..3f25a4c 100644 (file)
--- a/compiler/typecheck/TcSimplify.lhs
+++ b/compiler/typecheck/TcSimplify.lhs
@@ -2522,19 +2522,60 @@ tcSimplifyDeriv orig tyvars theta
        ; wanteds <- newDictBndrsO orig (substTheta tenv theta)
        ; (irreds, _) <- tryHardCheckLoop doc wanteds
 
+       ; let (tv_dicts, others) = partition isTyVarDict irreds
+       ; addNoInstanceErrs others
+
        ; let rev_env = zipTopTvSubst tvs (mkTyVarTys tyvars)
-             simpl_theta = substTheta rev_env (map dictPred irreds)
+             simpl_theta = substTheta rev_env (map dictPred tv_dicts)
                -- This reverse-mapping is a pain, but the result
                -- should mention the original TyVars not TcTyVars
 
-       -- NB: the caller will further check the tv_dicts for
-       --     legal instance-declaration form
-
        ; return simpl_theta }
   where
     doc = ptext SLIT("deriving classes for a data type")
 \end{code}
 
+Note [Exotic derived instance contexts]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Consider
+       data T a b c = MkT (Foo a b c) deriving( Eq )
+       instance (C Int a, Eq b, Eq c) => Eq (Foo a b c)
+
+Notice that this instance (just) satisfies the Paterson termination 
+conditions.  Then we *could* derive an instance decl like this:
+
+       instance (C Int a, Eq b, Eq c) => Eq (T a b c) 
+
+even though there is no instance for (C Int a), because there just
+*might* be an instance for, say, (C Int Bool) at a site where we
+need the equality instance for T's.  
+
+However, this seems pretty exotic, and it's quite tricky to allow
+this, and yet give sensible error messages in the (much more common)
+case where we really want that instance decl for C.
+
+So for now we simply require that the derived instance context
+should have only type-variable constraints.
+
+Here is another example:
+       data Fix f = In (f (Fix f)) deriving( Eq )
+Here, if we are prepared to allow -fallow-undecidable-instances we
+could derive the instance
+       instance Eq (f (Fix f)) => Eq (Fix f)
+but this is so delicate that I don't think it should happen inside
+'deriving'. If you want this, write it yourself!
+
+NB: if you want to lift this condition, make sure you still meet the
+termination conditions!  If not, the deriving mechanism generates
+larger and larger constraints.  Example:
+  data Succ a = S a
+  data Seq a = Cons a (Seq (Succ a)) | Nil deriving Show
+
+Note the lack of a Show instance for Succ.  First we'll generate
+  instance (Show (Succ a), Show a) => Show (Seq a)
+and then
+  instance (Show (Succ (Succ a)), Show (Succ a), Show a) => Show (Seq a)
+and so on.  Instead we want to complain of no instance for (Show (Succ a)).
 
 
 @tcSimplifyDefault@ just checks class-type constraints, essentially;