Opened 10 years ago

Closed 20 months ago

Last modified 20 months ago

#457 closed bug (fixed)

Strictness problem

Reported by: nilsanders Owned by: jstolarek
Priority: normal Milestone:
Component: Compiler Version: 6.4.1
Keywords: Cc: michal.terepeta@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect result at runtime Test Case: simplCore/should_run/T457.hs
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description (last modified by simonmar)

As requested, this is a resubmission (and update) of a
previously reported bug, to get it into the bug tracker.

The following program should output something involving
Correct:

> module Main where

> f x = case x of
>   x@True  -> \y -> x && y
>   x@False -> \y -> x && y

> main = putStrLn $ f (error "Correct") `seq` "Error"

However, whether it does so is a non-trivial function
of the GHC version and optimisation settings:

GHC version     -O2?  Correct?
------------------------------
4.08.1          No    Yes
4.08.1          Yes   No
5.04.2          No    No
5.04.2          Yes   Yes
6.0.1           _     No
6.2.2           _     No
6.4             _     No
6.4.1.20050820  _     No

All tests were run on a Solaris system.

Different fs give different behaviour, at least for
6.0.1. Try e.g.

> f x = case x of
>   True  -> id
>   False -> id

Change History (18)

comment:1 Changed 9 years ago by simonmar

  • Description modified (diff)
  • severity changed from normal to minor
  • Version changed from None to 6.4.1

comment:2 Changed 9 years ago by simonpj

  • Architecture set to Unknown
  • difficulty set to Unknown
  • Operating System set to Unknown
  • Owner changed from nobody to simonpj
  • Status changed from assigned to new

Related to #317.

Caused by the fact that we eta-expand around the case expression. Not eta-expanding hurts performance in important cases (we think). Still thinking what to do about this; it's technically wrong but seldom seems to bite in practice.

Simon

comment:3 Changed 9 years ago by simonmar

See also #826

comment:4 Changed 8 years ago by igloo

  • Milestone set to 6.8

This is still wrong in 6.6 and the HEAD.

comment:5 Changed 8 years ago by guest

comment:6 Changed 7 years ago by simonmar

  • Milestone changed from 6.8 branch to _|_
  • Owner simonpj deleted

comment:7 Changed 7 years ago by simonmar

  • Architecture changed from Unknown to Unknown/Multiple

comment:8 Changed 7 years ago by simonmar

  • Operating System changed from Unknown to Unknown/Multiple

comment:9 follow-up: Changed 5 years ago by igloo

  • Type of failure set to Incorrect result at runtime

Still happens in 6.12.

comment:10 in reply to: ↑ 9 Changed 5 years ago by michalt

  • Cc michal.terepeta@… added

Replying to igloo:

Still happens in 6.12.

Current HEAD (7.1.20101015):

  • without optimisations - correct
  • with -O or -O2 - error

comment:11 Changed 20 months ago by nomeata

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

In 7.6.3, this seems to be fixed.

comment:12 Changed 20 months ago by jstolarek

Do we have a test case for that? If not we should add one so we can catch any regressions in the future.

comment:13 Changed 20 months ago by nomeata

  • Resolution fixed deleted
  • Status changed from closed to new

Not yet, but now there is one:
https://github.com/nomeata/ghc-testsuite/compare/T457
feel free to merge into HEAD (reopening this ticket for that).

comment:14 Changed 20 months ago by nomeata

  • Status changed from new to patch

comment:15 Changed 20 months ago by jstolarek

  • Owner set to jstolarek

I'll validate and merge.

comment:16 Changed 20 months ago by Jan Stolarek <jan.stolarek@…>

In 5dd852a891bf2956933a2e901f8ff183b78bc1a8/testsuite:

Add test cases for T457

(fixes #457)

comment:17 Changed 20 months ago by jstolarek

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

comment:18 Changed 20 months ago by jstolarek

  • Test Case set to simplCore/should_run/T457.hs
Note: See TracTickets for help on using tickets.