From ddee9788c7085a5a1ceed2dd924750427a85d61d Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Wed, 20 Dec 2023 15:12:14 -0700 Subject: [PATCH] Fix bug in v0.5.1 for comparisons that underflow (large negatives) (#89) * Fix bug in v0.5.1 for comparisons that underflow (large negatives) * Bump version to v0.5.2 for this bug fix --- Project.toml | 2 +- src/FixedPointDecimals.jl | 70 +++++++++++++++------ test/FixedDecimal.jl | 126 +++++++++++++++++++++++++++----------- 3 files changed, 142 insertions(+), 56 deletions(-) diff --git a/Project.toml b/Project.toml index 0297101..3f313f9 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "FixedPointDecimals" uuid = "fb4d412d-6eee-574d-9565-ede6634db7b0" authors = ["Fengyang Wang ", "Curtis Vogt "] -version = "0.5.1" +version = "0.5.2" [deps] Parsers = "69de0a69-1ddd-5017-9359-2bf0b02dc9f0" diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index bedb2eb..9b1a747 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -558,19 +558,34 @@ for comp_op in (:(==), :(<), :(<=)) xi, yi = promote(x.i, y.i) if f1 > f2 C = coefficient(newFD) ÷ coefficient(FD{T2,f2}) - yi, overflowed = Base.mul_with_overflow(yi, C) - if $(comp_op == :(==)) - overflowed && return false - else - # y overflowed, so it's bigger than x, since it doesn't fit in x's type - overflowed && return true + yi, wrapped = Base.mul_with_overflow(yi, C) + if wrapped + is_underflow = (y.i < 0) + if !is_underflow + # Whether we're computing `==`, `<` or `<=`, if y overflowed, it + # means it's bigger than x. + return $(comp_op == :(==)) ? false : true + else # underflow + # If y is negative, then y is definitely less than x, since y is so + # small, it doesn't even fit in y's type. + return false + end end else C = coefficient(newFD) ÷ coefficient(FD{T1,f1}) - xi, overflowed = Base.mul_with_overflow(xi, C) - # Whether we're computing `==`, `<` or `<=`, if x overflowed, it means it's - # bigger than y, so this is false. - overflowed && return false + xi, wrapped = Base.mul_with_overflow(xi, C) + if wrapped + is_underflow = (x.i < 0) + if !is_underflow + # Whether we're computing `==`, `<` or `<=`, if x overflowed, it + # means it's bigger than y, so this is false. + return false + else # underflow + # If x is negative, then x is definitely less than y, since x is so + # small, it doesn't even fit in y's type. + return $(comp_op == :(==)) ? false : true + end + end end return $comp_op(xi, yi) end @@ -593,10 +608,19 @@ for comp_op in (:(==), :(<), :(<=)) end # Now it's safe to truncate x down to y's type. xi = x % T - xi, overflowed = Base.mul_with_overflow(xi, coefficient(FD{T,f})) - # Whether we're computing `==`, `<` or `<=`, if x overflowed, it means it's - # bigger than y, so this is false. - overflowed && return false + xi, wrapped = Base.mul_with_overflow(xi, coefficient(FD{T,f})) + if wrapped + is_underflow = (x < 0) + if !is_underflow + # Whether we're computing `==`, `<` or `<=`, if x overflowed, it means it's + # bigger than y, so this is false. + return false + else # underflow + # If x is negative, then x is definitely less than y, since x is so + # small, it doesn't even fit in y's type. + return $(comp_op == :(==)) ? false : true + end + end return $comp_op(xi, y.i) end end @@ -618,12 +642,18 @@ for comp_op in (:(==), :(<), :(<=)) end # Now it's safe to truncate x down to y's type. yi = y % T - yi, overflowed = Base.mul_with_overflow(yi, coefficient(FD{T,f})) - if $(comp_op == :(==)) - overflowed && return false - else - # y overflowed, so it's bigger than x, since it doesn't fit in x's type - overflowed && return true + yi, wrapped = Base.mul_with_overflow(yi, coefficient(FD{T,f})) + if wrapped + is_underflow = (y < 0) + if !is_underflow + # Whether we're computing `==`, `<` or `<=`, if y overflowed, it means it's + # bigger than x. + return $(comp_op == :(==)) ? false : true + else # underflow + # If y is negative, then y is definitely less than x, since y is so + # small, it doesn't even fit in y's type. + return false + end end return $comp_op(x.i, yi) end diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index 7d39fac..18d87d7 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -297,6 +297,10 @@ end @test FD{Int8, 2}(0) != typemax(Int16) @test FD{Int8, 0}(0) != typemin(Int16) @test FD{Int8, 2}(0) != typemin(Int16) + + # less precision allows smaller and bigger numbers + @test typemin(FD{Int8, 1}) != typemin(FD{Int8,2}) + @test typemax(FD{Int8, 1}) != typemax(FD{Int8,2}) end @testset "inequality between types" begin @test FD{Int8, 0}(1) <= FD{Int8, 2}(1) @@ -312,41 +316,93 @@ end @test FD{Int8, 0}(4) >= FD{Int16, 4}(1) # FD{Int16,4}(4) doesn't fit @test FD{Int16, 4}(1) < FD{Int8, 0}(4) # FD{Int16,4}(4) doesn't fit - # Integer == FD - @test 1 <= FD{Int8, 2}(1) <= 1 - @test 1 >= FD{Int8, 2}(1) >= 1 - @test 2 > FD{Int8, 2}(1) - @test FD{Int8, 2}(1) < 2 - @test 2 >= FD{Int8, 2}(1) - @test FD{Int8, 2}(1) <= 2 - @test 1 <= FD{Int8, 0}(1) < 2 - - @test typemax(Int16) > FD{Int8, 0}(1) > typemin(Int16) - @test typemax(Int16) > FD{Int8, 2}(1) > typemin(Int16) - @test typemin(Int16) < FD{Int8, 0}(1) < typemax(Int16) - @test typemin(Int16) < FD{Int8, 2}(1) < typemax(Int16) - @test !(typemax(Int16) < FD{Int8, 0}(1) < typemin(Int16)) - @test !(typemax(Int16) < FD{Int8, 2}(1) < typemin(Int16)) - @test !(typemin(Int16) > FD{Int8, 0}(1) > typemax(Int16)) - @test !(typemin(Int16) > FD{Int8, 2}(1) > typemax(Int16)) - - @test typemax(Int16) > FD{Int8, 0}(-1) > typemin(Int16) - @test typemax(Int16) > FD{Int8, 2}(-1) > typemin(Int16) - @test typemin(Int16) < FD{Int8, 0}(-1) < typemax(Int16) - @test typemin(Int16) < FD{Int8, 2}(-1) < typemax(Int16) - @test !(typemax(Int16) < FD{Int8, 0}(-1) < typemin(Int16)) - @test !(typemax(Int16) < FD{Int8, 2}(-1) < typemin(Int16)) - @test !(typemin(Int16) > FD{Int8, 0}(-1) > typemax(Int16)) - @test !(typemin(Int16) > FD{Int8, 2}(-1) > typemax(Int16)) - - @test typemax(Int16) >= FD{Int8, 0}(0) >= typemin(Int16) - @test typemax(Int16) >= FD{Int8, 2}(0) >= typemin(Int16) - @test typemin(Int16) <= FD{Int8, 0}(0) <= typemax(Int16) - @test typemin(Int16) <= FD{Int8, 2}(0) <= typemax(Int16) - @test !(typemax(Int16) <= FD{Int8, 0}(-1) <= typemin(Int16)) - @test !(typemax(Int16) <= FD{Int8, 2}(-1) <= typemin(Int16)) - @test !(typemin(Int16) >= FD{Int8, 0}(-1) >= typemax(Int16)) - @test !(typemin(Int16) >= FD{Int8, 2}(-1) >= typemax(Int16)) + # less precision allows smaller numbers + @test typemin(FD{Int8, 1}) < typemin(FD{Int8,2}) + @test typemin(FD{Int8, 1}) <= typemin(FD{Int8,2}) + @test typemin(FD{Int8, 2}) > typemin(FD{Int8,1}) + @test typemin(FD{Int8, 2}) >= typemin(FD{Int8,1}) + # less precision allows bigger numbers + @test typemax(FD{Int8, 1}) > typemax(FD{Int8,2}) + @test typemax(FD{Int8, 1}) >= typemax(FD{Int8,2}) + @test typemax(FD{Int8, 2}) < typemax(FD{Int8,1}) + @test typemax(FD{Int8, 2}) <= typemax(FD{Int8,1}) + + @test !(typemin(FD{Int8, 2}) <= typemin(FD{Int8,1})) + @test !(typemin(FD{Int8, 1}) >= typemin(FD{Int8,2})) + @test !(typemin(FD{Int8, 1}) > typemin(FD{Int8,2})) + @test !(typemin(FD{Int8, 1}) >= typemin(FD{Int8,2})) + @test !(typemin(FD{Int8, 2}) < typemin(FD{Int8,1})) + @test !(typemin(FD{Int8, 2}) <= typemin(FD{Int8,1})) + @test !(typemax(FD{Int8, 1}) < typemax(FD{Int8,2})) + @test !(typemax(FD{Int8, 1}) <= typemax(FD{Int8,2})) + @test !(typemax(FD{Int8, 2}) > typemax(FD{Int8,1})) + @test !(typemax(FD{Int8, 2}) >= typemax(FD{Int8,1})) + + @testset "Integer and FD" begin + @test 1 <= FD{Int8, 2}(1) <= 1 + @test 1 >= FD{Int8, 2}(1) >= 1 + @test 2 > FD{Int8, 2}(1) + @test FD{Int8, 2}(1) < 2 + @test 2 >= FD{Int8, 2}(1) + @test FD{Int8, 2}(1) <= 2 + @test 1 <= FD{Int8, 0}(1) < 2 + + # negatives + @test -1 <= FD{Int8, 2}(-1) <= -1 + @test -1 >= FD{Int8, 2}(-1) >= -1 + @test -2 < FD{Int8, 2}(-1) + @test FD{Int8, 2}(-1) > -2 + @test -2 <= FD{Int8, 2}(-1) + @test FD{Int8, 2}(-1) >= -2 + @test -1 <= FD{Int8, 0}(-1) < 2 + + # Same types + @test typemax(Int8) > FD{Int8, 0}(1) > typemin(Int8) + @test typemax(Int8) > FD{Int8, 2}(1) > typemin(Int8) + @test typemin(Int8) < FD{Int8, 0}(1) < typemax(Int8) + @test typemin(Int8) < FD{Int8, 2}(1) < typemax(Int8) + @test !(typemax(Int8) < FD{Int8, 0}(1) < typemin(Int8)) + @test !(typemax(Int8) < FD{Int8, 2}(1) < typemin(Int8)) + @test !(typemin(Int8) > FD{Int8, 0}(1) > typemax(Int8)) + @test !(typemin(Int8) > FD{Int8, 2}(1) > typemax(Int8)) + + @test typemax(Int8) > FD{Int8, 0}(-1) > typemin(Int8) + @test typemax(Int8) > FD{Int8, 2}(-1) > typemin(Int8) + @test typemin(Int8) < FD{Int8, 0}(-1) < typemax(Int8) + @test typemin(Int8) < FD{Int8, 2}(-1) < typemax(Int8) + @test !(typemax(Int8) < FD{Int8, 0}(-1) < typemin(Int8)) + @test !(typemax(Int8) < FD{Int8, 2}(-1) < typemin(Int8)) + @test !(typemin(Int8) > FD{Int8, 0}(-1) > typemax(Int8)) + @test !(typemin(Int8) > FD{Int8, 2}(-1) > typemax(Int8)) + + # Different types + @test typemax(Int16) > FD{Int8, 0}(1) > typemin(Int16) + @test typemax(Int16) > FD{Int8, 2}(1) > typemin(Int16) + @test typemin(Int16) < FD{Int8, 0}(1) < typemax(Int16) + @test typemin(Int16) < FD{Int8, 2}(1) < typemax(Int16) + @test !(typemax(Int16) < FD{Int8, 0}(1) < typemin(Int16)) + @test !(typemax(Int16) < FD{Int8, 2}(1) < typemin(Int16)) + @test !(typemin(Int16) > FD{Int8, 0}(1) > typemax(Int16)) + @test !(typemin(Int16) > FD{Int8, 2}(1) > typemax(Int16)) + + @test typemax(Int16) > FD{Int8, 0}(-1) > typemin(Int16) + @test typemax(Int16) > FD{Int8, 2}(-1) > typemin(Int16) + @test typemin(Int16) < FD{Int8, 0}(-1) < typemax(Int16) + @test typemin(Int16) < FD{Int8, 2}(-1) < typemax(Int16) + @test !(typemax(Int16) < FD{Int8, 0}(-1) < typemin(Int16)) + @test !(typemax(Int16) < FD{Int8, 2}(-1) < typemin(Int16)) + @test !(typemin(Int16) > FD{Int8, 0}(-1) > typemax(Int16)) + @test !(typemin(Int16) > FD{Int8, 2}(-1) > typemax(Int16)) + + @test typemax(Int16) >= FD{Int8, 0}(0) >= typemin(Int16) + @test typemax(Int16) >= FD{Int8, 2}(0) >= typemin(Int16) + @test typemin(Int16) <= FD{Int8, 0}(0) <= typemax(Int16) + @test typemin(Int16) <= FD{Int8, 2}(0) <= typemax(Int16) + @test !(typemax(Int16) <= FD{Int8, 0}(-1) <= typemin(Int16)) + @test !(typemax(Int16) <= FD{Int8, 2}(-1) <= typemin(Int16)) + @test !(typemin(Int16) >= FD{Int8, 0}(-1) >= typemax(Int16)) + @test !(typemin(Int16) >= FD{Int8, 2}(-1) >= typemax(Int16)) + end end @testset "128-bit conversion correctness" begin