More functions from base-4.7.0.0 and base-4.8.0.0

[RyanGlScott]

I implemented some low-hanging fruit from #24:

• Backport bitDefault, testBitDefault, and popCountDefault in Data.Bits.Compat to all versions of base
  • Backport toIntegralSized to only base-4.7 (since it is implemented in terms of bitSizeMaybe)
• Backport nub and nubBy (as well as union and unionBy, which are implemented in terms of them) to fix a logic error
• Backport byteSwap16, byteSwap32, and byteSwap64 to Data.Word.Compat
• Backport fillBytes in Foreign.Marshal.Utils.Compat
• Backport showFFloatAlt and showGFloatAlt to Numeric.Compat

[sol]

Following "one assertion per test", this should be two test cases. In this particular case I think it's even more natural to do so:

it "shows a RealFloat value, always using decimal notation" $ do
  showFFloatAlt Nothing  (12 :: Double) "" `shouldBe` "12.0"
it "allows to specify the decimal places" $ do
  showFFloatAlt (Just 4) (12 :: Double) "" `shouldBe` "12.0000"

[sol]

Regarding nub, union, nubBy, etc. I think we need to decide if we want to backport bug fixes in general. If yes, unconditionally, or only if they are relevant in practice?

As I understand it there was no "bug" with nub and union, my assumption is that the old base versions and our custom version have the same observable behavior. If that is indeed the case, I would not redefine them.

For nubBy I think the example given in the GHC ticket is contrived and I have not seen nubBy seen used in that way in real code. If there is real world demand for that change, I would consider to backport it, if not I would skip it.

While providing definitions that were added in a later version of base is always safe from my perspective. The issue I have with redefining things is that users can run into name clashes. So I would only do it if we have a strong case for it.

I assume all the other changes are additions, right? They are fine with me.

[sol]

Of course, as we already redefine half of Data.List we may just redefine nubBy as well. So my point is not that strong.

[RyanGlScott]

I would consider the change to nubBy to be breaking, since the old and new implementations yield completely different answers with the provided test cases.

I don't really see redefined functions in base-compat as a huge issue, especially since you shouldn't be mixing Data.List and Data.List.Compat anyway. My philosophy is if it can be backported without too much pain, toss it in. I've never used nubBy before, but I'm sure there are people who would like to have it (as the original GHC ticket evidences).

[sol]

@RyanGlScott As we already have most of Data.List redefined, let's go with the change.

When it comes to whether this change was driven by practical demand, I think I disagree. As I understand it the ticket / bug report was the result of a theoretical discussion on IRC. I could be wrong, and I'm not saying there could not possibly be a practical case where this matters. But I still think nubBy is not commonly used in a way where this difference matters ;).

[sol]

Regarding my point about the example / bug for nubBy being contrived, @soenkehahn is much better in formalizing this than me:

The nubBy is meant to be used with equivalence relations, as stated in the docs. The given comparing function in the bug report is not an equivalence since it violates symmetry.

I wouldn't consider this a bug in nubBy.

[sol]

@RyanGlScott And I still think there is no good reason to redefine nub and union, as for = there is no difference in behavior for the two versions of nubBy (please correct me if you think I'm wrong).

[RyanGlScott]

There are differences in behavior for the two versions of nubBy, though (as well as nub, union, and unionBy), and the test cases in test/Data/List/CompatSpec.hs provide examples. If you run this in GHC 7.10:

data Asymmetric = A | B deriving Show

instance Eq Asymmetric where
  A == _ = True
  B == _ = False

main :: IO ()
main = do
  print $ nub [A, B]
  print $ nubBy (<) "12"
  print $ union [A] [A, B]
  print $ unionBy (<) "1" "21"

you'd get:

[A]
"1"
[A]
"11"

whereas in base-4.7.0.0 or earlier, it would output this:

[A,B]
"12"
[A,B]
"1"

You're right that none of these examples use equivalence relations, but they are discrepancies nonetheless.