Opened 3 years ago

Closed 3 years ago

#5587 closed bug (fixed)

Code using seq has wrong strictness (too lazy) when optimised

Reported by: michal.palka Owned by: simonpj
Priority: high Milestone: 7.4.1
Component: Compiler Version: 7.3
Keywords: seq strictness strict lazy Cc: michal.palka@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect result at runtime Test Case: simplCore/should_run/T5587
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

Following program prints [1] instead of crashing when compiled with -O -fno-full-laziness. Note that it's neccessary to use the extra definition hiddenError instead of calling error directly. The problem might be related to #5557, which, however, has already been fixed and optimisation is needed to trigger the current bug.

hiddenError = error "hidden error"

main = do
  print $ seq (head (map (\a -> \b -> hiddenError) (hiddenError::[] Bool))) id [1]

I used GHC 7.3.20111022 for triggering this problem.

Change History (5)

comment:1 Changed 3 years ago by igloo

  • Milestone set to 7.4.1
  • Owner set to simonpj
  • Priority changed from normal to high

Thanks for the report. I can reproduce this with 7.3.20111109.

comment:2 Changed 3 years ago by simonpj

Thanks for these interesting test cases, Michal.

GHC is deliberately slightly sloppy about bottoms. I have used your examples as a provocation to narrow the sloppiness down, but this particular ticket hits precisly the nub. Consider

f :: [a] -> b -> b
f [] = error "urk"
f (x:xs) = \v -> v

Now clearly (f bottom) should be bottom, but that means giving f arity 1. And that is BAD for monadic functions, where the monad conceals a function. It turns out that this ticket is an example. Here's the code:

f a b =  a

he :: [Bool]
he = hiddenError

main = print $ seq (head (map f he)) id [1]

Here is how the (head (map f he)) optimises:

head (map f he)

= { inline map }
  head (build (\cn. foldr (mapFB c f) n he))

= { head/build RULE }
  (\cn. foldr (mapFB c f) n he) (\x _ -> x) badHead

= { beta reduce }
  foldr (mapFB (\x _ -> x) f) badHead he

= { inline foldr }
  let go [] = badHead
      go (y:ys) = mapFB (\x _ -> x) f y (go ys)
  in go he

= { inline mapFB }
  let go []     = badHead
      go (y:ys) = (\x _ -> x) (f y) (go ys)
                = f y
                = \b. y
   in go he

Now look at that 'go' function. It's just like the example I started with.

So I have done this:

  • Added a new flag -fpedantic-bottoms that swithes off the dubious optimisation.

With this flag on, the example behaves as it should (try it).

Now there's a chance of compiling everything with -fpedantic-bottoms and seeing the effect on performance. Maybe my claim that it's important in practice isn't true.

The -fno-state-hack flag is similar, incidentally.

Simon

comment:3 Changed 3 years ago by simonpj

  • Test Case set to simplCore/should_run/T5587

comment:4 Changed 3 years ago by simonpj@…

commit 1790dbe4a5829af5bcdc5bc81eafb67b154008cc

Author: Simon Peyton Jones <[email protected]>
Date:   Wed Nov 16 14:03:30 2011 +0000

    Add -fpedantic-bottoms, and document it
    
    I did a bit of refactoring (of course) at the same time.
    See the discussion in Trac #5587.  Most of the real change
    is in CoreArity.

 compiler/coreSyn/CoreArity.lhs    |  111 ++++++++++++++++++++++++++-----------
 compiler/main/DynFlags.hs         |    2 +
 compiler/simplCore/SimplUtils.lhs |   52 ++++--------------
 docs/users_guide/flags.xml        |    9 +++
 docs/users_guide/using.xml        |   14 +++++
 5 files changed, 115 insertions(+), 73 deletions(-)

comment:5 Changed 3 years ago by simonpj

  • Resolution set to fixed
  • Status changed from new to closed

I'm going to declare this fixed now. At least the un-fixed part is documented in the user guide, and there's a flag to control it.

Simon

Note: See TracTickets for help on using tickets.