Opened 4 years ago

Closed 4 years ago

#11520 closed bug (fixed)

GHC falls into a hole if given incorrect kind signature

Reported by: bgamari Owned by:
Priority: highest Milestone: 8.0.1
Component: Compiler (Type checker) Version: 8.0.1-rc1
Keywords: TypeInType Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect warning at compile-time Test Case: polykinds/T11520
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description (last modified by bgamari)

If one provides an incorrect kind signature GHC throws up. For instance, this non-sense,

{-# LANGUAGE RankNTypes, PolyKinds, TypeInType, GADTs, UndecidableSuperClasses #-}

module Play where

import GHC.Types hiding (TyCon)

data TypeRep (a :: k)

class Typeable k => Typeable (a :: k) where
    typeRep :: TypeRep a

data Compose (f :: k1 -> *) (g :: k2 -> k1) (a :: k2) = Compose (f (g a))

-- Note how the kind signature on g is incorrect
instance (Typeable f, Typeable (g :: k), Typeable k) => Typeable (Compose f g) where
    typeRep = undefined

fails with

λ> :load Bug.hs
[1 of 1] Compiling Play             ( Bug.hs, interpreted )
ghc: panic! (the 'impossible' happened)
  (GHC version 8.1.20160122 for x86_64-unknown-linux):
	fvProv falls into a hole {abet}

Please report this as a GHC bug:

Change History (10)

comment:1 Changed 4 years ago by bgamari

Description: modified (diff)
Type of failure: None/UnknownIncorrect warning at compile-time

comment:2 Changed 4 years ago by bgamari

Description: modified (diff)
Summary: GHC falls into a holeGHC falls into a hole if given incorrect kind signature

comment:3 Changed 4 years ago by bgamari

Description: modified (diff)

comment:4 Changed 4 years ago by simonpj

The trouble here is this:

  • The kind error is indeed found and reported (i.e. collected in the bag of errors in the monad)
  • But we continue anyway into checkValidInstance
  • The latter calls fvProv to check for something like the size of instance
  • Which fails because the unsolved constraint has left a hole in the type

Really we should not even attempt checkValidInstance if we fail to kind-check the type.

We could use a checkNoErrs in TcHsTye.TcHsClsInstType. But there are many other uses of solveEqualities all of which in principle have the same concern. So I thought about adding checkNoErrs to solveEqualities. But there I see

-- | Type-check a thing that emits only equality constraints, then
-- solve those constraints. Emits errors -- but does not fail --
-- if there is trouble.
solveEqualities :: TcM a -> TcM a

Richard: why did you want the "...but does not fail..." bit? What goes wrong if it fais on error? In effect it always used to!

comment:5 Changed 4 years ago by simonpj

Keywords: TypeInType added
Priority: normalhighest

Highest prio because I think the fix is easy and an outright crash is bad.

comment:6 Changed 4 years ago by goldfire

The but does not fail bit is just to accurately describe the function. As usual, failing is fine, but it means you get fewer error messages at a go. In other words, we can just change the behavior.

But I much favor just fixing fvProv to do something other than panic on a coercion hole, like return an empty list. There are a bunch of these panics on coercion holes where I thought coercion holes should never be seen. But I've learned that erroneous code can make GHC take paths I hadn't considered. So I'll remove all of these panics and replace them with something that won't crash GHC.

comment:7 Changed 4 years ago by simonpj

I'd like to try wrapping solveEqualities in checkNoErrs. Yes, we fail more, but let's see if that worsens behaviour.

My reasoning: if we take a type signature that has kind errors in it, and proceed onwards with a kind-incorrect type, we are likely to get hard-to-explain knock-on errors down the line. Let's fail faster.

comment:8 Changed 4 years ago by Simon Peyton Jones <simonpj@…>

In b565830d/ghc:

Wrap solveEqualities in checkNoErrs

This simple change fixes Trac #11563, #11520, #11516, #11399.
See esp the comments in #11520.

See Note [Fail fast on kind errors] in TcSimplify

Merge to 8.0 branch

comment:9 Changed 4 years ago by simonpj

Status: newmerge
Test Case: polykinds/T11520

Ha! That worked.

One fix closes four tickets. Merge to 8.0 branch.

comment:10 Changed 4 years ago by bgamari

Resolution: fixed
Status: mergeclosed
Note: See TracTickets for help on using tickets.