Improve error message for newtypes and deriving clauses

Summary:
Change the error message generated when a deriving clause related to a newtype
fails to always suggested trying GeneralizedNewtypeDeriving, even in
situations where it may not work. Fixes #9600.

Test Plan: testsuite/deriving/should_fail/9600.hs

Reviewers: austin, bgamari, simonpj

Rebased-by: bgamari

Reviewed By: simonpj

Subscribers: bgamari, hvr, simonmar, carter

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

GHC Trac Issues: #9600

14 files changed, 75 insertions, 45 deletions

diff --git a/compiler/typecheck/TcDeriv.hs b/compiler/typecheck/TcDeriv.hs
index 429ba78638..6395ddf95d 100644
--- a/compiler/typecheck/TcDeriv.hs
+++ b/compiler/typecheck/TcDeriv.hs
@@ -1180,11 +1180,14 @@ cases, standalone deriving can still be used.
 -- the data constructors - but we need to be careful to fall back to the
 -- family tycon (with indexes) in error messages.
 
-data DerivStatus = CanDerive
+data DerivStatus = CanDerive                 -- Standard class, can derive
                  | DerivableClassError SDoc  -- Standard class, but can't do it
                  | DerivableViaInstance      -- See Note [Deriving any class]
                  | NonDerivableClass SDoc    -- Non-standard class
 
+-- A "standard" class is one defined in the Haskell report which GHC knows how
+-- to generate code for, such as Eq, Ord, Ix, etc.
+
 checkSideConditions :: DynFlags -> DerivContext -> Class -> [TcType]
                     -> TyCon -> [Type] -- tycon and its parameters
                     -> DerivStatus
@@ -1203,7 +1206,9 @@ classArgsErr :: Class -> [Type] -> SDoc
 classArgsErr cls cls_tys = quotes (ppr (mkClassPred cls cls_tys)) <+> ptext (sLit "is not a class")
 
 nonStdErr :: Class -> SDoc
-nonStdErr cls = quotes (ppr cls) <+> ptext (sLit "is not a derivable class")
+nonStdErr cls =
+      quotes (ppr cls)
+  <+> ptext (sLit "is not a standard derivable class (Eq, Show, etc.)")
 
 sideConditions :: DerivContext -> Class -> Maybe Condition
 sideConditions mtheta cls
@@ -1547,15 +1552,18 @@ mkNewTypeEqn dflags overlap_mode tvs
   = case checkSideConditions dflags mtheta cls cls_tys rep_tycon rep_tc_args of
       -- Error with standard class
       DerivableClassError msg
-        | might_derive_via_coercible -> bale_out (msg $$ suggest_nd)
+        | might_derive_via_coercible -> bale_out (msg $$ suggest_gnd)
         | otherwise                  -> bale_out msg
+
       -- Must use newtype deriving or DeriveAnyClass
       NonDerivableClass _msg
         -- Too hard, even with newtype deriving
         | newtype_deriving           -> bale_out cant_derive_err
         -- Try newtype deriving!
-        | might_derive_via_coercible -> bale_out (non_std $$ suggest_nd)
-        | otherwise                  -> bale_out non_std
+        -- Here we suggest GeneralizedNewtypeDeriving even in cases where it may
+        -- not be applicable. See Trac #9600.
+        | otherwise                  -> bale_out (non_std $$ suggest_gnd)
+
       -- CanDerive/DerivableViaInstance
       _ -> do when (newtype_deriving && deriveAnyClass) $
                 addWarnTc (sep [ ptext (sLit "Both DeriveAnyClass and GeneralizedNewtypeDeriving are enabled")
@@ -1569,8 +1577,8 @@ mkNewTypeEqn dflags overlap_mode tvs
         bale_out    = bale_out' newtype_deriving
         bale_out' b = failWithTc . derivingThingErr b cls cls_tys inst_ty
 
-        non_std    = nonStdErr cls
-        suggest_nd = ptext (sLit "Try GeneralizedNewtypeDeriving for GHC's newtype-deriving extension")
+        non_std     = nonStdErr cls
+        suggest_gnd = ptext (sLit "Try GeneralizedNewtypeDeriving for GHC's newtype-deriving extension")
 
         -- Here is the plan for newtype derivings.  We see
         --        newtype T a1...an = MkT (t ak+1...an) deriving (.., C s1 .. sm, ...)
@@ -2146,7 +2154,7 @@ derivingThingErr newtype_deriving clas tys ty why
           $$ nest 2 extra) <> colon,
          nest 2 why]
   where
