Make division operations always throw on overflow!

Also update the README

Author: NHDaly

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)`

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)

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