#15114 closed bug (fixed)

ghc 8.4.1 lifts errors out of if then else

Reported by: elaforge Owned by:
Priority: normal Milestone: 8.6.1
Component: Compiler Version: 8.4.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case: simplCore/should_run/T15114
Blocked By: Blocking:
Related Tickets: #5129 Differential Rev(s):
Wiki Page:

Description (last modified by adamse)

Ok, here's a short module:

import qualified Control.Exception as Exception

main :: IO ()
main = do
    unserialize
    putStrLn "all is well"

unserialize :: IO Char
unserialize =
    if definitelyTrue
        then do
            return 'a'
        else do
            Exception.evaluate (error "wrong place")

{-# NOINLINE definitelyTrue #-}
definitelyTrue :: Bool
definitelyTrue = True

When compiled with -O on 8.4.1, this should print "wrong place". Without -O, or with 8.4.2, or if True can be inlined, or without evaluate, all is well. I can reproduce this on OS X 10.13.4.

This could be related to a known bug with Exception.evaluate in 8.4.1: #13930

Change History (9)

comment:1 Changed 18 months ago by adamse

Description: modified (diff)

comment:2 Changed 18 months ago by diatchki

As another data point, I can reproduce this with 8.4.1 on Linux too with -O. However, it goes away if I compile with -O2.

comment:3 Changed 18 months ago by hvr

The title is misleading IMO: it's not that the else branch is taken instead of the then branch, but rather that the evaluate effect floats/leaks out of the inactive control flow path. I.e. the a branch is still taken; it's just that the error exception is triggered before we can reach it.

Here's a modified example which better shows what I mean:

import qualified Control.Exception as Exception
import Debug.Trace

main :: IO ()
main = do
    c <- unserialize
    putStrLn "all is well"
    print c

unserialize :: IO Char
unserialize =
    if definitelyTrue
        then do
            putStrLn "HEY"
            return 'a'
        else do
            Exception.evaluate (traceShow "wrong place" 'b')

{-# NOINLINE definitelyTrue #-}
definitelyTrue :: Bool
definitelyTrue = True

comment:4 Changed 18 months ago by simonpj

I think the bug also does not manifest in GHC 8.2.2.

It's probably already-fixed, but at least it's an excellent regression test. And I'd like to be sure that it really IS fixed, rather than merely concealed, in 8.4.2.

Does anyone feel able to bisect to the commit that fixes it?

comment:5 Changed 18 months ago by RyanGlScott

I've confirmed that the commit that fixed this was ac1ade1ad25f145ab27219c4fb8366e982658873 (Fix seq# case of exprOkForSpeculation).

comment:6 Changed 18 months ago by elaforge

Summary: ghc 8.4.1 optimizes True to Falseghc 8.4.1 lifts errors out of if then else

Sounds like close as a duplicate then. Shall I submit as a regression test?

comment:7 Changed 18 months ago by simonpj

Thanks. That makes me sure it relates to #5129.

Don't worry, I'll add it as a regression test myself, shortly. Thanks for boiling it down.

comment:8 Changed 18 months ago by Simon Peyton Jones <simonpj@…>

In 6742ce2/ghc:

Test Trac #15114

comment:9 Changed 18 months ago by RyanGlScott

Resolution: fixed
Status: newclosed
Test Case: simplCore/should_run/T15114
Note: See TracTickets for help on using tickets.