Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#5557 closed bug (fixed)

Code using seq has wrong strictness (too lazy)

Reported by: michal.palka Owned by:
Priority: high Milestone: 7.4.1
Component: Compiler Version: 7.2.1
Keywords: seq strictness strict lazy Cc: michal.palka@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect result at runtime Test Case: ghci/scripts/T5557
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

Following snippet of code gets miscompiled regardless of optimisation level:

([seq (seq (tail ([]::[Int])) (\a -> error "aaa"))] !! 0) [1]

The result of printing the value is [1], whereas the correct result should be throwing an error:

testModule: Prelude.tail: empty list

The same problem occurs if I try to run the code in GHCI.

Looking at the Core at -O0 suggests that the expression with two seqs gets transformed into the identity function, despite that tail [] could (and will) crash. Since the problem is visible in Core, I mark this bug as independent of the architecture.

I tried it in GHC versions 7.2.1 and 7.3.20111013, which gave same wrong results.

Change History (5)

comment:1 Changed 6 years ago by michal.palka

Cc: michal.palka@… added

comment:2 Changed 6 years ago by igloo

Milestone: 7.4.1
Priority: normalhigh

Hmm, very interesting:

Prelude> ([seq (seq undefined (\a -> error "a"))] !! 0) [1]
[1]
Prelude> ([seq (seq undefined (\a -> undefined))] !! 0) [1]
*** Exception: Prelude.undefined

comment:3 Changed 6 years ago by simonpj@…

commit 6d5dfbf750320dd7bd0fea8e2965935fcedbe15e

Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Fri Oct 21 16:34:21 2011 +0100

    Be even more careful about eta expansion when bottom is involved
    
    See Note [Dealing with bottom], reproduced below.  Fixes Trac #5557.
    
    3.  Note [Dealing with bottom]
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    Consider
    	f = \x -> error "foo"
    Here, arity 1 is fine.  But if it is
    	f = \x -> case x of
    			True  -> error "foo"
    			False -> \y -> x+y
    then we want to get arity 2.  Technically, this isn't quite right, because
    	(f True) `seq` 1
    should diverge, but it'll converge if we eta-expand f.  Nevertheless, we
    do so; it improves some programs significantly, and increasing convergence
    isn't a bad thing.  Hence the ABot/ATop in ArityType.
    
    However, this really isn't always the Right Thing, and we have several
    tickets reporting unexpected bahaviour resulting from this
    transformation.  So we try to limit it as much as possible:
    
     * Do NOT move a lambda outside a known-bottom case expression
          case undefined of { (a,b) -> \y -> e }
       This showed up in Trac #5557
    
     * Do NOT move a lambda outside a case if all the branches of
       the case are known to return bottom.
          case x of { (a,b) -> \y -> error "urk" }
       This case is less important, but the idea is that if the fn is
       going to diverge eventually anyway then getting the best arity
       isn't an issue, so we might as well play safe
    
    Of course both these are readily defeated by disguising the bottoms.

 compiler/coreSyn/CoreArity.lhs |   34 +++++++++++++++++++++++++++++++---
 1 files changed, 31 insertions(+), 3 deletions(-)

comment:4 Changed 6 years ago by simonpj

Resolution: fixed
Status: newclosed
Test Case: ghci/scripts/T5557

comment:5 Changed 6 years ago by michal.palka

I tried retesting the same property with GHC 7.3.20111022 and no bug was triggered by this or similar terms, so it seems to be fixed.

Note: See TracTickets for help on using tickets.