-    extra | newtype_deriving = ptext (sLit "(even with cunning newtype deriving)")
+    extra | newtype_deriving = ptext (sLit "(even with cunning GeneralizedNewtypeDeriving)")
           | otherwise        = Outputable.empty
     pred = mkClassPred clas (tys ++ [ty])
 
diff --git a/testsuite/tests/deriving/should_fail/T2721.stderr b/testsuite/tests/deriving/should_fail/T2721.stderr
index 908db94dbe..693ccd2dbd 100644
--- a/testsuite/tests/deriving/should_fail/T2721.stderr
+++ b/testsuite/tests/deriving/should_fail/T2721.stderr
@@ -1,6 +1,6 @@
 
-T2721.hs:15:28:
+T2721.hs:15:28: error:
     Can't make a derived instance of ‘C N’
-      (even with cunning newtype deriving):
+      (even with cunning GeneralizedNewtypeDeriving):
       the class has associated types
     In the newtype declaration for ‘N’
diff --git a/testsuite/tests/deriving/should_fail/T3833.stderr b/testsuite/tests/deriving/should_fail/T3833.stderr
index 4c6bf564ea..da7da919bc 100644
--- a/testsuite/tests/deriving/should_fail/T3833.stderr
+++ b/testsuite/tests/deriving/should_fail/T3833.stderr
@@ -1,6 +1,6 @@
 
-T3833.hs:9:1:
+T3833.hs:9:1: error:
     Can't make a derived instance of ‘Monoid (DecodeMap e)’:
-      ‘Monoid’ is not a derivable class
+      ‘Monoid’ is not a standard derivable class (Eq, Show, etc.)
       Try GeneralizedNewtypeDeriving for GHC's newtype-deriving extension
     In the stand-alone deriving instance for ‘Monoid (DecodeMap e)’
diff --git a/testsuite/tests/deriving/should_fail/T3834.stderr b/testsuite/tests/deriving/should_fail/T3834.stderr
index d732124454..3eec64a6c5 100644
--- a/testsuite/tests/deriving/should_fail/T3834.stderr
+++ b/testsuite/tests/deriving/should_fail/T3834.stderr
@@ -1,6 +1,6 @@
 
-T3834.hs:8:1:
+T3834.hs:8:1: error:
     Can't make a derived instance of ‘C T’:
-      ‘C’ is not a derivable class
+      ‘C’ is not a standard derivable class (Eq, Show, etc.)
       Try GeneralizedNewtypeDeriving for GHC's newtype-deriving extension
     In the stand-alone deriving instance for ‘C T’
