From 426b0ddc79890f80a8ceeef135371533f066b9ba Mon Sep 17 00:00:00 2001
From: Ryan Scott <ryan.gl.scott@gmail.com>
Date: Mon, 14 Oct 2019 11:21:45 -0400
Subject: Don't skip validity checks for built-in classes (#17355)

Issue #17355 occurred because the control flow for
`TcValidity.check_valid_inst_head` was structured in such a way that
whenever it checked a special, built-in class (like `Generic` or
`HasField`), it would skip the most important check of all:
`checkValidTypePats`, which rejects nonsense like this:

```hs
instance Generic (forall a. a)
```

This fixes the issue by carving out `checkValidTypePats` from
`check_valid_inst_head` so that `checkValidTypePats` is always
invoked. `check_valid_inst_head` has also been renamed to
`check_special_inst_head` to reflect its new purpose of _only_
checking for instances headed by special classes.

Fixes #17355.
---
 compiler/typecheck/TcValidity.hs                    | 11 ++++++-----
 testsuite/tests/typecheck/should_fail/T17355.hs     | 11 +++++++++++
 testsuite/tests/typecheck/should_fail/T17355.stderr |  9 +++++++++
 testsuite/tests/typecheck/should_fail/all.T         |  1 +
 4 files changed, 27 insertions(+), 5 deletions(-)
 create mode 100644 testsuite/tests/typecheck/should_fail/T17355.hs
 create mode 100644 testsuite/tests/typecheck/should_fail/T17355.stderr

diff --git a/compiler/typecheck/TcValidity.hs b/compiler/typecheck/TcValidity.hs
index f97c9a7f27..e72aa7f7b0 100644
--- a/compiler/typecheck/TcValidity.hs
+++ b/compiler/typecheck/TcValidity.hs
@@ -1468,7 +1468,8 @@ checkValidInstHead ctxt clas cls_args
   = do { dflags   <- getDynFlags
        ; is_boot  <- tcIsHsBootOrSig
        ; is_sig   <- tcIsHsig
-       ; check_valid_inst_head dflags is_boot is_sig ctxt clas cls_args
+       ; check_special_inst_head dflags is_boot is_sig ctxt clas cls_args
+       ; checkValidTypePats (classTyCon clas) cls_args
        }
 
 {-
@@ -1496,10 +1497,10 @@ in hsig files, where `is_sig` is True.
 
 -}
 
-check_valid_inst_head :: DynFlags -> Bool -> Bool
-                      -> UserTypeCtxt -> Class -> [Type] -> TcM ()
+check_special_inst_head :: DynFlags -> Bool -> Bool
+                        -> UserTypeCtxt -> Class -> [Type] -> TcM ()
 -- Wow!  There are a surprising number of ad-hoc special cases here.
-check_valid_inst_head dflags is_boot is_sig ctxt clas cls_args
+check_special_inst_head dflags is_boot is_sig ctxt clas cls_args
 
   -- If not in an hs-boot file, abstract classes cannot have instances
   | isAbstractClass clas
@@ -1549,7 +1550,7 @@ check_valid_inst_head dflags is_boot is_sig ctxt clas cls_args
   = failWithTc (instTypeErr clas cls_args msg)
 
   | otherwise
-  = checkValidTypePats (classTyCon clas) cls_args
+  = pure ()
   where
     clas_nm = getName clas
     ty_args = filterOutInvisibleTypes (classTyCon clas) cls_args
diff --git a/testsuite/tests/typecheck/should_fail/T17355.hs b/testsuite/tests/typecheck/should_fail/T17355.hs
new file mode 100644
index 0000000000..cf9fb651cc
--- /dev/null
+++ b/testsuite/tests/typecheck/should_fail/T17355.hs
@@ -0,0 +1,11 @@
+{-# LANGUAGE RankNTypes, DataKinds #-}
+module T17355 where
+
+import GHC.Generics
+import GHC.Records
+
+data Foo = Foo { poly :: forall a. a -> a }
+
+instance Generic (forall a . a)
+instance HasField "myPoly" Foo (forall a. a -> a) where
+    getField (Foo x) = x
diff --git a/testsuite/tests/typecheck/should_fail/T17355.stderr b/testsuite/tests/typecheck/should_fail/T17355.stderr
new file mode 100644
index 0000000000..5212ef8787
--- /dev/null
+++ b/testsuite/tests/typecheck/should_fail/T17355.stderr
@@ -0,0 +1,9 @@
+
+T17355.hs:9:10: error:
+    • Illegal polymorphic type: forall a. a
+    • In the instance declaration for ‘Generic (forall a. a)’
+
+T17355.hs:10:10: error:
+    • Illegal polymorphic type: forall a. a -> a
+    • In the instance declaration for
+        ‘HasField "myPoly" Foo (forall a. a -> a)’
diff --git a/testsuite/tests/typecheck/should_fail/all.T b/testsuite/tests/typecheck/should_fail/all.T
index 6b66d41975..18e94f7974 100644
--- a/testsuite/tests/typecheck/should_fail/all.T
+++ b/testsuite/tests/typecheck/should_fail/all.T
@@ -544,3 +544,4 @@ test('UnliftedNewtypesMultiFieldGadt', normal, compile_fail, [''])
 test('T13834', normal, compile_fail, [''])
 test('T17077', normal, compile_fail, [''])
 test('T17213', [extra_files(['T17213a.hs'])], multimod_compile_fail, ['T17213', '-v0'])
+test('T17355', normal, compile_fail, [''])
-- 
cgit v1.2.1