From c81bb83410799f98057d5058bc5f12a441925b4d Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Thu, 23 Nov 2023 19:36:34 -0700 Subject: [PATCH 01/21] Let all the FixedDecimals operations overflow, matching Int overflow --- src/FixedPointDecimals.jl | 24 +++++++++++++++++------- test/FixedDecimal.jl | 30 ++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index 31de943..d779097 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -187,13 +187,13 @@ end # these functions are needed to avoid InexactError when converting from the # integer type -Base.:*(x::Integer, y::FD{T, f}) where {T, f} = reinterpret(FD{T, f}, T(x * y.i)) -Base.:*(x::FD{T, f}, y::Integer) where {T, f} = reinterpret(FD{T, f}, T(x.i * y)) +Base.:*(x::Integer, y::FD{T, f}) where {T, f} = reinterpret(FD{T, f}, *(promote(x, y.i)...)) +Base.:*(x::FD{T, f}, y::Integer) where {T, f} = reinterpret(FD{T, f}, *(promote(x.i, y)...)) function Base.:/(x::FD{T, f}, y::FD{T, f}) where {T, f} powt = coefficient(FD{T, f}) quotient, remainder = fldmod(widemul(x.i, powt), y.i) - reinterpret(FD{T, f}, T(_round_to_nearest(quotient, remainder, y.i))) + reinterpret(FD{T, f}, _round_to_nearest(quotient, remainder, y.i)) end # These functions allow us to perform division with integers outside of the range of the @@ -202,12 +202,12 @@ function Base.:/(x::Integer, y::FD{T, f}) where {T, f} powt = coefficient(FD{T, f}) powtsq = widemul(powt, powt) quotient, remainder = fldmod(widemul(x, powtsq), y.i) - reinterpret(FD{T, f}, T(_round_to_nearest(quotient, remainder, y.i))) + reinterpret(FD{T, f}, _round_to_nearest(quotient, remainder, y.i)) end function Base.:/(x::FD{T, f}, y::Integer) where {T, f} quotient, remainder = fldmod(x.i, y) - reinterpret(FD{T, f}, T(_round_to_nearest(quotient, remainder, y))) + reinterpret(FD{T, f}, _round_to_nearest(quotient, remainder, y)) end # integerification @@ -362,12 +362,22 @@ end for divfn in [:div, :fld, :fld1, :cld] # div(x.i, y.i) eliminates the scaling coefficient, so we call the FD constructor. # We don't need any widening logic, since we won't be multiplying by the coefficient. - @eval Base.$divfn(x::T, y::T) where {T <: FD} = T($divfn(x.i, y.i)) + #@eval Base.$divfn(x::T, y::T) where {T <: FD} = T($divfn(x.i, y.i)) + # @eval Base.$divfn(x::T, y::T) where {T <: FD} = $divfn(promote(x.i, y.i)...) + # TODO(PR): I'm not sure about this one... + # What should it *mean* for `typemax(FD) ÷ FD(0.5)` to overflow? + @eval function Base.$divfn(x::T, y::T) where {T <: FD} + C = coefficient(T) + return reinterpret(T, C * $divfn(promote(x.i, y.i)...)) + end end if VERSION >= v"1.4.0-" # div(x.i, y.i) eliminates the scaling coefficient, so we call the FD constructor. # We don't need any widening logic, since we won't be multiplying by the coefficient. - Base.div(x::T, y::T, r::RoundingMode) where {T <: FD} = T(div(x.i, y.i, r)) + @eval function Base.div(x::T, y::T, r::RoundingMode) where {T <: FD} + C = coefficient(T) + return reinterpret(T, C * div(x.i, y.i, r)) + end end Base.convert(::Type{AbstractFloat}, x::FD) = convert(floattype(typeof(x)), x) diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index 723f6ba..f649b3f 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -712,6 +712,36 @@ end end end +@testset "overflow" begin + @testset "addition" begin + @test typemax(FD2) + eps(FD2) == typemin(FD2) + @test typemin(FD2) + (-eps(FD2)) == typemax(FD2) + end + + @testset "subtraction" begin + @test typemin(FD2) - eps(FD2) == typemax(FD2) + @test typemax(FD2) - (-eps(FD2)) == typemin(FD2) + end + + @testset "multiplication" begin + @test typemax(FD2) * 2 == FD2(-0.02) + @test typemin(FD2) * 2 == FD2(0) + end + + @testset "division" begin + @test typemax(FD2) / FD2(0.5) == FD2(-0.02) + @test typemin(FD2) / FD2(0.5) == FD2(0) + end + + @testset "truncating division" begin + # TODO(PR): Is this the expected value? + @test typemax(FD2) ÷ FD2(0.5) == FD2(-0.16) + @test typemin(FD2) ÷ FD2(0.5) == FD2(0.16) + @test typemax(FD2) ÷ eps(FD2) == FD2(-1) + @test typemin(FD2) ÷ eps(FD2) == FD2(0) + end +end + @testset "isinteger" begin # Note: Test cannot be used unless we can construct `FD{Int8,6}` # @testset "overflow" begin From 9da03cd67e4866862c1cf52f23d9c57e3ad0a374 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Wed, 6 Dec 2023 17:04:09 -0700 Subject: [PATCH 02/21] Add checked_* methods for FixedDecimals --- src/FixedPointDecimals.jl | 44 +++++++++++++++++++++++++++++++++++++++ test/FixedDecimal.jl | 7 +++++-- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index d779097..131154f 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -380,6 +380,50 @@ if VERSION >= v"1.4.0-" end end +# --- Checked arithmetic --- + +Base.checked_add(x::FD, y::FD) = Base.checked_add(promote(x, y)...) +Base.checked_sub(x::FD, y::FD) = Base.checked_sub(promote(x, y)...) +Base.checked_mul(x::FD, y::FD) = Base.checked_mul(promote(x, y)...) +Base.checked_div(x::FD, y::FD) = Base.checked_div(promote(x, y)...) + +Base.checked_add(x, y::FD) = Base.checked_add(promote(x, y)...) +Base.checked_add(x::FD, y) = Base.checked_add(promote(x, y)...) +Base.checked_sub(x, y::FD) = Base.checked_sub(promote(x, y)...) +Base.checked_sub(x::FD, y) = Base.checked_sub(promote(x, y)...) +Base.checked_mul(x, y::FD) = Base.checked_mul(promote(x, y)...) +Base.checked_mul(x::FD, y) = Base.checked_mul(promote(x, y)...) +Base.checked_div(x, y::FD) = Base.checked_div(promote(x, y)...) +Base.checked_div(x::FD, y) = Base.checked_div(promote(x, y)...) + +function Base.checked_add(x::T, y::T) where {T<:FD} + z, b = Base.add_with_overflow(x.i, y.i) + b && Base.Checked.throw_overflowerr_binaryop(:+, x, y) + return reinterpret(T, z) +end +function Base.checked_sub(x::T, y::T) where {T<:FD} + z, b = Base.sub_with_overflow(x.i, y.i) + b && Base.Checked.throw_overflowerr_binaryop(:-, x, y) + return reinterpret(T, z) +end +function Base.checked_mul(x::FD{T,f}, y::FD{T,f}) where {T, f} + powt = coefficient(FD{T, f}) + quotient, remainder = fldmodinline(widemul(x.i, y.i), powt) + v = _round_to_nearest(quotient, remainder, powt) + typemin(T) <= v <= typemax(T) || Base.Checked.throw_overflowerr_binaryop(:*, x, y) + return reinterpret(FD{T, f}, T(v)) +end +function Base.checked_div(x::FD{T,f}, y::FD{T,f}) where {T,f} + C = coefficient(FD{T, f}) + v1 = div(promote(x.i, y.i)...) + v2, b = Base.Checked.mul_with_overflow(C, v1) + b && Base.Checked.throw_overflowerr_binaryop(:÷, x, y) + typemin(T) <= v2 <= typemax(T) || Base.Checked.throw_overflowerr_binaryop(:÷, x, y) + return reinterpret(FD{T, f}, T(v2)) +end + +# -------------------------- + Base.convert(::Type{AbstractFloat}, x::FD) = convert(floattype(typeof(x)), x) function Base.convert(::Type{TF}, x::FD{T, f}) where {TF <: AbstractFloat, T, f} convert(TF, x.i / coefficient(FD{T, f}))::TF diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index f649b3f..957fa4d 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -625,8 +625,11 @@ end end @testset "limits" begin - @test_throws InexactError Int8(1) / FD{Int8,2}(0.4) - @test_throws InexactError FD{Int8,2}(1) / FD{Int8,2}(0.4) + @test_throws OverflowError Base.checked_add(FD{Int8,2}(1), FD{Int8,2}(1)) + @test_throws OverflowError Base.checked_add(FD{Int8,2}(1), FD{Int8,2}(0.4)) + + @test_throws OverflowError Base.checked_div(Int8(1), FD{Int8,2}(0.4)) + @test_throws OverflowError Base.checked_div(FD{Int8,2}(1), FD{Int8,2}(0.4)) end @testset "limits of $T" for T in CONTAINER_TYPES From f19c14747ef0210e30f78ee33fadd1b5014243c0 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Thu, 7 Dec 2023 10:14:54 -0700 Subject: [PATCH 03/21] Fix dispatch for checked_mul and checked_div. Fix checked_div implementation --- src/FixedPointDecimals.jl | 11 +++++------ test/FixedDecimal.jl | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index 131154f..1256045 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -406,20 +406,19 @@ function Base.checked_sub(x::T, y::T) where {T<:FD} b && Base.Checked.throw_overflowerr_binaryop(:-, x, y) return reinterpret(T, z) end -function Base.checked_mul(x::FD{T,f}, y::FD{T,f}) where {T, f} +function Base.checked_mul(x::FD{T,f}, y::FD{T,f}) where {T<:Integer,f} powt = coefficient(FD{T, f}) quotient, remainder = fldmodinline(widemul(x.i, y.i), powt) v = _round_to_nearest(quotient, remainder, powt) typemin(T) <= v <= typemax(T) || Base.Checked.throw_overflowerr_binaryop(:*, x, y) return reinterpret(FD{T, f}, T(v)) end -function Base.checked_div(x::FD{T,f}, y::FD{T,f}) where {T,f} +function Base.checked_div(x::FD{T,f}, y::FD{T,f}) where {T<:Integer,f} C = coefficient(FD{T, f}) - v1 = div(promote(x.i, y.i)...) - v2, b = Base.Checked.mul_with_overflow(C, v1) + # Note: The div() will already throw for divide-by-zero and typemin(T) ÷ -1. + v, b = Base.Checked.mul_with_overflow(C, div(x.i, y.i)) b && Base.Checked.throw_overflowerr_binaryop(:÷, x, y) - typemin(T) <= v2 <= typemax(T) || Base.Checked.throw_overflowerr_binaryop(:÷, x, y) - return reinterpret(FD{T, f}, T(v2)) + return reinterpret(FD{T, f}, v) end # -------------------------- diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index 957fa4d..49faf9d 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -628,7 +628,7 @@ end @test_throws OverflowError Base.checked_add(FD{Int8,2}(1), FD{Int8,2}(1)) @test_throws OverflowError Base.checked_add(FD{Int8,2}(1), FD{Int8,2}(0.4)) - @test_throws OverflowError Base.checked_div(Int8(1), FD{Int8,2}(0.4)) + @test_throws OverflowError Base.checked_div(Int8(1), FD{Int8,2}(0.5)) @test_throws OverflowError Base.checked_div(FD{Int8,2}(1), FD{Int8,2}(0.4)) end From ea46f72c2532b74052d74e665291e7104fab2e87 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Thu, 7 Dec 2023 10:25:32 -0700 Subject: [PATCH 04/21] Add checked `/` operation for FDs: `checked_decimal_division` --- src/FixedPointDecimals.jl | 24 ++++++++++++++++++++++++ test/FixedDecimal.jl | 7 +++++++ 2 files changed, 31 insertions(+) diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index 1256045..71b0d13 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -421,6 +421,30 @@ function Base.checked_div(x::FD{T,f}, y::FD{T,f}) where {T<:Integer,f} return reinterpret(FD{T, f}, v) end +# We introduce a new function for this since Base.Checked only supports integers, and ints +# don't have a decimal division operation. +""" + FixedPointDecimals.checked_decimal_division(x::FD, y::FD) -> FD + +Calculates `x / y`, checking for overflow errors where applicable. + +The overflow protection may impose a perceptible performance penalty. + +See also: +- `Base.checked_div` for truncating division. +""" +checked_decimal_division(x::FD, y::FD) = checked_decimal_division(promote(x, y)...) +checked_decimal_division(x, y::FD) = checked_decimal_division(promote(x, y)...) +checked_decimal_division(x::FD, y) = checked_decimal_division(promote(x, y)...) + +function checked_decimal_division(x::FD{T,f}, y::FD{T,f}) where {T<:Integer,f} + powt = coefficient(FD{T, f}) + quotient, remainder = fldmod(widemul(x.i, powt), y.i) + v = _round_to_nearest(quotient, remainder, y.i) + typemin(T) <= v <= typemax(T) || Base.Checked.throw_overflowerr_binaryop(:/, x, y) + return reinterpret(FD{T, f}, v) +end + # -------------------------- Base.convert(::Type{AbstractFloat}, x::FD) = convert(floattype(typeof(x)), x) diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index 49faf9d..a1f512d 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -630,6 +630,13 @@ end @test_throws OverflowError Base.checked_div(Int8(1), FD{Int8,2}(0.5)) @test_throws OverflowError Base.checked_div(FD{Int8,2}(1), FD{Int8,2}(0.4)) + + @testset "checked_decimal_division" begin + using FixedPointDecimals: checked_decimal_division + + @test checked_decimal_division(Int8(1), FD{Int8,2}(0.8)) == FD{Int8,2}(1.25) + @test_throws OverflowError checked_decimal_division(Int8(1), FD{Int8,2}(0.7)) + end end @testset "limits of $T" for T in CONTAINER_TYPES From c465e6f718d72015dbbc707edc0b875941210fb3 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Thu, 7 Dec 2023 10:45:05 -0700 Subject: [PATCH 05/21] Add README sections around overflow, conversions and inexact errors --- README.md | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/README.md b/README.md index 698f202..f10abf2 100644 --- a/README.md +++ b/README.md @@ -44,3 +44,60 @@ julia> 0.1 + 0.2 julia> FixedDecimal{Int,1}(0.1) + FixedDecimal{Int,1}(0.2) FixedDecimal{Int64,1}(0.3) ``` + +### Arithmetic details: Overflow and checked math + +By default, all arithmetic operations on FixedDecimals **will silently overflow**, following the standard behavior for bit integer types in Julia. For example: +```julia +julia> FixedDecimal{Int8,2}(1.0) + FixedDecimal{Int8,2}(1.0) +FixedDecimal{Int8,2}(-0.56) + +julia> -FixedDecimal{Int8,2}(-1.28) # negative typemin wraps to typemin again +FixedDecimal{Int8,2}(-1.28) + +julia> abs(FixedDecimal{Int8,2}(-1.28)) # negative typemin wraps to typemin again +FixedDecimal{Int8,2}(-1.28) +``` + +In most applications dealing with `FixedDecimals`, you will likely want to use the **checked arithmetic** operations instead. These operations will _throw an OverflowError_ on overflow or underflow, rather than silently wrapping. For example: +```julia +julia> Base.checked_mul(FixedDecimal{Int8,2}(1.2), FixedDecimal{Int8,2}(1.2)) +ERROR: OverflowError: 1.20 * 1.20 overflowed for type FixedDecimal{Int8, 2} + +julia> Base.checked_add(FixedDecimal{Int8,2}(1.2), 1) +ERROR: OverflowError: 1.20 + 1.00 overflowed for type FixedDecimal{Int8, 2} + +julia> Base.checked_div(Int8(1), FixedDecimal{Int8,2}(0.5)) +ERROR: OverflowError: 1.00 ÷ 0.50 overflowed for type FixedDecimal{Int8, 2} +``` + +**Checked division:** Note that `checked_div` performs truncating, integer division. Julia Base does not provide a function to perform checked decimal division, so we provide one in this package, `FixedPointDecimals.checked_decimal_division`. + +Here are all the checked arithmetic operations supported by `FixedDecimal`s: +- `Base.checked_add` +- `Base.checked_sub` +- `Base.checked_mul` +- `Base.checked_div` +- `FixedPointDecimals.checked_decimal_division` +- `Base.checked_cld` +- `Base.checked_fld` +- `Base.checked_rem` +- `Base.checked_mod` +- `Base.checked_neg` +- `Base.checked_abs` + +### Conversions, Promotions, and Inexact Errors. + +Note that arithmetic operations will _promote_ all arguments to the same FixedDecimal type +before performing the operation. If you are promoting a non-FixedDecimal _number_ to a FixedDecimal, there is always a chance that the Number will not fit in the FD type. In that case, the conversion will throw an exception. Here are some examples: +```julia +julia> FixedDecimal{Int8,2}(2) # 200 doesn't fit in Int8 +ERROR: InexactError: convert(FixedDecimal{Int8, 2}, 2) + +julia> FixedDecimal{Int8,2}(1) + 2 # Same here: 2 is promoted to FD{Int8,2}(2) +ERROR: InexactError: convert(FixedDecimal{Int8, 2}, 2) + +julia> FixedDecimal{Int8,2}(1) + FixedDecimal{Int8,1}(2) # Promote to the higher-precision type again throws. +ERROR: InexactError: convert(FixedDecimal{Int8, 2}, 2.0) +``` + From fc1d927fd32f08630d9d7f9b204c575d522f53f7 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Thu, 7 Dec 2023 12:41:26 -0700 Subject: [PATCH 06/21] Add the missing checked_ operations: fld,cld,rem,mod,abs,neg. Add tests --- README.md | 22 +++++++++---------- src/FixedPointDecimals.jl | 45 +++++++++++++++++++++++++++++++++------ test/FixedDecimal.jl | 23 ++++++++++++++++++-- 3 files changed, 71 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index f10abf2..c3449a7 100644 --- a/README.md +++ b/README.md @@ -74,17 +74,17 @@ ERROR: OverflowError: 1.00 ÷ 0.50 overflowed for type FixedDecimal{Int8, 2} **Checked division:** Note that `checked_div` performs truncating, integer division. Julia Base does not provide a function to perform checked decimal division, so we provide one in this package, `FixedPointDecimals.checked_decimal_division`. Here are all the checked arithmetic operations supported by `FixedDecimal`s: -- `Base.checked_add` -- `Base.checked_sub` -- `Base.checked_mul` -- `Base.checked_div` -- `FixedPointDecimals.checked_decimal_division` -- `Base.checked_cld` -- `Base.checked_fld` -- `Base.checked_rem` -- `Base.checked_mod` -- `Base.checked_neg` -- `Base.checked_abs` +- `Base.checked_add(x,y)` +- `Base.checked_sub(x,y)` +- `Base.checked_mul(x,y)` +- `Base.checked_div(x,y)` +- `FixedPointDecimals.checked_decimal_division(x,y)` +- `Base.checked_cld(x,y)` +- `Base.checked_fld(x,y)` +- `Base.checked_rem(x,y)` +- `Base.checked_mod(x,y)` +- `Base.checked_neg(x)` +- `Base.checked_abs(x)` ### Conversions, Promotions, and Inexact Errors. diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index 71b0d13..a84eb15 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -386,6 +386,10 @@ Base.checked_add(x::FD, y::FD) = Base.checked_add(promote(x, y)...) Base.checked_sub(x::FD, y::FD) = Base.checked_sub(promote(x, y)...) Base.checked_mul(x::FD, y::FD) = Base.checked_mul(promote(x, y)...) Base.checked_div(x::FD, y::FD) = Base.checked_div(promote(x, y)...) +Base.checked_cld(x::FD, y::FD) = Base.checked_cld(promote(x, y)...) +Base.checked_fld(x::FD, y::FD) = Base.checked_fld(promote(x, y)...) +Base.checked_rem(x::FD, y::FD) = Base.checked_rem(promote(x, y)...) +Base.checked_mod(x::FD, y::FD) = Base.checked_mod(promote(x, y)...) Base.checked_add(x, y::FD) = Base.checked_add(promote(x, y)...) Base.checked_add(x::FD, y) = Base.checked_add(promote(x, y)...) @@ -395,6 +399,14 @@ Base.checked_mul(x, y::FD) = Base.checked_mul(promote(x, y)...) Base.checked_mul(x::FD, y) = Base.checked_mul(promote(x, y)...) Base.checked_div(x, y::FD) = Base.checked_div(promote(x, y)...) Base.checked_div(x::FD, y) = Base.checked_div(promote(x, y)...) +Base.checked_cld(x, y::FD) = Base.checked_cld(promote(x, y)...) +Base.checked_cld(x, y::FD) = Base.checked_cld(promote(x, y)...) +Base.checked_fld(x::FD, y) = Base.checked_fld(promote(x, y)...) +Base.checked_fld(x, y::FD) = Base.checked_fld(promote(x, y)...) +Base.checked_rem(x::FD, y) = Base.checked_rem(promote(x, y)...) +Base.checked_rem(x, y::FD) = Base.checked_rem(promote(x, y)...) +Base.checked_mod(x::FD, y) = Base.checked_mod(promote(x, y)...) +Base.checked_mod(x, y::FD) = Base.checked_mod(promote(x, y)...) function Base.checked_add(x::T, y::T) where {T<:FD} z, b = Base.add_with_overflow(x.i, y.i) @@ -413,12 +425,33 @@ function Base.checked_mul(x::FD{T,f}, y::FD{T,f}) where {T<:Integer,f} typemin(T) <= v <= typemax(T) || Base.Checked.throw_overflowerr_binaryop(:*, x, y) return reinterpret(FD{T, f}, T(v)) end -function Base.checked_div(x::FD{T,f}, y::FD{T,f}) where {T<:Integer,f} - C = coefficient(FD{T, f}) - # Note: The div() will already throw for divide-by-zero and typemin(T) ÷ -1. - v, b = Base.Checked.mul_with_overflow(C, div(x.i, y.i)) - b && Base.Checked.throw_overflowerr_binaryop(:÷, x, y) - return reinterpret(FD{T, f}, v) +# Checked division functions +for divfn in [:div, :fld, :cld] + @eval function Base.$(Symbol("checked_$divfn"))(x::FD{T,f}, y::FD{T,f}) where {T<:Integer,f} + C = coefficient(FD{T, f}) + # Note: The div() will already throw for divide-by-zero and typemin(T) ÷ -1. + v, b = Base.Checked.mul_with_overflow(C, $divfn(x.i, y.i)) + b && _throw_overflowerr_op($(QuoteNode(divfn)), x, y) + return reinterpret(FD{T, f}, v) + end +end +for remfn in [:rem, :mod] + # rem and mod already check for divide-by-zero and typemin(T) ÷ -1, so nothing to do. + @eval Base.$(Symbol("checked_$remfn"))(x::T, y::T) where {T <: FD} = $remfn(x, y) +end + +_throw_overflowerr_op(op, x::T, y::T) where T = throw(OverflowError("$op($x, $y) overflowed for type $T")) + +function Base.checked_neg(x::T) where {T<:FD} + r = -x + (x<0) & (r<0) && Base.Checked.throw_overflowerr_negation(x) + return r +end +function Base.checked_abs(x::FD) + r = ifelse(x<0, -x, x) + r<0 || return r + msg = LazyString("checked arithmetic: cannot compute |x| for x = ", x, "::", typeof(x)) + throw(OverflowError(msg)) end # We introduce a new function for this since Base.Checked only supports integers, and ints diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index a1f512d..12009fc 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -624,11 +624,17 @@ end @test FD{Int8,1}(2) / Int8(20) == FD{Int8,1}(0.1) end - @testset "limits" begin + @testset "limits: overflow" begin @test_throws OverflowError Base.checked_add(FD{Int8,2}(1), FD{Int8,2}(1)) + @test_throws OverflowError Base.checked_add(FD{Int8,2}(1), 1) @test_throws OverflowError Base.checked_add(FD{Int8,2}(1), FD{Int8,2}(0.4)) - @test_throws OverflowError Base.checked_div(Int8(1), FD{Int8,2}(0.5)) + @test_throws OverflowError Base.checked_sub(FD{Int8,2}(1), FD{Int8,2}(-1)) + @test_throws OverflowError Base.checked_sub(1, FD{Int8,2}(-1)) + @test_throws OverflowError Base.checked_sub(FD{Int8,2}(1), FD{Int8,2}(0.4)) + + @test_throws OverflowError Base.checked_div(FD{Int8,2}(1), FD{Int8,2}(0.5)) + @test_throws OverflowError Base.checked_div(1, FD{Int8,2}(0.5)) @test_throws OverflowError Base.checked_div(FD{Int8,2}(1), FD{Int8,2}(0.4)) @testset "checked_decimal_division" begin @@ -637,6 +643,19 @@ end @test checked_decimal_division(Int8(1), FD{Int8,2}(0.8)) == FD{Int8,2}(1.25) @test_throws OverflowError checked_decimal_division(Int8(1), FD{Int8,2}(0.7)) end + + @test_throws OverflowError Base.checked_fld(FD{Int8,2}(-1), FD{Int8,2}(0.9)) + @test_throws OverflowError Base.checked_cld(FD{Int8,2}(-1), FD{Int8,2}(1.1)) + + # Rem and Mod only throw DivideError and nothing more. They can't overflow, since + # they can only return smaller values than the arguments. + @test_throws DivideError Base.checked_rem(FD{Int8,2}(-1), FD{Int8,2}(0)) + @test_throws DivideError Base.checked_mod(FD{Int8,2}(-1), FD{Int8,2}(0)) + + @test_throws OverflowError Base.checked_abs(typemin(FD{Int8,2})) + @test_throws OverflowError Base.checked_neg(typemin(FD{Int8,2})) + @test Base.checked_abs(typemax(FD{Int8,2})) == FD{Int8,2}(1.27) + @test Base.checked_neg(typemax(FD{Int8,2})) == FD{Int8,2}(-1.27) end @testset "limits of $T" for T in CONTAINER_TYPES From e134ee4bf8611a0fa375b5f291804e39ad240ab3 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Fri, 8 Dec 2023 17:05:16 -0700 Subject: [PATCH 07/21] Fix up tests --- test/FixedDecimal.jl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index 12009fc..fb1341a 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -539,7 +539,7 @@ end # signed integers using two's complement have one additional negative value if x < 0 && x == typemin(x) - @test_throws InexactError x / -one(x) + @test x / -one(x) == x # -typemin(x) == typemin(x) else @test x / -one(x) == -x end @@ -631,7 +631,7 @@ end @test_throws OverflowError Base.checked_sub(FD{Int8,2}(1), FD{Int8,2}(-1)) @test_throws OverflowError Base.checked_sub(1, FD{Int8,2}(-1)) - @test_throws OverflowError Base.checked_sub(FD{Int8,2}(1), FD{Int8,2}(0.4)) + @test_throws OverflowError Base.checked_sub(FD{Int8,2}(-1), FD{Int8,2}(0.4)) @test_throws OverflowError Base.checked_div(FD{Int8,2}(1), FD{Int8,2}(0.5)) @test_throws OverflowError Base.checked_div(1, FD{Int8,2}(0.5)) @@ -644,8 +644,10 @@ end @test_throws OverflowError checked_decimal_division(Int8(1), FD{Int8,2}(0.7)) end + # Rounds down to 2 @test_throws OverflowError Base.checked_fld(FD{Int8,2}(-1), FD{Int8,2}(0.9)) - @test_throws OverflowError Base.checked_cld(FD{Int8,2}(-1), FD{Int8,2}(1.1)) + # Rounds up to 2 + @test_throws OverflowError Base.checked_cld(FD{Int8,2}(1), FD{Int8,2}(0.9)) # Rem and Mod only throw DivideError and nothing more. They can't overflow, since # they can only return smaller values than the arguments. From 2efb4685b846ce5f89930df3112280b2ce46955f Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Fri, 8 Dec 2023 17:46:21 -0700 Subject: [PATCH 08/21] Add corner cases tests --- test/FixedDecimal.jl | 59 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index fb1341a..1b9bd72 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -625,6 +625,7 @@ end end @testset "limits: overflow" begin + # Easy to reason about cases of overflow: @test_throws OverflowError Base.checked_add(FD{Int8,2}(1), FD{Int8,2}(1)) @test_throws OverflowError Base.checked_add(FD{Int8,2}(1), 1) @test_throws OverflowError Base.checked_add(FD{Int8,2}(1), FD{Int8,2}(0.4)) @@ -633,6 +634,11 @@ end @test_throws OverflowError Base.checked_sub(1, FD{Int8,2}(-1)) @test_throws OverflowError Base.checked_sub(FD{Int8,2}(-1), FD{Int8,2}(0.4)) + @test_throws OverflowError Base.checked_mul(FD{Int8,2}(1.2), FD{Int8,2}(1.2)) + @test_throws OverflowError Base.checked_mul(FD{Int8,1}(12), 2) + @test_throws OverflowError Base.checked_mul(FD{Int8,0}(120), 2) + @test_throws OverflowError Base.checked_mul(120, FD{Int8,0}(2)) + @test_throws OverflowError Base.checked_div(FD{Int8,2}(1), FD{Int8,2}(0.5)) @test_throws OverflowError Base.checked_div(1, FD{Int8,2}(0.5)) @test_throws OverflowError Base.checked_div(FD{Int8,2}(1), FD{Int8,2}(0.4)) @@ -644,7 +650,7 @@ end @test_throws OverflowError checked_decimal_division(Int8(1), FD{Int8,2}(0.7)) end - # Rounds down to 2 + # Rounds down to -2 @test_throws OverflowError Base.checked_fld(FD{Int8,2}(-1), FD{Int8,2}(0.9)) # Rounds up to 2 @test_throws OverflowError Base.checked_cld(FD{Int8,2}(1), FD{Int8,2}(0.9)) @@ -658,6 +664,57 @@ end @test_throws OverflowError Base.checked_neg(typemin(FD{Int8,2})) @test Base.checked_abs(typemax(FD{Int8,2})) == FD{Int8,2}(1.27) @test Base.checked_neg(typemax(FD{Int8,2})) == FD{Int8,2}(-1.27) + + @testset "Overflow corner cases" begin + @testset for I in (Int128, UInt128, Int8, UInt8), f in (0,2) + T = FD{I, f} + issigned(I) = signed(I) === I + + @test_throws OverflowError Base.checked_add(typemax(T), eps(T)) + issigned(I) && @test_throws OverflowError Base.checked_add(typemin(T), -eps(T)) + @test_throws OverflowError Base.checked_add(typemax(T), 1) + @test_throws OverflowError Base.checked_add(1, typemax(T)) + + @test_throws OverflowError Base.checked_sub(typemin(T), eps(T)) + issigned(I) && @test_throws OverflowError Base.checked_sub(typemax(T), -eps(T)) + @test_throws OverflowError Base.checked_sub(typemin(T), 1) + if issigned(I) && 2.0 <= typemax(T) + @test_throws OverflowError Base.checked_sub(-2, typemax(T)) + end + + @test_throws OverflowError Base.checked_mul(typemax(T), typemax(T)) + issigned(I) && @test_throws OverflowError Base.checked_mul(typemin(T), typemax(T)) + if 2.0 <= typemax(T) + @test_throws OverflowError Base.checked_mul(typemax(T), 2) + @test_throws OverflowError Base.checked_mul(2, typemax(T)) + issigned(I) && @test_throws OverflowError Base.checked_mul(typemin(T), 2) + issigned(I) && @test_throws OverflowError Base.checked_mul(2, typemin(T)) + end + + if f > 0 + @test_throws OverflowError Base.checked_div(typemax(T), eps(T)) + issigned(I) && @test_throws OverflowError Base.checked_div(typemin(T), eps(T)) + issigned(I) && @test_throws OverflowError Base.checked_div(typemax(T), -eps(T)) + + issigned(I) && @test_throws DivideError Base.checked_div(typemax(T), T(0)) + issigned(I) && @test_throws DivideError Base.checked_div(typemin(T), T(0)) + issigned(I) && @test_throws DivideError Base.checked_div(typemin(T), -eps(T)) + end + + if f > 0 + @test_throws OverflowError Base.checked_fld(typemax(T), eps(T)) + issigned(I) && @test_throws OverflowError Base.checked_fld(typemin(T), eps(T)) + issigned(I) && @test_throws OverflowError Base.checked_fld(typemax(T), -eps(T)) + + @test_throws OverflowError Base.checked_cld(typemax(T), eps(T)) + issigned(I) && @test_throws OverflowError Base.checked_cld(typemin(T), eps(T)) + issigned(I) && @test_throws OverflowError Base.checked_cld(typemax(T), -eps(T)) + end + + issigned(I) && @test_throws OverflowError Base.checked_abs(typemin(T)) + issigned(I) && @test_throws OverflowError Base.checked_neg(typemin(T)) + end + end end @testset "limits of $T" for T in CONTAINER_TYPES From 7793ffc3c5c100f2eb0e854fbf1cec59239821b7 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Fri, 8 Dec 2023 17:52:39 -0700 Subject: [PATCH 09/21] Fix OverflowError for older versions of julia --- src/FixedPointDecimals.jl | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index a84eb15..662d9eb 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -391,15 +391,15 @@ Base.checked_fld(x::FD, y::FD) = Base.checked_fld(promote(x, y)...) Base.checked_rem(x::FD, y::FD) = Base.checked_rem(promote(x, y)...) Base.checked_mod(x::FD, y::FD) = Base.checked_mod(promote(x, y)...) -Base.checked_add(x, y::FD) = Base.checked_add(promote(x, y)...) Base.checked_add(x::FD, y) = Base.checked_add(promote(x, y)...) -Base.checked_sub(x, y::FD) = Base.checked_sub(promote(x, y)...) +Base.checked_add(x, y::FD) = Base.checked_add(promote(x, y)...) Base.checked_sub(x::FD, y) = Base.checked_sub(promote(x, y)...) -Base.checked_mul(x, y::FD) = Base.checked_mul(promote(x, y)...) +Base.checked_sub(x, y::FD) = Base.checked_sub(promote(x, y)...) Base.checked_mul(x::FD, y) = Base.checked_mul(promote(x, y)...) -Base.checked_div(x, y::FD) = Base.checked_div(promote(x, y)...) +Base.checked_mul(x, y::FD) = Base.checked_mul(promote(x, y)...) Base.checked_div(x::FD, y) = Base.checked_div(promote(x, y)...) -Base.checked_cld(x, y::FD) = Base.checked_cld(promote(x, y)...) +Base.checked_div(x, y::FD) = Base.checked_div(promote(x, y)...) +Base.checked_cld(x::FD, y) = Base.checked_cld(promote(x, y)...) Base.checked_cld(x, y::FD) = Base.checked_cld(promote(x, y)...) Base.checked_fld(x::FD, y) = Base.checked_fld(promote(x, y)...) Base.checked_fld(x, y::FD) = Base.checked_fld(promote(x, y)...) @@ -450,8 +450,14 @@ end function Base.checked_abs(x::FD) r = ifelse(x<0, -x, x) r<0 || return r - msg = LazyString("checked arithmetic: cannot compute |x| for x = ", x, "::", typeof(x)) - throw(OverflowError(msg)) + _throw_overflow_abs(x) +end +if VERSION >= v"1.8.0-" + @noinline _throw_overflow_abs(x) = + throw(OverflowError(LazyString("checked arithmetic: cannot compute |x| for x = ", x, "::", typeof(x)))) +else + @noinline _throw_overflow_abs(x) = + throw(OverflowError("checked arithmetic: cannot compute |x| for x = $x")) end # We introduce a new function for this since Base.Checked only supports integers, and ints From 30aef4fc00146ad1cc272f6f9a2440f99aba0659 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Fri, 8 Dec 2023 18:07:04 -0700 Subject: [PATCH 10/21] Add fld/cld tests, but they still seem wrong, and abs/neg --- test/FixedDecimal.jl | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index 1b9bd72..b0b0096 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -817,6 +817,7 @@ end end @testset "division" begin + # TODO(PR): Is this the expected value? @test typemax(FD2) / FD2(0.5) == FD2(-0.02) @test typemin(FD2) / FD2(0.5) == FD2(0) end @@ -828,6 +829,25 @@ end @test typemax(FD2) ÷ eps(FD2) == FD2(-1) @test typemin(FD2) ÷ eps(FD2) == FD2(0) end + + @testset "fld / cld" begin + # TODO(PR): Is this the expected value? + @test fld(typemax(FD2), FD2(0.5)) == FD2(-0.16) + @test fld(typemin(FD2), FD2(0.5)) == FD2(-0.84) + @test fld(typemax(FD2), eps(FD2)) == FD2(-1) + @test fld(typemin(FD2), eps(FD2)) == FD2(0) + + # TODO(PR): Is this the expected value? + @test cld(typemax(FD2), FD2(0.5)) == FD2(0.84) + @test cld(typemin(FD2), FD2(0.5)) == FD2(0.16) + @test cld(typemax(FD2), eps(FD2)) == FD2(-1) + @test cld(typemin(FD2), eps(FD2)) == FD2(0) + end + + @testset "abs / neg" begin + @test abs(typemin(FD2)) == typemin(FD2) + @test -(typemin(FD2)) == typemin(FD2) + end end @testset "isinteger" begin From 07bf40fea667baca60ad5d1312fd99f97f70930e Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Fri, 8 Dec 2023 18:20:37 -0700 Subject: [PATCH 11/21] Change overflow tests to FD{Int8,1} to make the results easier to think about --- test/FixedDecimal.jl | 45 ++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index b0b0096..08309eb 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -801,52 +801,53 @@ end end @testset "overflow" begin + T = FD{Int8, 1} @testset "addition" begin - @test typemax(FD2) + eps(FD2) == typemin(FD2) - @test typemin(FD2) + (-eps(FD2)) == typemax(FD2) + @test typemax(T) + eps(T) == typemin(T) + @test typemin(T) + (-eps(T)) == typemax(T) end @testset "subtraction" begin - @test typemin(FD2) - eps(FD2) == typemax(FD2) - @test typemax(FD2) - (-eps(FD2)) == typemin(FD2) + @test typemin(T) - eps(T) == typemax(T) + @test typemax(T) - (-eps(T)) == typemin(T) end @testset "multiplication" begin - @test typemax(FD2) * 2 == FD2(-0.02) - @test typemin(FD2) * 2 == FD2(0) + @test typemax(T) * 2 == T(-0.2) + @test typemin(T) * 2 == T(0) end @testset "division" begin # TODO(PR): Is this the expected value? - @test typemax(FD2) / FD2(0.5) == FD2(-0.02) - @test typemin(FD2) / FD2(0.5) == FD2(0) + @test typemax(T) / T(0.5) == FD2(-0.2) + @test typemin(T) / T(0.5) == FD2(0) end @testset "truncating division" begin # TODO(PR): Is this the expected value? - @test typemax(FD2) ÷ FD2(0.5) == FD2(-0.16) - @test typemin(FD2) ÷ FD2(0.5) == FD2(0.16) - @test typemax(FD2) ÷ eps(FD2) == FD2(-1) - @test typemin(FD2) ÷ eps(FD2) == FD2(0) + @test typemax(T) ÷ T(0.5) == T(-0.6) + @test typemin(T) ÷ T(0.5) == T(0.6) + @test typemax(T) ÷ eps(T) == T(-1) + @test typemin(T) ÷ eps(T) == T(0) end @testset "fld / cld" begin # TODO(PR): Is this the expected value? - @test fld(typemax(FD2), FD2(0.5)) == FD2(-0.16) - @test fld(typemin(FD2), FD2(0.5)) == FD2(-0.84) - @test fld(typemax(FD2), eps(FD2)) == FD2(-1) - @test fld(typemin(FD2), eps(FD2)) == FD2(0) + @test fld(typemax(T), T(0.5)) == T(-0.6) + @test fld(typemin(T), T(0.5)) == T(-0.4) + @test fld(typemax(T), eps(T)) == T(-1) + @test fld(typemin(T), eps(T)) == T(0) # TODO(PR): Is this the expected value? - @test cld(typemax(FD2), FD2(0.5)) == FD2(0.84) - @test cld(typemin(FD2), FD2(0.5)) == FD2(0.16) - @test cld(typemax(FD2), eps(FD2)) == FD2(-1) - @test cld(typemin(FD2), eps(FD2)) == FD2(0) + @test cld(typemax(T), T(0.5)) == T(0.4) + @test cld(typemin(T), T(0.5)) == T(0.6) + @test cld(typemax(T), eps(T)) == T(-1) + @test cld(typemin(T), eps(T)) == T(0) end @testset "abs / neg" begin - @test abs(typemin(FD2)) == typemin(FD2) - @test -(typemin(FD2)) == typemin(FD2) + @test abs(typemin(T)) == typemin(T) + @test -(typemin(T)) == typemin(T) end end From 137c8b4d8a603b32278b6e9af6d06565f890ba43 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Fri, 8 Dec 2023 18:33:30 -0700 Subject: [PATCH 12/21] Add promotions tests --- test/FixedDecimal.jl | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index 08309eb..1da8f74 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -624,7 +624,7 @@ end @test FD{Int8,1}(2) / Int8(20) == FD{Int8,1}(0.1) end - @testset "limits: overflow" begin + @testset "limits: overflow checked math" begin # Easy to reason about cases of overflow: @test_throws OverflowError Base.checked_add(FD{Int8,2}(1), FD{Int8,2}(1)) @test_throws OverflowError Base.checked_add(FD{Int8,2}(1), 1) @@ -665,7 +665,7 @@ end @test Base.checked_abs(typemax(FD{Int8,2})) == FD{Int8,2}(1.27) @test Base.checked_neg(typemax(FD{Int8,2})) == FD{Int8,2}(-1.27) - @testset "Overflow corner cases" begin + @testset "checked math corner cases" begin @testset for I in (Int128, UInt128, Int8, UInt8), f in (0,2) T = FD{I, f} issigned(I) = signed(I) === I @@ -715,6 +715,22 @@ end issigned(I) && @test_throws OverflowError Base.checked_neg(typemin(T)) end end + + @testset "checked math promotions" begin + x = FD{Int8,1}(1) + y = FD{Int64,1}(2) + @testset for op in ( + Base.checked_add, Base.checked_sub, Base.checked_mul, Base.checked_div, + Base.checked_cld, Base.checked_fld, Base.checked_rem, Base.checked_mod, + FixedPointDecimals.checked_decimal_division, + ) + @test op(x, y) === op(FD{Int64,1}(1), y) + @test op(y, x) === op(y, FD{Int64,1}(1)) + + @test op(x, 2) === op(x, FD{Int8,1}(2)) + @test op(2, x) === op(FD{Int8,1}(2), x) + end + end end @testset "limits of $T" for T in CONTAINER_TYPES From e1dd56d8a89df65532bbb3eb36e821209a7d6c32 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Mon, 18 Dec 2023 16:37:41 -0700 Subject: [PATCH 13/21] Rename checked_rdiv; reexport checked* --- src/FixedPointDecimals.jl | 17 ++++++++++++----- test/FixedDecimal.jl | 10 +++++----- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index 662d9eb..f1a3447 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -27,6 +27,13 @@ module FixedPointDecimals export FixedDecimal, RoundThrows +# (Re)export checked_* arithmetic functions +# - Defined in this package: +export checked_rdiv +# - Reexported from Base: +export checked_abs, checked_add, checked_cld, checked_div, checked_fld, + checked_mod, checked_mul, checked_neg, checked_rem, checked_sub + using Base: decompose, BitInteger import Parsers @@ -463,7 +470,7 @@ end # We introduce a new function for this since Base.Checked only supports integers, and ints # don't have a decimal division operation. """ - FixedPointDecimals.checked_decimal_division(x::FD, y::FD) -> FD + FixedPointDecimals.checked_rdiv(x::FD, y::FD) -> FD Calculates `x / y`, checking for overflow errors where applicable. @@ -472,11 +479,11 @@ The overflow protection may impose a perceptible performance penalty. See also: - `Base.checked_div` for truncating division. """ -checked_decimal_division(x::FD, y::FD) = checked_decimal_division(promote(x, y)...) -checked_decimal_division(x, y::FD) = checked_decimal_division(promote(x, y)...) -checked_decimal_division(x::FD, y) = checked_decimal_division(promote(x, y)...) +checked_rdiv(x::FD, y::FD) = checked_rdiv(promote(x, y)...) +checked_rdiv(x, y::FD) = checked_rdiv(promote(x, y)...) +checked_rdiv(x::FD, y) = checked_rdiv(promote(x, y)...) -function checked_decimal_division(x::FD{T,f}, y::FD{T,f}) where {T<:Integer,f} +function checked_rdiv(x::FD{T,f}, y::FD{T,f}) where {T<:Integer,f} powt = coefficient(FD{T, f}) quotient, remainder = fldmod(widemul(x.i, powt), y.i) v = _round_to_nearest(quotient, remainder, y.i) diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index 1da8f74..0ac523e 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -643,11 +643,11 @@ end @test_throws OverflowError Base.checked_div(1, FD{Int8,2}(0.5)) @test_throws OverflowError Base.checked_div(FD{Int8,2}(1), FD{Int8,2}(0.4)) - @testset "checked_decimal_division" begin - using FixedPointDecimals: checked_decimal_division + @testset "checked_rdiv" begin + using FixedPointDecimals: checked_rdiv - @test checked_decimal_division(Int8(1), FD{Int8,2}(0.8)) == FD{Int8,2}(1.25) - @test_throws OverflowError checked_decimal_division(Int8(1), FD{Int8,2}(0.7)) + @test checked_rdiv(Int8(1), FD{Int8,2}(0.8)) == FD{Int8,2}(1.25) + @test_throws OverflowError checked_rdiv(Int8(1), FD{Int8,2}(0.7)) end # Rounds down to -2 @@ -722,7 +722,7 @@ end @testset for op in ( Base.checked_add, Base.checked_sub, Base.checked_mul, Base.checked_div, Base.checked_cld, Base.checked_fld, Base.checked_rem, Base.checked_mod, - FixedPointDecimals.checked_decimal_division, + FixedPointDecimals.checked_rdiv, ) @test op(x, y) === op(FD{Int64,1}(1), y) @test op(y, x) === op(y, FD{Int64,1}(1)) From d119dce81d2a33749dd50877f11b838f09b2aea6 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Mon, 18 Dec 2023 16:39:13 -0700 Subject: [PATCH 14/21] Enable nightly CI --- .github/workflows/CI.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index f1e3890..275c5ef 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -15,7 +15,7 @@ jobs: version: - '1.6' - '1' -# - 'nightly' + - 'nightly' os: - ubuntu-latest - macOS-latest From 41a69fd3b34701b739f266a2567163c68c6d78c4 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Mon, 18 Dec 2023 16:55:42 -0700 Subject: [PATCH 15/21] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tomáš Drvoštěp --- src/FixedPointDecimals.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index f1a3447..72f7c71 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -447,7 +447,7 @@ for remfn in [:rem, :mod] @eval Base.$(Symbol("checked_$remfn"))(x::T, y::T) where {T <: FD} = $remfn(x, y) end -_throw_overflowerr_op(op, x::T, y::T) where T = throw(OverflowError("$op($x, $y) overflowed for type $T")) +@noinline _throw_overflowerr_op(op, x::T, y::T) where T = throw(OverflowError("$op($x, $y) overflowed for type $T")) function Base.checked_neg(x::T) where {T<:FD} r = -x From 3270ad56cdbeb14d865bbaa0627f1d19a2aa1834 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Mon, 18 Dec 2023 16:56:23 -0700 Subject: [PATCH 16/21] Improve perf: don't force-promote inside *() This allows BigInt * Int to avoid allocating another BigInt. --- src/FixedPointDecimals.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index 72f7c71..4284365 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -194,8 +194,8 @@ end # these functions are needed to avoid InexactError when converting from the # integer type -Base.:*(x::Integer, y::FD{T, f}) where {T, f} = reinterpret(FD{T, f}, *(promote(x, y.i)...)) -Base.:*(x::FD{T, f}, y::Integer) where {T, f} = reinterpret(FD{T, f}, *(promote(x.i, y)...)) +Base.:*(x::Integer, y::FD{T, f}) where {T, f} = reinterpret(FD{T, f}, *(x, y.i)) +Base.:*(x::FD{T, f}, y::Integer) where {T, f} = reinterpret(FD{T, f}, *(x.i, y)) function Base.:/(x::FD{T, f}, y::FD{T, f}) where {T, f} powt = coefficient(FD{T, f}) From 4cb2d5def4a6a563667020a17957d80607490e84 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Mon, 18 Dec 2023 17:17:35 -0700 Subject: [PATCH 17/21] Make division operations always throw on overflow! Also update the README --- README.md | 8 +++-- src/FixedPointDecimals.jl | 66 ++++++++++++++++++++------------------- test/FixedDecimal.jl | 30 +++++++++--------- 3 files changed, 54 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index c3449a7..4342adc 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ FixedDecimal{Int64,1}(0.3) ### Arithmetic details: Overflow and checked math -By default, all arithmetic operations on FixedDecimals **will silently overflow**, following the standard behavior for bit integer types in Julia. For example: +By default, all arithmetic operations on FixedDecimals, except division, **will silently overflow** following the standard behavior for bit integer types in Julia. For example: ```julia julia> FixedDecimal{Int8,2}(1.0) + FixedDecimal{Int8,2}(1.0) FixedDecimal{Int8,2}(-0.56) @@ -59,6 +59,8 @@ julia> abs(FixedDecimal{Int8,2}(-1.28)) # negative typemin wraps to typemin aga FixedDecimal{Int8,2}(-1.28) ``` +*Note that **division** on FixedDecimals will throw OverflowErrors on overflow, and will not wrap. This decision may be reevaluated in a future breaking version change release of FixedDecimals. Please keep this in mind.* + In most applications dealing with `FixedDecimals`, you will likely want to use the **checked arithmetic** operations instead. These operations will _throw an OverflowError_ on overflow or underflow, rather than silently wrapping. For example: ```julia julia> Base.checked_mul(FixedDecimal{Int8,2}(1.2), FixedDecimal{Int8,2}(1.2)) @@ -71,14 +73,14 @@ julia> Base.checked_div(Int8(1), FixedDecimal{Int8,2}(0.5)) ERROR: OverflowError: 1.00 ÷ 0.50 overflowed for type FixedDecimal{Int8, 2} ``` -**Checked division:** Note that `checked_div` performs truncating, integer division. Julia Base does not provide a function to perform checked decimal division, so we provide one in this package, `FixedPointDecimals.checked_decimal_division`. +**Checked division:** Note that `checked_div` performs truncating, integer division. Julia Base does not provide a function to perform checked *decimal* division (`/`), so we provide one in this package, `FixedPointDecimals.checked_rdiv`. However, as noted above, the default division arithmetic operators will throw on overflow anyway. Here are all the checked arithmetic operations supported by `FixedDecimal`s: - `Base.checked_add(x,y)` - `Base.checked_sub(x,y)` - `Base.checked_mul(x,y)` - `Base.checked_div(x,y)` -- `FixedPointDecimals.checked_decimal_division(x,y)` +- `FixedPointDecimals.checked_rdiv(x,y)` - `Base.checked_cld(x,y)` - `Base.checked_fld(x,y)` - `Base.checked_rem(x,y)` diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index 4284365..e4a9af6 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -197,25 +197,9 @@ end Base.:*(x::Integer, y::FD{T, f}) where {T, f} = reinterpret(FD{T, f}, *(x, y.i)) Base.:*(x::FD{T, f}, y::Integer) where {T, f} = reinterpret(FD{T, f}, *(x.i, y)) -function Base.:/(x::FD{T, f}, y::FD{T, f}) where {T, f} - powt = coefficient(FD{T, f}) - quotient, remainder = fldmod(widemul(x.i, powt), y.i) - reinterpret(FD{T, f}, _round_to_nearest(quotient, remainder, y.i)) -end - -# These functions allow us to perform division with integers outside of the range of the -# FixedDecimal. -function Base.:/(x::Integer, y::FD{T, f}) where {T, f} - powt = coefficient(FD{T, f}) - powtsq = widemul(powt, powt) - quotient, remainder = fldmod(widemul(x, powtsq), y.i) - reinterpret(FD{T, f}, _round_to_nearest(quotient, remainder, y.i)) -end - -function Base.:/(x::FD{T, f}, y::Integer) where {T, f} - quotient, remainder = fldmod(x.i, y) - reinterpret(FD{T, f}, _round_to_nearest(quotient, remainder, y)) -end +Base.:/(x::FD, y::FD) = checked_rdiv(x, y) +Base.:/(x::Integer, y::FD) = checked_rdiv(x, y) +Base.:/(x::FD, y::Integer) = checked_rdiv(x, y) # integerification Base.trunc(x::FD{T, f}) where {T, f} = FD{T, f}(div(x.i, coefficient(FD{T, f}))) @@ -366,17 +350,19 @@ for remfn in [:rem, :mod, :mod1, :min, :max] end # TODO: When we upgrade to a min julia version >=1.4 (i.e Julia 2.0), this block can be # dropped in favor of three-argument `div`, below. -for divfn in [:div, :fld, :fld1, :cld] - # div(x.i, y.i) eliminates the scaling coefficient, so we call the FD constructor. - # We don't need any widening logic, since we won't be multiplying by the coefficient. - #@eval Base.$divfn(x::T, y::T) where {T <: FD} = T($divfn(x.i, y.i)) - # @eval Base.$divfn(x::T, y::T) where {T <: FD} = $divfn(promote(x.i, y.i)...) - # TODO(PR): I'm not sure about this one... - # What should it *mean* for `typemax(FD) ÷ FD(0.5)` to overflow? - @eval function Base.$divfn(x::T, y::T) where {T <: FD} - C = coefficient(T) - return reinterpret(T, C * $divfn(promote(x.i, y.i)...)) - end +# The division functions all default to *throwing OverflowError* rather than +# wrapping on integer overflow. +# This decision may be changed in a future release of FixedPointDecimals. +Base.div(x::FD, y::FD) = Base.checked_div(x, y) +Base.fld(x::FD, y::FD) = Base.checked_fld(x, y) +Base.cld(x::FD, y::FD) = Base.checked_cld(x, y) +# There is not checked_fld1, so this is implemented here: +function Base.fld1(x::FD{T,f}, y::FD{T,f}) where {T, f} + C = coefficient(FD{T, f}) + # Note: fld1() will already throw for divide-by-zero and typemin(T) ÷ -1. + v, b = Base.Checked.mul_with_overflow(C, fld1(x.i, y.i)) + b && _throw_overflowerr_op(:fld1, x, y) + return reinterpret(FD{T, f}, v) end if VERSION >= v"1.4.0-" # div(x.i, y.i) eliminates the scaling coefficient, so we call the FD constructor. @@ -480,8 +466,6 @@ See also: - `Base.checked_div` for truncating division. """ checked_rdiv(x::FD, y::FD) = checked_rdiv(promote(x, y)...) -checked_rdiv(x, y::FD) = checked_rdiv(promote(x, y)...) -checked_rdiv(x::FD, y) = checked_rdiv(promote(x, y)...) function checked_rdiv(x::FD{T,f}, y::FD{T,f}) where {T<:Integer,f} powt = coefficient(FD{T, f}) @@ -491,6 +475,24 @@ function checked_rdiv(x::FD{T,f}, y::FD{T,f}) where {T<:Integer,f} return reinterpret(FD{T, f}, v) end +# These functions allow us to perform division with integers outside of the range of the +# FixedDecimal. +function checked_rdiv(x::Integer, y::FD{T, f}) where {T<:Integer, f} + powt = coefficient(FD{T, f}) + powtsq = widemul(powt, powt) + quotient, remainder = fldmod(widemul(x, powtsq), y.i) + v = _round_to_nearest(quotient, remainder, y.i) + typemin(T) <= v <= typemax(T) || Base.Checked.throw_overflowerr_binaryop(:/, x, y) + reinterpret(FD{T, f}, v) +end +function checked_rdiv(x::FD{T, f}, y::Integer) where {T<:Integer, f} + quotient, remainder = fldmod(x.i, y) + v = _round_to_nearest(quotient, remainder, y) + typemin(T) <= v <= typemax(T) || Base.Checked.throw_overflowerr_binaryop(:/, x, y) + reinterpret(FD{T, f}, v) +end + + # -------------------------- Base.convert(::Type{AbstractFloat}, x::FD) = convert(floattype(typeof(x)), x) diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index 0ac523e..6fb2f91 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -539,7 +539,7 @@ end # signed integers using two's complement have one additional negative value if x < 0 && x == typemin(x) - @test x / -one(x) == x # -typemin(x) == typemin(x) + @test_throws OverflowError x / -one(x) else @test x / -one(x) == -x end @@ -835,30 +835,30 @@ end @testset "division" begin # TODO(PR): Is this the expected value? - @test typemax(T) / T(0.5) == FD2(-0.2) - @test typemin(T) / T(0.5) == FD2(0) + @test_throws OverflowError typemax(T) / T(0.5) + @test_throws OverflowError typemin(T) / T(0.5) end @testset "truncating division" begin # TODO(PR): Is this the expected value? - @test typemax(T) ÷ T(0.5) == T(-0.6) - @test typemin(T) ÷ T(0.5) == T(0.6) - @test typemax(T) ÷ eps(T) == T(-1) - @test typemin(T) ÷ eps(T) == T(0) + @test_throws OverflowError typemax(T) ÷ T(0.5) + @test_throws OverflowError typemin(T) ÷ T(0.5) + @test_throws OverflowError typemax(T) ÷ eps(T) + @test_throws OverflowError typemin(T) ÷ eps(T) end @testset "fld / cld" begin # TODO(PR): Is this the expected value? - @test fld(typemax(T), T(0.5)) == T(-0.6) - @test fld(typemin(T), T(0.5)) == T(-0.4) - @test fld(typemax(T), eps(T)) == T(-1) - @test fld(typemin(T), eps(T)) == T(0) + @test_throws OverflowError fld(typemax(T), T(0.5)) + @test_throws OverflowError fld(typemin(T), T(0.5)) + @test_throws OverflowError fld(typemax(T), eps(T)) + @test_throws OverflowError fld(typemin(T), eps(T)) # TODO(PR): Is this the expected value? - @test cld(typemax(T), T(0.5)) == T(0.4) - @test cld(typemin(T), T(0.5)) == T(0.6) - @test cld(typemax(T), eps(T)) == T(-1) - @test cld(typemin(T), eps(T)) == T(0) + @test_throws OverflowError cld(typemax(T), T(0.5)) + @test_throws OverflowError cld(typemin(T), T(0.5)) + @test_throws OverflowError cld(typemax(T), eps(T)) + @test_throws OverflowError cld(typemin(T), eps(T)) end @testset "abs / neg" begin From 0cf90926003ac4bd767aec026855d99d57de973e Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Mon, 18 Dec 2023 17:22:59 -0700 Subject: [PATCH 18/21] Add overflow tests for fld1 and div(::RoundMode) --- src/FixedPointDecimals.jl | 9 ++++++--- test/FixedDecimal.jl | 14 ++++++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index e4a9af6..bd65256 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -367,9 +367,12 @@ end if VERSION >= v"1.4.0-" # div(x.i, y.i) eliminates the scaling coefficient, so we call the FD constructor. # We don't need any widening logic, since we won't be multiplying by the coefficient. - @eval function Base.div(x::T, y::T, r::RoundingMode) where {T <: FD} - C = coefficient(T) - return reinterpret(T, C * div(x.i, y.i, r)) + @eval function Base.div(x::FD{T, f}, y::FD{T, f}, r::RoundingMode) where {T<:Integer, f} + C = coefficient(FD{T, f}) + # Note: The div() will already throw for divide-by-zero and typemin(T) ÷ -1. + v, b = Base.Checked.mul_with_overflow(C, div(x.i, y.i, r)) + b && _throw_overflowerr_op(:div, x, y) + return reinterpret(FD{T, f}, v) end end diff --git a/test/FixedDecimal.jl b/test/FixedDecimal.jl index 6fb2f91..0b42cfa 100644 --- a/test/FixedDecimal.jl +++ b/test/FixedDecimal.jl @@ -834,27 +834,33 @@ end end @testset "division" begin - # TODO(PR): Is this the expected value? @test_throws OverflowError typemax(T) / T(0.5) @test_throws OverflowError typemin(T) / T(0.5) end @testset "truncating division" begin - # TODO(PR): Is this the expected value? @test_throws OverflowError typemax(T) ÷ T(0.5) @test_throws OverflowError typemin(T) ÷ T(0.5) @test_throws OverflowError typemax(T) ÷ eps(T) @test_throws OverflowError typemin(T) ÷ eps(T) + + @test_throws OverflowError div(typemax(T), T(0.5), RoundUp) + @test_throws OverflowError div(typemin(T), T(0.5), RoundUp) + @test_throws OverflowError div(typemax(T), eps(T), RoundUp) + @test_throws OverflowError div(typemin(T), eps(T), RoundUp) end @testset "fld / cld" begin - # TODO(PR): Is this the expected value? @test_throws OverflowError fld(typemax(T), T(0.5)) @test_throws OverflowError fld(typemin(T), T(0.5)) @test_throws OverflowError fld(typemax(T), eps(T)) @test_throws OverflowError fld(typemin(T), eps(T)) - # TODO(PR): Is this the expected value? + @test_throws OverflowError fld1(typemax(T), T(0.5)) + @test_throws OverflowError fld1(typemin(T), T(0.5)) + @test_throws OverflowError fld1(typemax(T), eps(T)) + @test_throws OverflowError fld1(typemin(T), eps(T)) + @test_throws OverflowError cld(typemax(T), T(0.5)) @test_throws OverflowError cld(typemin(T), T(0.5)) @test_throws OverflowError cld(typemax(T), eps(T)) From 8eefc80a40a72503519581794915a0d74395e0c5 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Mon, 18 Dec 2023 17:25:29 -0700 Subject: [PATCH 19/21] Cleanup --- src/FixedPointDecimals.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index bd65256..55c6a68 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -194,8 +194,8 @@ end # these functions are needed to avoid InexactError when converting from the # integer type -Base.:*(x::Integer, y::FD{T, f}) where {T, f} = reinterpret(FD{T, f}, *(x, y.i)) -Base.:*(x::FD{T, f}, y::Integer) where {T, f} = reinterpret(FD{T, f}, *(x.i, y)) +Base.:*(x::Integer, y::FD{T, f}) where {T, f} = reinterpret(FD{T, f}, x * y.i) +Base.:*(x::FD{T, f}, y::Integer) where {T, f} = reinterpret(FD{T, f}, x.i * y) Base.:/(x::FD, y::FD) = checked_rdiv(x, y) Base.:/(x::Integer, y::FD) = checked_rdiv(x, y) From 34a930691be2f575ba97121680e4ed5d164c4e5c Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Mon, 18 Dec 2023 17:34:39 -0700 Subject: [PATCH 20/21] This is a breaking release! --- Project.toml | 2 +- src/FixedPointDecimals.jl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Project.toml b/Project.toml index 7046d0b..9ef7f60 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.4.4" +version = "0.5.0" [deps] Parsers = "69de0a69-1ddd-5017-9359-2bf0b02dc9f0" diff --git a/src/FixedPointDecimals.jl b/src/FixedPointDecimals.jl index 55c6a68..89389c4 100644 --- a/src/FixedPointDecimals.jl +++ b/src/FixedPointDecimals.jl @@ -356,7 +356,7 @@ end Base.div(x::FD, y::FD) = Base.checked_div(x, y) Base.fld(x::FD, y::FD) = Base.checked_fld(x, y) Base.cld(x::FD, y::FD) = Base.checked_cld(x, y) -# There is not checked_fld1, so this is implemented here: +# There is no checked_fld1, so this is implemented here: function Base.fld1(x::FD{T,f}, y::FD{T,f}) where {T, f} C = coefficient(FD{T, f}) # Note: fld1() will already throw for divide-by-zero and typemin(T) ÷ -1. From 73410da48e2c8c504efb0fc1e7975688bb993d00 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Mon, 18 Dec 2023 17:35:37 -0700 Subject: [PATCH 21/21] compat to README --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 4342adc..3ca5e2e 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,8 @@ FixedDecimal{Int64,1}(0.3) ### Arithmetic details: Overflow and checked math +_NOTE: This section applies to FixedPointDecimals v0.5+._ + By default, all arithmetic operations on FixedDecimals, except division, **will silently overflow** following the standard behavior for bit integer types in Julia. For example: ```julia julia> FixedDecimal{Int8,2}(1.0) + FixedDecimal{Int8,2}(1.0)