instance Ord IntSet is broken since #670

[Bodigrim]

Upgrading GHC 8.10.4 -> 8.10.5 appears more challenging than expected. This bug fix release actually bumps containers from 0.6.2.1 to 0.6.4.1, which is a 2500-lines long patch, breaking instance Ord IntSet:

$ cabal repl --constraint 'containers == 0.6.2.1'

> Data.IntSet.fromList [255,256] > Data.IntSet.singleton 254
True

$ cabal repl --constraint 'containers == 0.6.4.1’

> Data.IntSet.fromList [255,256] > Data.IntSet.singleton 254
False

CC @wz1000, because it would be nice to fix this one way or another in GHC 8.10.6.

[treeowl]

I have to make a release anyway. We had some Ord IntMap issues; was IntSet changed then too, and wrong? Or is it fixed in master?

…

On Wed, Jun 23, 2021, 2:15 PM Bodigrim ***@***.***> wrote: Upgrading GHC 8.10.4 -> 8.10.5 appears more challenging than expected. This bug fix release actually bumps containers from 0.6.2.1 to 0.6.4.1, which is a 2500-lines long patch, breaking instance Ord IntSet: $ cabal repl --constraint 'containers == 0.6.2.1'
> Data.IntSet.fromList [255,256] > Data.IntSet.singleton 254 True $ cabal repl --constraint 'containers == 0.6.4.1’
> Data.IntSet.fromList [255,256] > Data.IntSet.singleton 254 False CC @wz1000 <https://github.com/wz1000>, because it would be nice to fix this one way or another in GHC 8.10.6. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#783>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOOF7KSHKLOF7B34LS6UHLTUIQEBANCNFSM47GKJANA> .

[bgamari]

8.10.6 will indeed be blocked until this is fixed.

[sjakobi]

Indeed it looks like the bug is a result of the Ord IntSet changes in #670.

Ord IntMap looks correct:

containers/containers/src/Data/IntMap/Internal.hs

Lines 3308 to 3309 in 56331cd

	instance Ord a => Ord (IntMap a) where
	compare m1 m2 = compare (toList m1) (toList m2)

How about we simply go back to the old comparison via toList?

(CC @jwaldmann)

[wz1000]

Are there any major bugs in 0.6.2.1? As noted, it is a pretty big bump for a minor release of the compiler, and I don't think there was a good reason for it. I suspect it might be best to revert the containers bump for GHC 8.10.6.

[treeowl]

Yes, let's.

…

On Wed, Jun 23, 2021, 2:39 PM Simon Jakobi ***@***.***> wrote: Indeed it looks like the bug is a result of the Ord IntSet changes in #670 <#670>. Ord IntMap looks correct: https://github.com/haskell/containers/blob/56331cdd270575baf9d86da873c72facbbe0956a/containers/src/Data/IntMap/Internal.hs#L3308-L3309 How about we simply go back to the old comparison via toList? (CC @jwaldmann <https://github.com/jwaldmann>) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#783 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOOF7N3BO7XKUJMBTQWUITTUIS4LANCNFSM47GKJANA> .

[sjakobi]

Are there any major bugs in 0.6.2.1?

@wz1000 Unfortunately yes: https://hackage.haskell.org/package/containers-0.6.3.1/changelog

[treeowl]

@jwaldmann , do you think this is a really quick fix, or should I revert #670 to get this out the door?

[wz1000]

Any progress on this? 8.10.6 is getting really close and this is one of the remaining blockers.

[treeowl]

I will deal with it today.

…

On Sun, Jun 27, 2021, 7:18 AM wz1000 ***@***.***> wrote: Any progress on this? 8.10.6 is getting really close and this is one of the remaining blockers. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#783 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOOF7ILB5MOHPEN65IUWNDTU4CJBANCNFSM47GKJANA> .

[jwaldmann]

@jwaldmann , do you think this is a really quick fix, or should I revert #670 to get this out the door?

yikes. No, I don't have a quick fix right now. Will try to look into this next week.

[treeowl]

Thanks. I reverted the instance change for now (leaving the rest of your commit alone). It would be nice to understand how this slipped through the testing cracks (i.e., what made it hard to test) and plug that gap.