[GHC] #14765: Levity polymorphism panic
#14765: Levity polymorphism panic -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.6.1 Component: Compiler | Version: 8.5 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: Compile-time Unknown/Multiple | crash or panic Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- I imagine I have made a mistake, but GHC shouldn't panic! {{{#!hs {-# 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 }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14765 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14765: Levity polymorphism panic -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.6.1 Component: Compiler | Version: 8.5 Resolution: | Keywords: | LevityPolymorphism Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple crash or panic | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by goldfire): * keywords: => LevityPolymorphism -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14765#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14765: Levity polymorphism panic -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.6.1 Component: Compiler | Version: 8.5 Resolution: | Keywords: | LevityPolymorphism Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple crash or panic | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Richard, this is a continuation of #12709. 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) 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? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14765#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14765: Levity polymorphism panic -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.6.1 Component: Compiler | Version: 8.5 Resolution: | Keywords: | LevityPolymorphism Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple crash or panic | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by goldfire): 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.) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14765#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14765: Levity polymorphism panic -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.6.1 Component: Compiler | Version: 8.5 Resolution: | Keywords: | LevityPolymorphism Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple crash or panic | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): More thoughts
Perhaps we could just make that more efficient by not using tryM.
I really don't like having isUnliftedType return False for levity
No: we need to know if there are any error messages ''from the enclosed thing''. 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. 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. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14765#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14765: Levity polymorphism panic -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.6.1 Component: Compiler | Version: 8.5 Resolution: | Keywords: | LevityPolymorphism Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple crash or panic | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by goldfire): 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. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14765#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC