Commit a0d8e92e authored by Ryan Scott's avatar Ryan Scott Committed by Marge Bot

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.
parent 64bf7f51
Pipeline #17260 failed with stages
in 207 minutes and 49 seconds
......@@ -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)
......
{-# LANGUAGE FlexibleContexts #-}
module T17955 where
import Data.Coerce
newtype T = Coercible () T => T ()
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’
......@@ -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, [''])
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