[Git][ghc/ghc][master] Run checkNewDataCon before constraint-solving newtype constructors

[Marge Bot]

 Marge Bot pushed to branch master at Glasgow Haskell Compiler / GHC


Commits:
a0d8e92e by Ryan Scott at 2020-03-29T17:33:20-04:00
Run checkNewDataCon before constraint-solving newtype constructors

Within `checkValidDataCon`, we used to run `checkValidType` on the
argument types of a newtype constructor before running
`checkNewDataCon`, which ensures that the user does not attempt
non-sensical things such as newtypes with multiple arguments or
constraints. This works out in most situations, but this falls over
on a corner case revealed in #17955:

```hs
newtype T = Coercible () T => T ()
```

`checkValidType`, among other things, peforms an ambiguity check on
the context of a data constructor, and that it turn invokes the
constraint solver. It turns out that there is a special case in the
constraint solver for representational equalities (read: `Coercible`
constraints) that causes newtypes to be unwrapped (see
`Note [Unwrap newtypes first]` in `TcCanonical`). This special case
does not know how to cope with an ill formed newtype like `T`, so
it ends up panicking.

The solution is surprisingly simple: just invoke `checkNewDataCon`
before `checkValidType` to ensure that the illicit newtype
constructor context is detected before the constraint solver can
run amok with it.

Fixes #17955.

- - - - -


4 changed files:

- compiler/typecheck/TcTyClsDecls.hs
- + testsuite/tests/typecheck/should_fail/T17955.hs
- + testsuite/tests/typecheck/should_fail/T17955.stderr
- testsuite/tests/typecheck/should_fail/all.T


Changes:

=====================================
compiler/typecheck/TcTyClsDecls.hs
=====================================
@@ -3992,9 +3992,6 @@ checkValidDataCon dflags existential_ok tc con
           -- Reason: it's really the argument of an equality constraint
         ; checkValidMonoType orig_res_ty
 
-          -- Check all argument types for validity
-        ; checkValidType ctxt (dataConUserType con)
-
           -- If we are dealing with a newtype, we allow levity polymorphism
           -- regardless of whether or not UnliftedNewtypes is enabled. A
           -- later check in checkNewDataCon handles this, producing a
@@ -4002,9 +3999,16 @@ checkValidDataCon dflags existential_ok tc con
         ; unless (isNewTyCon tc)
             (mapM_ (checkForLevPoly empty) (dataConOrigArgTys con))
 
-          -- Extra checks for newtype data constructors
+          -- Extra checks for newtype data constructors. Importantly, these
+          -- checks /must/ come before the call to checkValidType below. This
+          -- is because checkValidType invokes the constraint solver, and
+          -- invoking the solver on an ill formed newtype constructor can
+          -- confuse GHC to the point of panicking. See #17955 for an example.
         ; when (isNewTyCon tc) (checkNewDataCon con)
 
+          -- Check all argument types for validity
+        ; checkValidType ctxt (dataConUserType con)
+
           -- Check that existentials are allowed if they are used
         ; checkTc (existential_ok || isVanillaDataCon con)
                   (badExistential con)


=====================================
testsuite/tests/typecheck/should_fail/T17955.hs
=====================================
@@ -0,0 +1,6 @@
+{-# LANGUAGE FlexibleContexts #-}
+module T17955 where
+
+import Data.Coerce
+
+newtype T = Coercible () T => T ()


=====================================
testsuite/tests/typecheck/should_fail/T17955.stderr
=====================================
@@ -0,0 +1,6 @@
+
+T17955.hs:6:13: error:
+    • A newtype constructor cannot have a context in its type
+      T :: Coercible () T => () -> T
+    • In the definition of data constructor ‘T’
+      In the newtype declaration for ‘T’


=====================================
testsuite/tests/typecheck/should_fail/all.T
=====================================
@@ -561,3 +561,4 @@ test('T17566c', normal, compile_fail, [''])
 test('T17773', normal, compile_fail, [''])
 test('T17021', normal, compile_fail, [''])
 test('T17021b', normal, compile_fail, [''])
+test('T17955', normal, compile_fail, [''])



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/a0d8e92e9c9b67426aa139d6bc46363d8940f992

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/a0d8e92e9c9b67426aa139d6bc46363d8940f992
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20200329/a51cead8/attachment-0001.html>