diff --git a/testsuite/tests/deriving/should_fail/T9600-1.hs b/testsuite/tests/deriving/should_fail/T9600-1.hs
new file mode 100644
index 0000000000..8116d08728
--- /dev/null
+++ b/testsuite/tests/deriving/should_fail/T9600-1.hs
@@ -0,0 +1,5 @@
+{-# LANGUAGE GeneralizedNewtypeDeriving #-}
+
+import Control.Applicative
+
+newtype Foo a = Foo (a -> a) deriving Applicative
diff --git a/testsuite/tests/deriving/should_fail/T9600-1.stderr b/testsuite/tests/deriving/should_fail/T9600-1.stderr
new file mode 100644
index 0000000000..88d24079a0
--- /dev/null
+++ b/testsuite/tests/deriving/should_fail/T9600-1.stderr
@@ -0,0 +1,6 @@
+
+T9600-1.hs:5:39: error:
+    Can't make a derived instance of ‘Applicative Foo’
+      (even with cunning GeneralizedNewtypeDeriving):
+      cannot eta-reduce the representation type enough
+    In the newtype declaration for ‘Foo’
diff --git a/testsuite/tests/deriving/should_fail/T9600.hs b/testsuite/tests/deriving/should_fail/T9600.hs
new file mode 100644
index 0000000000..a6663ce8c8
--- /dev/null
+++ b/testsuite/tests/deriving/should_fail/T9600.hs
@@ -0,0 +1,3 @@
+import Control.Applicative
+
+newtype Foo a = Foo (a -> a) deriving Applicative
diff --git a/testsuite/tests/deriving/should_fail/T9600.stderr b/testsuite/tests/deriving/should_fail/T9600.stderr
new file mode 100644
index 0000000000..2e88277f54
--- /dev/null
+++ b/testsuite/tests/deriving/should_fail/T9600.stderr
@@ -0,0 +1,6 @@
+
+T9600.hs:3:39: error:
+    Can't make a derived instance of ‘Applicative Foo’:
+      ‘Applicative’ is not a standard derivable class (Eq, Show, etc.)
+      Try GeneralizedNewtypeDeriving for GHC's newtype-deriving extension
+    In the newtype declaration for ‘Foo’
diff --git a/testsuite/tests/deriving/should_fail/all.T b/testsuite/tests/deriving/should_fail/all.T
index 2e2511305e..94120d2912 100644
--- a/testsuite/tests/deriving/should_fail/all.T
+++ b/testsuite/tests/deriving/should_fail/all.T
@@ -51,6 +51,8 @@ test('T6147', normal, compile_fail, [''])
 test('T8851', normal, compile_fail, [''])
 test('T9071', normal, multimod_compile_fail, ['T9071',''])
 test('T9071_2', normal, compile_fail, [''])
+test('T9600',  normal, compile_fail, [''])
+test('T9600-1', normal, compile_fail, [''])
 test('T9687', normal, compile_fail, [''])
 
 test('T8984', normal, compile_fail, [''])
diff --git a/testsuite/tests/deriving/should_fail/drvfail009.stderr b/testsuite/tests/deriving/should_fail/drvfail009.stderr
index b9dd90c758..563bc5aa8a 100644
--- a/testsuite/tests/deriving/should_fail/drvfail009.stderr
+++ b/testsuite/tests/deriving/should_fail/drvfail009.stderr
@@ -1,23 +1,23 @@
 
-drvfail009.hs:10:31:
+drvfail009.hs:10:31: error:
     Expecting one more argument to ‘C’
     Expected kind ‘* -> Constraint’,
       but ‘C’ has kind ‘* -> * -> Constraint’
     In the newtype declaration for ‘T1’
 
-drvfail009.hs:13:31:
+drvfail009.hs:13:31: error:
     Cannot derive well-kinded instance of form ‘Monad (T2 ...)’
       Class ‘Monad’ expects an argument of kind ‘* -> *’
     In the newtype declaration for ‘T2’
 
-drvfail009.hs:16:33:
+drvfail009.hs:16:33: error:
     Can't make a derived instance of ‘Monad T3’
-      (even with cunning newtype deriving):
+      (even with cunning GeneralizedNewtypeDeriving):
       cannot eta-reduce the representation type enough
     In the newtype declaration for ‘T3’
 
-drvfail009.hs:19:42:
+drvfail009.hs:19:42: error:
     Can't make a derived instance of ‘Monad T4’
-      (even with cunning newtype deriving):
+      (even with cunning GeneralizedNewtypeDeriving):
       cannot eta-reduce the representation type enough
     In the newtype declaration for ‘T4’
diff --git a/testsuite/tests/generics/T5462No1.stderr b/testsuite/tests/generics/T5462No1.stderr
index 87ef888461..7c1aec8d79 100644
--- a/testsuite/tests/generics/T5462No1.stderr
+++ b/testsuite/tests/generics/T5462No1.stderr
@@ -1,20 +1,20 @@
 [1 of 2] Compiling GFunctor         ( GFunctor/GFunctor.hs, out_T5462No1/GFunctor.o )
 [2 of 2] Compiling T5462No1         ( T5462No1.hs, out_T5462No1/T5462No1.o )
 
-T5462No1.hs:24:42:
+T5462No1.hs:24:42: error:
     Can't make a derived instance of ‘GFunctor F’:
-      ‘GFunctor’ is not a derivable class
+      ‘GFunctor’ is not a standard derivable class (Eq, Show, etc.)
       Try GeneralizedNewtypeDeriving for GHC's newtype-deriving extension
     In the newtype declaration for ‘F’
 
-T5462No1.hs:26:23:
+T5462No1.hs:26:23: error:
     Can't make a derived instance of ‘C1 G’:
-      ‘C1’ is not a derivable class
+      ‘C1’ is not a standard derivable class (Eq, Show, etc.)
       Try enabling DeriveAnyClass
     In the data declaration for ‘G’
 
-T5462No1.hs:27:23:
+T5462No1.hs:27:23: error:
     Can't make a derived instance of ‘C2 H’:
-      ‘C2’ is not a derivable class
+      ‘C2’ is not a standard derivable class (Eq, Show, etc.)
       Try enabling DeriveAnyClass
     In the data declaration for ‘H’
diff --git a/testsuite/tests/module/mod53.stderr b/testsuite/tests/module/mod53.stderr
index 2630e9cc7c..a4c176d11a 100644
--- a/testsuite/tests/module/mod53.stderr
+++ b/testsuite/tests/module/mod53.stderr
@@ -1,6 +1,6 @@
 
-mod53.hs:4:22:
+mod53.hs:4:22: error:
     Can't make a derived instance of ‘C T’:
-      ‘C’ is not a derivable class
+      ‘C’ is not a standard derivable class (Eq, Show, etc.)
       Try enabling DeriveAnyClass
     In the data declaration for ‘T’
diff --git a/testsuite/tests/parser/should_fail/readFail039.stderr b/testsuite/tests/parser/should_fail/readFail039.stderr
index 81d5001624..91b9a16553 100644
--- a/testsuite/tests/parser/should_fail/readFail039.stderr
+++ b/testsuite/tests/parser/should_fail/readFail039.stderr
@@ -1,6 +1,6 @@
 
-readFail039.hs:8:14:
+readFail039.hs:8:14: error:
     Can't make a derived instance of ‘C Foo’:
-      ‘C’ is not a derivable class
+      ‘C’ is not a standard derivable class (Eq, Show, etc.)
       Try GeneralizedNewtypeDeriving for GHC's newtype-deriving extension
     In the newtype declaration for ‘Foo’
diff --git a/testsuite/tests/safeHaskell/ghci/p16.stderr b/testsuite/tests/safeHaskell/ghci/p16.stderr
index e14727bc49..893cf2d8ee 100644
--- a/testsuite/tests/safeHaskell/ghci/p16.stderr
+++ b/testsuite/tests/safeHaskell/ghci/p16.stderr
@@ -1,15 +1,15 @@
-

-<no location info>: warning:

-    -XGeneralizedNewtypeDeriving is not allowed in Safe Haskell; ignoring -XGeneralizedNewtypeDeriving

-

-<interactive>:15:29: error:

-    Can't make a derived instance of ‘Op T2’:

-      ‘Op’ is not a derivable class

-      Try GeneralizedNewtypeDeriving for GHC's newtype-deriving extension

-    In the newtype declaration for ‘T2’

-

-<interactive>:18:9: error:

-    Data constructor not in scope: T2 :: T -> t

-    Perhaps you meant ‘T1’ (line 12)

-

-<interactive>:21:4: error: Variable not in scope: y

+
+<no location info>: warning:
+    -XGeneralizedNewtypeDeriving is not allowed in Safe Haskell; ignoring -XGeneralizedNewtypeDeriving
+
+<interactive>:15:29: error:
+    Can't make a derived instance of ‘Op T2’:
+      ‘Op’ is not a standard derivable class (Eq, Show, etc.)
+      Try GeneralizedNewtypeDeriving for GHC's newtype-deriving extension
+    In the newtype declaration for ‘T2’
+
+<interactive>:18:9: error:
+    Data constructor not in scope: T2 :: T -> t
+    Perhaps you meant ‘T1’ (line 12)
+
+<interactive>:21:4: error: Variable not in scope: y