Levity polymorphism panic

I imagine I have made a mistake, but GHC shouldn't panic!

{-# language TypeInType, ScopedTypeVariables, MagicHash #-}

import GHC.Exts (TYPE, Proxy#, proxy#)

fold :: forall rep a (r :: TYPE rep).
          (r -> a -> Proxy# r -> r) -> (Proxy# r -> r) -> [a] -> r
fold f k [] = k proxy#
fold f k (x : xs) = fold f (f (k proxy#) x) xs

This gives me

ghc-stage2: panic! (the 'impossible' happened)
  (GHC version 8.5.20171211 for x86_64-unknown-linux):
        splitFunTy
  ()
  Call stack:
      CallStack (from HasCallStack):
        callStackDoc, called at compiler/utils/Outputable.hs:1150:37 in ghc:Outputable
        pprPanic, called at compiler/types/Type.hs:921:30 in ghc:Type

Trac metadata

Trac field	Value
Version	8.5
Type	Bug
TypeOfFailure	OtherFailure
Priority	normal
Resolution	Unresolved
Component	Compiler
Test case
Differential revisions
BlockedBy
Related
Blocking
CC
Operating system
Architecture

changed milestone to %8.6.1

changed weight to 5

added Tbug Trac import labels

Richard, this is a continuation of #12709 (closed). During desugaring we have

f a b

Turns out that there's a levity-polymophism error in (f a), so (bizarrely) dsWhneNoErrs returns (). Then the desugarer tries to apply that to b, and that fails because () isn't a function type.

Hhe cure (for #12709 (closed)) was ugly enough; now we find that it doesn't acutally work! Replacing an expression by () doesn't make sense -- the types are wrong. I can see two solutions (there may be others):

• Instead of returning () return undefined @ty where ty is the needed type.

But the whole dsWhenNoErrs machinery is very heavy; it calls tryM (which in turn allcoates fresh IO refes for errors etc) on every single application node. The cure is worse than the disease. So instead

• We could define desugarer-only version of mkCoreApp, which doesn't blow up

  on levity-polymorphic args. (Maybe it builds an ordinary App, say.) It's

  a bit of code to duplicate but not much, and it's easy to understand.

(Alternatively we could make isUnliftedType return False for a levity-poly type. That would be even simpler.)

What do you think?

A few thoughts:

• I really don't like having isUnliftedType return False for levity polymorphism. Panicking is the right behavior. Anything else might mask bugs.
• How would the new mkCoreApp work? Would it check its argument for levity polymorphism and then react accordingly? This check is not necessarily cheap, and it would be redundant with other checks for LP.
• We could make the new mkCoreApp monadic and have it do the only LP check. But then it would need to take the undesugared argument, so that error messages can be at all sensible. That would work in several cases, but sometimes it's not so easy.
• You're right that returning () is gross. Perhaps returning undefined is better.
• dsWhenNoErrs uses askNoErrsDs which does indeed use tryM. But there's no great reason askNoErrsDs needs to allocate fresh refs for messages, given that messages are always propagates in askNoErrsDs. Perhaps we could just make that more efficient by not using tryM.

Bottom line: I don't love the current design. But the devil is in the details, and I'm not yet convinced about any new design either. We'd have to try it and see. (I am convinced enough that undefined is better than what we have now.)

More thoughts

Perhaps we could just make that more efficient by not using tryM.

No: we need to know if there are any error messages from the enclosed thing.

I really don't like having isUnliftedType return False for levity polymorphism.

Actually I've decided I do! The compiler should panic if there is no other alternative. Here there is -- and Lint will catch the error shortly afterwards in a much more civilised way. It's such a simple fix.

How would the new mkCoreApp work?

Just like the current one, but without panicing in isUnliftedType, and with the entire apparatus introduced by #12709 (closed). That is, build a term that may not pass Lint; but it doesn't matter because we are going to report LP errors anyway and stop.

So a "new mkCoreApp" duplicates code, but in exchange means that all other uses of isUnliftedType can panic if you still feel strongly that they should.

So are you suggesting that this new mkCoreApp doesn't use isUnliftedType, but instead re-creates it? Who reports the LP error? I suppose that could remain the same as it is right now... but then the check in mkCoreApp will be redundant.

To be clear, I am OK with having a special mkCoreApp that doesn't panic on an LP argument used in the desugarer, as long as isUnliftedType still panics. My worry here is more about performance, but perhaps it would be an improvement over the status quo.

changed milestone to %8.8.1

These won't be addressed in GHC 8.6.

changed milestone to %8.10.1

Bumping milestones of low-priority tickets.

Bumping milestones of low-priority tickets.

added compiler crash label

added Pnormal label

removed milestone

mentioned in issue #19615 (closed)

mentioned in merge request !6081 (merged)

closed with merge request !6081 (merged)

mentioned in commit 901f0e1b