Opened 4 months ago

Closed 2 months ago

#15499 closed bug (fixed)

Panic in occurence analysis phase (?), getRuntimeRep

Reported by: _deepfire Owned by: RyanGlScott
Priority: high Milestone: 8.6.2
Component: Compiler Version: 8.4.3
Keywords: Cc: goldfire, simonpj, bgamari
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Compile-time crash or panic Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D5060
Wiki Page:

Description

Compiling this:

{-# LANGUAGE DataKinds, GADTs, KindSignatures #-}
module Holo ()
where

data ADT (p :: Integer) where
  ADT ::
    { a :: a
    , b :: Integer
    } -> ADT p

foo = undefined {b=undefined}

..yields:

ghc: panic! (the 'impossible' happened)
  (GHC version 8.4.3 for x86_64-unknown-linux):
        getRuntimeRep
  p_a29z :: Integer
  Call stack:
      CallStack (from HasCallStack):
        callStackDoc, called at compiler/utils/Outputable.hs:1150:37 in ghc:Outputable
        pprPanic, called at compiler/types/Type.hs:1967:18 in ghc:Type

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

-v4 suggests this happend during the occurence analysis phase (log attached).

Attachments (1)

ghc-843-panic.log (45.0 KB) - added by _deepfire 4 months ago.
ghc -c Holo.hs -v4

Download all attachments as: .zip

Change History (14)

Changed 4 months ago by _deepfire

Attachment: ghc-843-panic.log added

ghc -c Holo.hs -v4

comment:1 Changed 4 months ago by _deepfire

Potentially related are #14786 and #14175, due to also being getRuntimeRep panics (#14175 only had it in this form on 8.3HEAD).

Last edited 4 months ago by _deepfire (previous) (diff)

comment:2 Changed 4 months ago by RyanGlScott

Operating System: LinuxUnknown/Multiple
Owner: set to RyanGlScott

Ugh, this is my fault. I introduced this regression in ef26182e2014b0a2a029ae466a4b121bf235e4e4 (Track the order of user-written tyvars in DataCon).

The -v4 logs are a bit misleading, as the real problem lies in desugaring, not occurrence analysis. Essentially, GHC tries to desugar this:

foo :: forall p. ADT p
foo = undefined {b=undefined}

Into this:

foo :: forall p. ADT p
foo @p = case undefined of
           ADT @a x y -> ADT @p @a x undefined

If the fact that I wrote ADT @p @a looks strange to you, that's because it is! The real type of the ADT constructor is:

ADT :: forall a p. a -> Integer -> ADT p

Due to the fact that type variables in GADT type signatures are now quantified in toposorted order after the aforementioned commit. This means that the correct order of arguments should be ADT @a @p. However, the code in dsExpr assumes that the universally quantified type variables always come before the existentially quantified type variables, so @p ends up being applied before @a, leading to utter disaster and chaos down the line.

Thankfully, this isn't too difficult to fix. Patch incoming.

comment:3 Changed 4 months ago by RyanGlScott

Differential Rev(s): Phab:D5060
Status: newpatch

comment:4 Changed 4 months ago by _deepfire

RyanGlScott, bgamari, do you think we can get this into 8.4.4?

comment:5 Changed 4 months ago by _deepfire

Cc: bgamari added

comment:6 Changed 4 months ago by RyanGlScott

By the way, it's worth noting that working around this issue is very simple: just quantify the type variables in universals-then-existentials order. That is, do this:

data ADT (p :: Integer) where
  ADT :: forall p a.
    { a :: a
    , b :: Integer
    } -> ADT p

comment:7 Changed 4 months ago by _deepfire

RyanGlScott, thank you for the workaround!

comment:8 Changed 4 months ago by Krzysztof Gogolewski <krz.gogolewski@…>

In 63b6a1d4/ghc:

Be mindful of GADT tyvar order when desugaring record updates

Summary:
After commit ef26182e2014b0a2a029ae466a4b121bf235e4e4,
the type variable binders in GADT constructor type signatures
are now quantified in toposorted order, instead of always having
all the universals before all the existentials. Unfortunately, that
commit forgot to update some code (which was assuming the latter
scenario) in `DsExpr` which desugars record updates. This wound
up being the cause of #15499.

This patch makes up for lost time by desugaring record updates in
a way such that the desugared expression applies type arguments to
the right-hand side constructor in the correct order—that is, the
order in which they were quantified by the user.

Test Plan: make test TEST=T15499

Reviewers: simonpj, bgamari

Reviewed By: simonpj

Subscribers: rwbarton, carter

GHC Trac Issues: #15499

Differential Revision: https://phabricator.haskell.org/D5060

comment:9 Changed 4 months ago by monoidal

Status: patchmerge

comment:10 Changed 4 months ago by bgamari

Resolution: fixed
Status: mergeclosed

deepfire, it's rather unlikely that there will be a 8.4.4. However, I've merged the fix to 8.6.1 and the ghc-8.4 branch just in case.

comment:11 Changed 4 months ago by bgamari

Status: closedmerge

comment:12 Changed 2 months ago by RyanGlScott

Milestone: 8.4.48.6.2

Moving to the 8.6.2 milestone, since these tickets were all recently marked as merge.

comment:13 Changed 2 months ago by bgamari

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