Ignoring the cast (which I don't think is relevant) that is:
case (\_ _ -> ...) of {}
I've reduced the bug so the code only depends on Prelude, requiring no external libraries. I have not been able to remove the requirement for two modules. The partial implementation of ReaderT is taken from the transformers library then specialised to IO.
That's quite different since (after inlining a few bits):
$wlvl$wlvl = \ _ -> error "here"m = $wlvl void#
So m evaluates to _|_ in GHC 7.8 which means a case with no alternatives is correct. However, in 7.10 the scrutinee evaluates to a lambda, not _|_, so no alternatives is wrong.
Ok, I think here is what happens, at least in my smaller test case:
Call Arity determines that error1Arg is always called with two arguments. Hence CallArity=2. This is correct.
The simplifier tries to eta-expand error1Arg to take two arguments. This fails in mkEtaWW in CoreArity, where we have this comment:
-- This *can* legitmately happen: -- e.g. coerce Int (\x. x) Essentially the programmer is -- playing fast and loose with types (Happy does this a lot). -- So we simply decline to eta-expand. Otherwise we'd end up -- with an explicit lambda having a non-function type
But still, the Arity field is updated.
Since it has Arity=2, exprIsHNF (error1Arg @ _ ()) is true, and the simplifier does strange things.
So maybe the solution is to not update Arity with the result from Call Arity, but instead let eta-expansion happen and update Arity with the manifest arity after eta expansion. I’ll try that.
Ok, I might have found a suitable fix. You can have a look at D747, although it’s still pending validation (and the new lint check might yet uncover other problems).
Or not quite. I think I did find a bug, but not the only one: With the change in D747, my small example and your example, when merged into one module, no longer fail. But your original test case still fails with Call Arity, and compiles fine without.
There still seems to be some fishiness with Call Arity and undefined. /me continues brooding over the Core.
for acbfc19a (recent ghc-7.10 branch snapshot) vs. 029a296a (one commit before the big typeable-change that caused shake-test oracle test to start failing)
And well, the Core output looks just the same, so I'm not sure if the Buggy.hs/Errors.hs test-case highlights the same issue causing shake-test to fail.
I simplified your code slightly, removing everything about type classes:
module Buggy(buggy) whereimport Errorsnewtype ReaderT r a = ReaderT { runReaderT :: r -> IO a }p = liftReaderT (return ())m >>> k = ReaderT $ \r -> do runReaderT m r; runReaderT k rliftReaderT :: IO a -> ReaderT r aliftReaderT m = ReaderT (const m){-# NOINLINE buggy #-}buggy :: (() -> Bool) -> () -> Bool -> IO ()buggy fun unit bool = runReaderT ( (if bool then liftReaderT $ print () else p) >>> (if fun unit then error2Args unit unit >>> p else p)) ()
still exhibits the problem. Removing the newtype, or turning it into a data makes the problem go away.
I think I found it. The problem is a bad interaction between Call Arity and simplification based on the strictness result.
Consider this code:
e x = error "foo" x... case e () () of {} ...
The strictness signature for e will be <L,U>b. Note the b: This means: „If you give me one argument, then I’m going to be ⊥, which is true.
The Call Arity for e is 2, which is true: e is always called with two arguments.
So the simplifier phase following Call Arity will replace with
e x y = error "foo" x y
At the same time, the simplifier will replace e () () with e (). After all, both are known to be bottom. The result (after inlining) is the bogus
... case (\y -> error "foo" x y) of {} ...
that we see.
The simplifier looks at the b flag of the strictness analyzer in mkArgInfo, replacing isBotRes result_info with False makes the problem go away, but that is probably not the solution.
Zapping strictness analysis when Call Arity finds something is probably also a bad idea.
Maybe the best solution is to limit the number found by call arity to the number of arguments in the strictness signature, if the strictness signature ends in a b.
To assess what does happen in this error case, I augmented the code to be:
buggy fun unit bool = runReaderT (do if bool then liftReaderT $ print () else pure () if fun unit then do error2Args unit unit; liftReaderT $ print "here2" else pure () ) () :: IO ()
Running do print "here1"; buggy (const True) () True; print "here3" gives here1; here3. GHC has used the presence of error to remove the here2, but since the code just falls out of the end of the function it returns back and still prints here3. That's a nasty result, and pretty much describes what happens in the full test case (I'm in the middle of compiling, and then suddenly I stop and do the thing that comes after compiling).