Re: [GHC] #2647: Serious typo in IntMap.hs
#2647: Serious typo in IntMap.hs ---+ Reporter: sedillard | Owner: Type: bug| Status: closed Priority: normal | Milestone: 6.10.1 Component: libraries (other) |Version: 6.9 Severity: critical | Resolution: fixed Keywords: containers IntMap | Difficulty: Unknown Testcase: | Architecture: Unknown/Multiple Os: Unknown/Multiple | ---+ Changes (by igloo): * status: new = closed * difficulty: = Unknown * version: 6.8.3 = 6.9 * resolution: = fixed * milestone: = 6.10.1 Comment: Like others have said, good spot; thanks! I've applied your patch to HEAD and 6.10. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/2647#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler___ Glasgow-haskell-bugs mailing list Glasgow-haskell-bugs@haskell.org http://www.haskell.org/mailman/listinfo/glasgow-haskell-bugs
Re: [GHC] #2647: Serious typo in IntMap.hs
Someone has recently (since 6.8.3) taken a style pass over `IntMap.hs`. Apparently they didn't like reusing identifiers in different scopes, The reason someone made this change was to eliminate compiler warnings with -Wall, and as you rightly point out, the refactoring has introduced a new bug where there was no bug before (and there is no warning for this new bug). I think this illustrates the danger of the belief that warnings are themselves signs of danger. Sometimes a warning is simply an arbitrary style choice, and it is good practice to ignore it (or silence it) rather than change the code. Regards, Malcolm ___ Glasgow-haskell-bugs mailing list Glasgow-haskell-bugs@haskell.org http://www.haskell.org/mailman/listinfo/glasgow-haskell-bugs
Re: [GHC] #2647: Serious typo in IntMap.hs
#2647: Serious typo in IntMap.hs
--+-
Reporter: sedillard |Owner:
Type: bug| Status: new
Priority: normal |Milestone:
Component: libraries (other) | Version: 6.8.3
Severity: critical | Resolution:
Keywords: containers IntMap | Testcase:
Architecture: Unknown/Multiple | Os: Unknown/Multiple
--+-
Comment (by nominolo):
I presume this was changed in order to avoid warnings (many of the core
libraries are not {{{-Wall}}} clean yet). Good catch.
--
Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/2647#comment:1
GHC http://www.haskell.org/ghc/
The Glasgow Haskell Compiler___
Glasgow-haskell-bugs mailing list
Glasgow-haskell-bugs@haskell.org
http://www.haskell.org/mailman/listinfo/glasgow-haskell-bugs
Re: [GHC] #2647: Serious typo in IntMap.hs
#2647: Serious typo in IntMap.hs --+- Reporter: sedillard |Owner: Type: bug| Status: new Priority: normal |Milestone: Component: libraries (other) | Version: 6.8.3 Severity: critical | Resolution: Keywords: containers IntMap | Testcase: Architecture: Unknown/Multiple | Os: Unknown/Multiple --+- Comment (by [EMAIL PROTECTED]): No need to apologise. The fact that someone introduced a new bug, by changing the code to remove a -Wall warning, is a salutary lesson in the danger of trusting a mechanical warning system to tell you about real problems. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/2647#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler___ Glasgow-haskell-bugs mailing list Glasgow-haskell-bugs@haskell.org http://www.haskell.org/mailman/listinfo/glasgow-haskell-bugs
Re: [GHC] #2647: Serious typo in IntMap.hs
#2647: Serious typo in IntMap.hs --+- Reporter: sedillard |Owner: Type: bug| Status: new Priority: normal |Milestone: Component: libraries (other) | Version: 6.8.3 Severity: critical | Resolution: Keywords: containers IntMap | Testcase: Architecture: Unknown/Multiple | Os: Unknown/Multiple --+- Comment (by dons): How about adding a QuickCheck property that can distinguish the error, so it can't happen again. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/2647#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler___ Glasgow-haskell-bugs mailing list Glasgow-haskell-bugs@haskell.org http://www.haskell.org/mailman/listinfo/glasgow-haskell-bugs
Re: [GHC] #2647: Serious typo in IntMap.hs
#2647: Serious typo in IntMap.hs --+- Reporter: sedillard |Owner: Type: bug| Status: new Priority: normal |Milestone: Component: libraries (other) | Version: 6.8.3 Severity: critical | Resolution: Keywords: containers IntMap | Testcase: Architecture: Unknown/Multiple | Os: Unknown/Multiple --+- Comment (by sedillard): The properties are fine. (Well, each one on its own is fine. Together they don't cover the entire library, but that's another issue.) The problem was that the default Arbitrary instance for Int generates small numbers, so building on top of that we get trees filled with small numbers. I've changed the Arbitrary instance for IntMap / IntSet to flip random bits before building the tree. -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/2647#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler___ Glasgow-haskell-bugs mailing list Glasgow-haskell-bugs@haskell.org http://www.haskell.org/mailman/listinfo/glasgow-haskell-bugs
[GHC] #2647: Serious typo in IntMap.hs
#2647: Serious typo in IntMap.hs
--+-
Reporter: sedillard | Owner:
Type: bug| Status: new
Priority: normal | Component: libraries (other)
Version: 6.8.3 |Severity: critical
Keywords: containers IntMap |Testcase:
Architecture: Unknown/Multiple | Os: Unknown/Multiple
--+-
Someone has recently (since 6.8.3) taken a style pass over `IntMap.hs`.
Apparently they didn't like reusing identifiers in different scopes, so
they changed this function (at the very core of `IntMap`) from this
{{{
highestBitMask :: Nat - Nat
highestBitMask x
= case (x .|. shiftRL x 1) of
x - case (x .|. shiftRL x 2) of
x - case (x .|. shiftRL x 4) of
x - case (x .|. shiftRL x 8) of
x - case (x .|. shiftRL x 16) of
x - case (x .|. shiftRL x 32) of -- for 64 bit platforms
x - (x `xor` (shiftRL x 1))
}}}
to this
{{{
highestBitMask :: Nat - Nat
highestBitMask x0
= case (x0 .|. shiftRL x0 1) of
x1 - case (x1 .|. shiftRL x1 2) of
x2 - case (x2 .|. shiftRL x2 4) of
x3 - case (x3 .|. shiftRL x3 8) of
x4 - case (x3 .|. shiftRL x4 16) of
x5 - case (x4 .|. shiftRL x5 32) of -- for 64 bit platforms
x6 - (x6 `xor` (shiftRL x6 1))
}}}
If you stare at it long enough, you might find the typo. Don't cheat and
look at the patch. I'll give you a hint: quickcheck won't find it because
the tests don't use big enough integers.
Needless to say this completely breaks the library, and if it ain't
broke... :)
--
Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/2647
GHC http://www.haskell.org/ghc/
The Glasgow Haskell Compiler___
Glasgow-haskell-bugs mailing list
Glasgow-haskell-bugs@haskell.org
http://www.haskell.org/mailman/listinfo/glasgow-haskell-bugs
