Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

Arithmetic overflow in number tests #530

Closed
samplet opened this issue Nov 18, 2019 · 1 comment
Closed

Arithmetic overflow in number tests #530

samplet opened this issue Nov 18, 2019 · 1 comment

Comments

@samplet
Copy link

samplet commented Nov 18, 2019

I just saw a test failure due to “arithmetic overflow”. After a little digging, it looks like the testDividible property is to blame. It overflows whenever it tries to divide minBound by -1.

Take Int8 for example. If we try to divide -128 by -1, we end up with 128, which overflows Int8.

Here’s a log of the build failing on the GNU Guix build farm. (The seed for the tests is 18237695261609969434.)

I’m guessing it should just check for this special case like it checks for division by zero.

Hopefully this is helpful enough! I could send a PR but it seems like a lot of work for such a simple change. 😃

@samplet
Copy link
Author

samplet commented Nov 19, 2019

Hopefully this is helpful enough! I could send a PR but it seems like a lot of work for such a simple change. 😃

I spoke to soon! This is kinda tricky to sort out. I ended up treating the bounded, signed numbers differently than the rest. This required making a new testSignedNumber function that could use a new testSignedDividible. Both of these use Bounded and HasNegation so I can check the condition outlined in my first comment. This is a little unwieldy, and assumes that the signed numbers are using two’s compliment underneath (which is probably a pretty safe bet).

Here’s the patch, but there may be a much simpler solution – I just couldn’t think of it.

diff -ruN a/tests/Test/Foundation/Number.hs b/tests/Test/Foundation/Number.hs
--- a/tests/Test/Foundation/Number.hs	2019-09-01 23:58:08.000000000 -0400
+++ b/tests/Test/Foundation/Number.hs	2019-11-18 21:23:01.164549221 -0500
@@ -58,6 +58,16 @@
                       else a === (a `div` b) * b + (a `mod` b)
     ]
 
+testSignedDividible :: forall a . (Show a, Eq a, IsIntegral a, Bounded a, HasNegation a, IDivisible a, Arbitrary a, Typeable a)
+              => Proxy a -> Test
+testSignedDividible _ = Group "Divisible"
+    [ Property "(x `div` y) * y + (x `mod` y) == x" $ \(a :: a) b ->
+            case (a, b) of
+              (_, 0) -> True === True
+              (_, -1) | a == minBound -> True === True
+              _ -> a === (a `div` b) * b + (a `mod` b)
+    ]
+
 testOperatorPrecedence :: forall a . (Show a, Eq a, Prelude.Num a, IsIntegral a, Additive a, Subtractive a, Multiplicative a, Difference a ~ a, Arbitrary a, Typeable a)
                        => Proxy a -> Test
 testOperatorPrecedence _ = Group "Precedence"
@@ -83,13 +93,24 @@
     , testOperatorPrecedence proxy
     ]
 
+testSignedNumber :: (Show a, Eq a, Prelude.Num a, IsIntegral a, Bounded a, HasNegation a, Additive a, Multiplicative a, Subtractive a, Difference a ~ a, IDivisible a, Arbitrary a, Typeable a)
+           => String -> Proxy a -> Test
+testSignedNumber name proxy = Group name
+    [ testIntegral proxy
+    , testEqOrd proxy
+    , testAdditive proxy
+    , testMultiplicative proxy
+    , testSignedDividible proxy
+    , testOperatorPrecedence proxy
+    ]
+
 testNumberRefs :: [Test]
 testNumberRefs =
-    [ testNumber "Int" (Proxy :: Proxy Int)
-    , testNumber "Int8" (Proxy :: Proxy Int8)
-    , testNumber "Int16" (Proxy :: Proxy Int16)
-    , testNumber "Int32" (Proxy :: Proxy Int32)
-    , testNumber "Int64" (Proxy :: Proxy Int64)
+    [ testSignedNumber "Int" (Proxy :: Proxy Int)
+    , testSignedNumber "Int8" (Proxy :: Proxy Int8)
+    , testSignedNumber "Int16" (Proxy :: Proxy Int16)
+    , testSignedNumber "Int32" (Proxy :: Proxy Int32)
+    , testSignedNumber "Int64" (Proxy :: Proxy Int64)
     , testNumber "Integer" (Proxy :: Proxy Integer)
     , testNumber "Word" (Proxy :: Proxy Word)
     , testNumber "Word8" (Proxy :: Proxy Word8)

@vincenthz vincenthz closed this as not planned Won't fix, can't repro, duplicate, stale Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants