diff --git a/changelog b/changelog --- a/changelog +++ b/changelog @@ -1,3 +1,20 @@ +0.3.10.0 + + * Fix run-time crash (introduced in 0.3.9.0) in Eq and Ord when + comparing values with (opposite) extreme exponents. + Versions prior 0.3.9.0 instead reported invalid results. + + * Document that 'normalize' may wrap around (overflow) the exponent. + Therefore current 'toDecimalDigits' doesn't work correctly, + and in turn values of 'Scientific' with exponents close to + maxBound cannot be printed reliably. + This is limitation has been there for at least a decade + (it is present in scientific-0.3.4.4, the oldest scientific + buildable with GHC-8.0). + The toDecimalDigits issue cannot be fixed without at least changing + its type signature (e.g. use Integer for a scientific exponent), + thus postponed until next major release. + 0.3.9.0 * Reimplement most internal functions which used to normalize inputs diff --git a/scientific.cabal b/scientific.cabal --- a/scientific.cabal +++ b/scientific.cabal @@ -1,5 +1,5 @@ name: scientific -version: 0.3.9.0 +version: 0.3.10.0 synopsis: Numbers represented using scientific notation description: "Data.Scientific" provides the number type 'Scientific'. Scientific numbers are diff --git a/src/Data/Scientific.hs b/src/Data/Scientific.hs --- a/src/Data/Scientific.hs +++ b/src/Data/Scientific.hs @@ -237,8 +237,12 @@ GT -> c1 * magnitude (e1 - e2) == c2 LT -> c1 == c2 * magnitude (e2 - e1) where - log1 = integerLog10' c1 + e1 - log2 = integerLog10' c2 + e2 + -- note: the difference e1 - e2 may overflow and be negative, + -- however we shouldn't enter 'otherwise' branch in that case: + -- The log1 /= log2 check should rule those extreme cases. + -- + log1 = toInteger (integerLog10' c1) + toInteger e1 + log2 = toInteger (integerLog10' c2) + toInteger e2 -- | Scientific numbers can be safely compared for ordering. No magnitude @10^e@ -- is calculated so there's no risk of a blowup in space or time when comparing @@ -263,8 +267,8 @@ GT -> compare (c1 * magnitude (e1 - e2)) c2 LT -> compare c1 (c2 * magnitude (e2 - e1)) where - log1 = integerLog10' c1 + e1 - log2 = integerLog10' c2 + e2 + log1 = toInteger (integerLog10' c1) + toInteger e1 + log2 = toInteger (integerLog10' c2) + toInteger e2 -- | /WARNING:/ '+' and '-' compute the 'Integer' magnitude: @10^e@ where @e@ is -- the difference between the @'base10Exponent's@ of the arguments. If these @@ -1109,6 +1113,10 @@ -- -- The last property means that the coefficient will be normalized, i.e. doesn't -- contain trailing zeros. +-- +-- /Note:/ 'toDecimalDigits' uses 'normalize' and therefore ill-behaves +-- for extreme values. +-- toDecimalDigits :: Scientific -> ([Int], Int) toDecimalDigits (Scientific 0 _) = ([0], 0) toDecimalDigits (Scientific c' e') = @@ -1127,6 +1135,13 @@ -- -- You should rarely have a need for this function since scientific numbers are -- automatically normalized when pretty-printed and in 'toDecimalDigits'. +-- +-- /Note:/ 'normalize' ill-behaves for extreme values: the Int exponent +-- may overflow +-- +-- >>> normalize (scientific 10 maxBound) == scientific 1 minBound +-- True +-- normalize :: Scientific -> Scientific normalize (Scientific c e) = case compare c 0 of GT -> normalizePositive c e diff --git a/src/Utils.hs b/src/Utils.hs --- a/src/Utils.hs +++ b/src/Utils.hs @@ -75,7 +75,8 @@ -- | @magnitude e == 10 ^ e@ magnitude :: Num a => Int -> a -magnitude e | e < maxExpt = cachedPow10 e +magnitude e | e < 0 = error "magnitude: with negative argument" + | e < maxExpt = cachedPow10 e | otherwise = cachedPow10 hi * 10 ^ (e - hi) where cachedPow10 = fromInteger . Primitive.indexArray expts10 diff --git a/test/test.hs b/test/test.hs --- a/test/test.hs +++ b/test/test.hs @@ -64,12 +64,12 @@ ] , testGroup "toBoundedInteger" - [ testCase "1e1000000" $ Nothing @=? toBoundedInteger @Int (read "1e1000000") + [ testCase "1e1000000" $ Nothing @=? toBoundedInteger @Int (read "1e1000000") , testCase "10...0e-1" $ Nothing @=? toBoundedInteger @Int (read $ '1' : replicate 1000000 '0' ++ "e-1") ] , testGroup "floatingOrInteger" - [ testCase "1e1000000" $ Right (10 ^ (1000000 :: Int) :: Integer) @=? floatingOrInteger @Double @Integer (read "1e1000000") + [ testCase "1e1000000" $ Right (10 ^ (1000000 :: Int) :: Integer) @=? floatingOrInteger @Double @Integer (read "1e1000000") , testCase "10...0e-1" $ Right (10 ^ ( 999999 :: Int) :: Integer) @=? floatingOrInteger @Double @Integer (read $ '1' : replicate 1000000 '0' ++ "e-1") ] @@ -184,11 +184,26 @@ [ testProperty "==" $ \(s1 :: Scientific) (s2 :: Scientific) -> (s1 == s2) == (toRational s1 == toRational s2) , testProperty "s == s" $ \(s :: Scientific) -> s == s + + , testProperty "extreme" $ + -- This was true in scientific-0.3.8.0 + -- due 'normalize' inherent limitation causing overflow + -- + -- in scientific-0.3.9.0 this crashed. + not (scientific 10 maxBound == scientific 1 minBound) + ] , testGroup "Ord" [ testProperty "compare" $ \(s1 :: Scientific) (s2 :: Scientific) -> compare s1 s2 == compare (toRational s1) (toRational s2) + + , testProperty "extreme" $ + -- This was true in scientific-0.3.8.0 + -- due 'normalize' inherent limitation causing overflow + -- + -- in scientific-0.3.9.0 this crashed. + compare (scientific 10 maxBound) (scientific 1 minBound) === GT ] , testGroup "Num" @@ -279,7 +294,7 @@ ] , testGroup "toBoundedInteger" [ testGroup "correct conversion" - + [ testCase "100e-2" $ toBoundedInteger @Int (read "100e-2") @?= Just 1 , testProperty "Int64" $ toBoundedIntegerConversion (undefined :: Int64) , testProperty "Word64" $ toBoundedIntegerConversion (undefined :: Word64) @@ -454,7 +469,7 @@ , (10, (\c e' e -> scientific (c * 10 ^ min 10 (abs e')) e) <$> QC.arbitrary <*> intGen <*> intGen) ] - shrink s = + shrink s = [ scientific c e | (c, e) <- QC.shrink (Scientific.coefficient s, Scientific.base10Exponent s) ]