Opened 2 years ago

Closed 2 years ago

#13819 closed bug (fixed)

TypeApplications-related GHC panic

Reported by: Iceland_jack Owned by: goldfire
Priority: high Milestone: 8.4.1
Component: Compiler (Type checker) Version: 8.2.1-rc2
Keywords: TypeApplications Cc: goldfire, bgamari
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Compile-time crash or panic Test Case: typecheck/should_fail/T13819
Blocked By: Blocking:
Related Tickets: #13846, #13850 Differential Rev(s): Phab:D3754
Wiki Page:

Description

{-# LANGUAGE DeriveFunctor, TypeApplications #-}
-- {-# Language ViewPatterns    #-}
-- {-# Language InstanceSigs, ViewPatterns, TupleSections, GeneralizedNewtypeDeriving, TemplateHaskell, LambdaCase #-}

module D where

import Data.Coerce
import Control.Applicative

newtype A a = A (IO a)
  deriving Functor

instance Applicative A where
  pure = pure @(_ -> WrappedMonad A _) @(_ -> A _) pure
  
instance Monad A where

causes a panic

$ ghci -ignore-dot-ghci /tmp/a.hs
GHCi, version 8.3.20170605: http://www.haskell.org/ghc/  :? for help
[1 of 1] Compiling D                ( /tmp/a.hs, interpreted )
ghc: panic! (the 'impossible' happened)
  (GHC version 8.3.20170605 for x86_64-unknown-linux):
	repSplitAppTys
  w0_a1xc[tau:4]
  WrappedMonad A w0_a1xe[tau:4]
  []
  Call stack:
      CallStack (from HasCallStack):
        prettyCurrentCallStack, called at compiler/utils/Outputable.hs:1133:58 in ghc:Outputable
        callStackDoc, called at compiler/utils/Outputable.hs:1137:37 in ghc:Outputable
        pprPanic, called at compiler/types/Type.hs:809:9 in ghc:Type

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug

> 

Change History (17)

comment:1 Changed 2 years ago by Iceland_jack

Version: 8.0.18.3

comment:2 Changed 2 years ago by RyanGlScott

Component: CompilerCompiler (Type checker)
Keywords: TypeApplications added
Milestone: 8.2.1
Priority: normalhigh
Summary: GHC PanicsTypeApplications-related GHC panic
Type of failure: None/UnknownCompile-time crash or panic
Version: 8.38.2.1-rc2

Yikes, this is a regression from GHC 8.0.2.

comment:3 Changed 2 years ago by simonpj

Cc: goldfire added

Richard, this is yet more fallout from the terrible uo_thing mess.

The panic comes from the argument-counting in repSplitAppTys, when called from mkTypErrorThing or mkTypeErrorThingArgs. At the moment we construct this ErrorThing the kind of the type has not been zonked, so we have a FunTy t1 t2 with t1 :: kappa.

I guess this will be fixed when we tidy up uo_thing... but that is looking increasingly urgent.

comment:4 Changed 2 years ago by RyanGlScott

Cc: bgamari added

FWIW, commit b207b536ded40156f9adb168565ca78e1eef2c74 (Generalize kind of the (->) tycon) is what introduced this regression.

comment:5 Changed 2 years ago by simonpj

See also #13846

comment:6 Changed 2 years ago by RyanGlScott

Two more ways to trigger this panic:

$ ghci -XTypeApplications -ignore-dot-ghci
GHCi, version 8.3.20170605: http://www.haskell.org/ghc/  :? for help
Prelude> :t fmap @(_ -> _)
ghc: panic! (the 'impossible' happened)
  (GHC version 8.3.20170605 for x86_64-unknown-linux):
	repSplitAppTys
  w0_a1pF[tau:2]
  w0_a1pH[tau:2]
  []
  Call stack:
      CallStack (from HasCallStack):
        prettyCurrentCallStack, called at compiler/utils/Outputable.hs:1133:58 in ghc:Outputable
        callStackDoc, called at compiler/utils/Outputable.hs:1137:37 in ghc:Outputable
        pprPanic, called at compiler/types/Type.hs:809:9 in ghc:Type

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug
> cat wibble.hs 
ecase :: Either a b -> (a -> c) (b -> c) -> c
ecase (Left a) f _ = f a
ecase (Right b) _ g = g b

> ghci wibble.hs 
GHCi, version 8.2.0.20170507: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/erikd/.ghci
[1 of 1] Compiling Main             ( wibble.hs, interpreted )
ghc: panic! (the 'impossible' happened)
 (GHC version 8.2.0.20170507 for x86_64-unknown-linux): repSplitAppTys
 a_a1pA[sk:1]
 c_a1pC[sk:1]
 []
 Call stack:
  CallStack (from HasCallStack):
   prettyCurrentCallStack, called at compiler/utils/Outputable.hs:1134:58 in 
         ghc:Outputable
   callStackDoc, called at compiler/utils/Outputable.hs:1138:37 in 
         ghc:Outputable
   pprPanic, called at compiler/types/Type.hs:808:9 in ghc:Type

Both of these were also caused by b207b536ded40156f9adb168565ca78e1eef2c74.

comment:7 Changed 2 years ago by goldfire

My start toward fixing this is at https://github.com/goldfirere/ghc/tree/uo-thing but I'm about to go on holiday. I may have time to finish while traveling, but no guarantees.

If someone wants to carry this over the line, I'd be grateful. Otherwise, back in full action on July 5.

comment:8 Changed 2 years ago by goldfire

Just for the record, I'm back on this. Will have a fix soon.

comment:9 Changed 2 years ago by bgamari

Any update on this, goldfire?

comment:10 Changed 2 years ago by bgamari

Owner: set to goldfire

comment:11 Changed 2 years ago by goldfire

Differential Rev(s): Phab:D3754

As it turns out, yes! All fixed on my end. Right now, I have a raft of bugfixes that just need to get validated and such. This is one of them.

comment:12 Changed 2 years ago by bgamari

Status: newpatch

comment:13 Changed 2 years ago by bgamari

Milestone: 8.2.18.4.1

I'm afraid we're going to have to punt on this for 8.2.1. The patch is simply too large to merge this late in the game (especially since backports tend to be extra painful with the trees-that-grow change behind us). Perhaps we can make this happen for 8.2.2 if someone yells, but otherwise I would be tempted to simply wait for 8.4.

comment:14 Changed 2 years ago by Richard Eisenberg <rae@…>

In c2417b8/ghc:

Fix #13819 by refactoring TypeEqOrigin.uo_thing

The uo_thing field of TypeEqOrigin is used to track the
"thing" (either term or type) that has the type (kind) stored
in the TypeEqOrigin fields. Previously, this was sometimes a
proper Core Type, which needed zonking and tidying. Now, it
is only HsSyn: much simpler, and the error messages now use
the user-written syntax.

But this aspect of uo_thing didn't cause #13819; it was the
sibling field uo_arity that did. uo_arity stored the number
of arguments of uo_thing, useful when reporting something
like "should have written 2 fewer arguments". We wouldn't want
to say that if the thing didn't have two arguments. However,
in practice, GHC was getting this wrong, and this message
didn't seem all that helpful. Furthermore, the calculation
of the number of arguments is what caused #13819 to fall over.
This patch just removes uo_arity. In my opinion, the change
to error messages is a nudge in the right direction.

Test case: typecheck/should_fail/T13819

comment:15 Changed 2 years ago by goldfire

Status: patchmerge
Test Case: typecheck/should_fail/T13819

This patch isn't tiny, but it seems worthwhile to merge.

comment:16 Changed 2 years ago by RyanGlScott

Ben, I take it that commit c2417b87ff59c92fbfa8eceeff2a0d6152b11a47 (Fix #13819 by refactoring TypeEqOrigin.uo_thing) isn't going to be merged into 8.2.2? If so, we ought to close this.

It's interesting to note that even though that commit hasn't been merged to 8.2.2, the program in this ticket not longer panics on GHC 8.2.2! It turns out that commit cbf472384b5b583c24d1a1a32f3fa58d4f1501b1 (Small refactor of getRuntimeRep) separately fixed this panic, and that commit was backported to 8.2.2. So that's nice.

comment:17 Changed 2 years ago by bgamari

Resolution: fixed
Status: mergeclosed

Indeed I think it's out of the running for 8.2.2 and, given its size, perhaps out of scope for 8.2 on the whole.

Note: See TracTickets for help on using tickets.