Ticket #246 (closed bug: fixed)

Opened 5 years ago

Last modified 3 months ago

Wrong pat-match order for records

Reported by: simonpj Assigned to: simonpj
Priority: low Milestone: _|_
Component: Compiler Version: 6.4.1
Severity: minor Keywords:
Cc: Difficulty: Unknown
Test Case: deSugar/should_run/T246 Operating System: Unknown/Multiple
Architecture: Unknown/Multiple

Description (Last modified by simonpj)

In section 3.17.2 case 6 of the haskell report

There is some confusing language in the report. Furthermore there is either a bug in ghc, or hugs, depending on which way you interpret it.

it says: Matching against a constructor using labeled fields is the same as matching ordinary constructor patterns except that the fields are matched in the order they are named in the field list. All fields listed must be declared by the constructor; fields may not be named more than once. Fields not named by the pattern are ignored (matched against _).

If you interpret 'field list' to mean the order the fields appear in the pattern then given the code below "bar" should be printed, as the 'b' field is compared and fails so the a field is never matched against.

If you interpret 'field list' to mean the order the fields were DECLARED in, then this should equal _|_ as the 'a' field is matched first and is undefined.

ghc seems to follow the second interpretation, hugs the first. If the first is indeed the correct interpretation, (it is what I thought) I don't see a trivial translation to dispose of fields, as there is no easy way in Haskell98 to change the order of pattern matching without rewriting everything as a big mess of nested cases. (i mean, obviously it can be done, but the translation is harder than just placing the patterns in the right slots and dropping the field names)

-- the code --
data Foo = Foo { a,b::Int }

au = Foo { a = undefined, b = 0 }

main = case au of 
    Foo { b = 1, a = 0 } -> print "foo"
    _ -> print "bar"


ghc => error: Prelue.undefined
hugs => "bar"

Change History

12/12/05 10:51:52 changed by simonmar

  • priority changed from lowest to low.
  • severity changed from normal to minor.
  • os set to Unknown.
  • description changed.
  • architecture set to Unknown.

This was resolved in the Errata to the revised Haskell 98 Report, GHC is wrong.

http://www.haskell.org/definition/haskell98-revised-bugs.html

01/13/06 03:36:32 changed by simonmar

  • difficulty set to Unknown.
  • version changed from None to 6.4.1.

10/13/06 07:07:46 changed by igloo

  • milestone set to 6.8.

I've brought this up on the Haskell' list.

10/13/06 13:44:03 changed by igloo

Ooops, missed SimonM's comment and the errata. Just to reiterate, GHC is indeed wrong.

Sorry about that.

11/13/07 07:12:39 changed by simonmar

  • testcase changed.
  • milestone changed from 6.8 branch to _|_.

One approach to fixing this would be to generate the simple-minded left-to-right top-to-bottom pattern matching code in the desugarer, and let the simplifier optimise it.

11/13/07 09:23:30 changed by simonpj

  • description changed.

09/30/08 08:37:40 changed by simonmar

  • architecture changed from Unknown to Unknown/Multiple.

09/30/08 08:52:00 changed by simonmar

  • os changed from Unknown to Unknown/Multiple.

03/30/09 01:58:04 changed by simonpj

  • testcase set to deSugar/should_run/T246.
  • status changed from assigned to closed.
  • resolution changed from None to fixed.

Finally fixed!

Mon Mar 30 09:37:36 BST 2009  simonpj@microsoft.com
  * Fix Trac #246: order of matching in record patterns
  
  While I was looking at the desugaring of pattern matching (fixing
  Trac #3126) I finally got around to fixing another long-standing bug:
  when matching in a record pattern, GHC should match left-to-right in
  the programmer-specfied order, *not* left-to-right positionally in 
  the original record declaration.
  
  Needless to say, that requires a little more code. 
  See Note [Record patterns] in MatchCon.lhs

Changing the pattern-match order tickled a bug in GHC itself, so you must take this patch too

Mon Mar 30 09:49:12 BST 2009  simonpj@microsoft.com
  * Fix an nasty black hole, concerning computation of isRecursiveTyCon
  
  Fixing #246 (pattern-match order in record patterns) made GHC go into
  a black hole, by changing the order of patterm matching in
  TyCon.isProductTyCon!  It turned out that GHC had been avoiding the
  black hole only by the narrowest of margins up to now!
  
  The black hole concerned the computation of which type constructors
  are recursive, in TcTyDecls.calcRecFlags.  We now refrain from using
  isProductTyCon there, since it triggers the black hole (very
  indirectly).  See the "YUK YUK" comment in the body of calcRecFlags.
  
  As it turns out, the fact that TyCon.isProductTyCon matched on the
  algTcRec field was quite redundant, so I removed that too.  However,
  without the fix to calcRecFlags, this wouldn't fix the black hole
  because of the use of isRecursiveTyCon in BuildTyCl.mkNewTyConRhs.
  
  Anyway, it's fine now.

Probably don't merge to 6.10. It's a change in semantics (albeit fixing wrong semantics). If it broke GHC, it might break someone else's code.

Simon