Simplify rationals on addition and substraction (#168)

* Simplify rationals on addition and substraction

* Fix

* Fix

* Fix format

* aqua-lint fix

Author: blegat

src/implementations/BigFloat.jl

@@ -44,7 +44,9 @@ operate!(::typeof(one), x::BigFloat) = _set_si!(x, 1)
 
 # +
 
-promote_operation(::typeof(+), ::Vararg{Type{BigFloat},N}) where {N} = BigFloat
+function promote_operation(::typeof(+), ::Type{BigFloat}, ::Type{BigFloat})
+    return BigFloat
+end
 
 function operate_to!(output::BigFloat, ::typeof(+), a::BigFloat, b::BigFloat)
     ccall(
@@ -78,7 +80,7 @@ end
 
 # *
 
-promote_operation(::typeof(*), ::Vararg{Type{BigFloat},N}) where {N} = BigFloat
+promote_operation(::typeof(*), ::Type{BigFloat}, ::Type{BigFloat}) = BigFloat
 
 function operate_to!(output::BigFloat, ::typeof(*), a::BigFloat, b::BigFloat)
     ccall(

src/implementations/BigInt.jl

@@ -27,7 +27,7 @@ operate!(::typeof(one), x::BigInt) = Base.GMP.MPZ.set_si!(x, 1)
 
 # +
 
-promote_operation(::typeof(+), ::Vararg{Type{BigInt},N}) where {N} = BigInt
+promote_operation(::typeof(+), ::Type{BigInt}, ::Type{BigInt}) = BigInt
 
 function operate_to!(output::BigInt, ::typeof(+), a::BigInt, b::BigInt)
     return Base.GMP.MPZ.add!(output, a, b)
@@ -43,18 +43,25 @@ end
 
 # *
 
-promote_operation(::typeof(*), ::Vararg{Type{BigInt},N}) where {N} = BigInt
+promote_operation(::typeof(*), ::Type{BigInt}, ::Type{BigInt}) = BigInt
 
 function operate_to!(output::BigInt, ::typeof(*), a::BigInt, b::BigInt)
     return Base.GMP.MPZ.mul!(output, a, b)
 end
 
+promote_operation(::typeof(div), ::Type{BigInt}, ::Type{BigInt}) = BigInt
+
+function operate_to!(output::BigInt, ::typeof(div), a::BigInt, b::BigInt)
+    return Base.GMP.MPZ.tdiv_q!(output, a, b)
+end
+
 # gcd
 
 function promote_operation(
     ::Union{typeof(gcd),typeof(lcm)},
-    ::Vararg{Type{BigInt},N},
-) where {N}
+    ::Type{BigInt},
+    ::Type{BigInt},
+)
     return BigInt
 end
 

src/implementations/Rational.jl

@@ -43,12 +43,22 @@ function promote_operation(
     return Rational{promote_sum_mul(S, T)}
 end
 
+function _buffered_simplify(buffer, x::Rational)
+    operate_to!(buffer, gcd, x.num, x.den)
+    if !isone(buffer)
+        operate!(div, x.num, buffer)
+        operate!(div, x.den, buffer)
+    end
+end
+
 function operate_to!(output::Rational, ::typeof(+), x::Rational, y::Rational)
     xd, yd = Base.divgcd(promote(x.den, y.den)...)
     # TODO: Use `checked_mul` and `checked_add` like in Base
     operate_to!(output.num, *, x.num, yd)
     operate!(add_mul, output.num, y.num, xd)
     operate_to!(output.den, *, x.den, yd)
+    # Reuse `xd` as it is a local copy created by this method
+    _buffered_simplify(xd, output)
     return output
 end
 
@@ -68,6 +78,8 @@ function operate_to!(output::Rational, ::typeof(-), x::Rational, y::Rational)
     operate_to!(output.num, *, x.num, yd)
     operate!(sub_mul, output.num, y.num, xd)
     operate_to!(output.den, *, x.den, yd)
+    # Reuse `xd` as it is a local copy created by this method
+    _buffered_simplify(xd, output)
     return output
 end
 

src/interface.jl

@@ -61,13 +61,13 @@ end
 promote_operation_fallback(::typeof(*), ::Type{T}) where {T} = T
 
 function promote_operation_fallback(
-    ::typeof(*),
+    op::Union{typeof(*),typeof(+),typeof(gcd),typeof(lcm)},
     ::Type{S},
     ::Type{T},
     ::Type{U},
     args::Vararg{Type,N},
 ) where {S,T,U,N}
-    return promote_operation(*, promote_operation(*, S, T), U, args...)
+    return promote_operation(op, promote_operation(op, S, T), U, args...)
 end
 
 # `Vararg` gives extra allocations on Julia v1.3, see

src/shortcuts.jl

@@ -57,6 +57,22 @@ Shortcut for `operate(*, a, b, ...)`, see [`operate`](@ref).
 """
 mul(args::Vararg{Any,N}) where {N} = operate(*, args...)
 
+"""
+    div_to!!(output, a, b)
+
+Return `div(a, b)` possibly modifying `output`.
+"""
+function div_to!!(output, a, b)
+    return operate_to!!(output, div, a, b)
+end
+
+"""
+    div!!(a, b)
+
+Return `div(a, b)` possibly modifying `a`.
+"""
+div!!(a, b) = operate!!(div, a, b)
+
 """
     gcd_to!!(a, b, c, ...)
 

test/big.jl

@@ -4,12 +4,22 @@
 # v.2.0. If a copy of the MPL was not distributed with this file, You can obtain
 # one at http://mozilla.org/MPL/2.0/.
 
-function allocation_test(op, T, short, short_to, n)
-    a = T(2)
-    b = T(3)
-    c = T(4)
+function allocation_test(
+    op,
+    T,
+    short,
+    short_to,
+    n;
+    a = T(2),
+    b = T(3),
+    c = T(4),
+)
     @test MA.promote_operation(op, T, T) == T
-    @test MA.promote_operation(op, T, T, T) == T
+    alloc_test(() -> MA.promote_operation(op, T, T), 0)
+    if op != div && op != -
+        @test MA.promote_operation(op, T, T, T) == T
+        alloc_test(() -> MA.promote_operation(op, T, T, T), 0)
+    end
     g = op(a, b)
     @test c === short_to(c, a, b)
     @test g == c
@@ -23,9 +33,35 @@ end
     MA.Test.int_test(T)
     @testset "Allocation" begin
         allocation_test(+, T, MA.add!!, MA.add_to!!, T <: Rational ? 168 : 0)
+        allocation_test(-, T, MA.sub!!, MA.sub_to!!, T <: Rational ? 168 : 0)
         allocation_test(*, T, MA.mul!!, MA.mul_to!!, T <: Rational ? 240 : 0)
+        if T <: Rational # https://github.com/jump-dev/MutableArithmetics.jl/issues/167
+            allocation_test(
+                +,
+                T,
+                MA.add!!,
+                MA.add_to!!,
+                168,
+                a = T(1 // 2),
+                b = T(3 // 2),
+                c = T(5 // 2),
+            )
+            allocation_test(
+                -,
+                T,
+                MA.sub!!,
+                MA.sub_to!!,
+                168,
+                a = T(1 // 2),
+                b = T(3 // 2),
+                c = T(5 // 2),
+            )
+        end
         # Requires https://github.com/JuliaLang/julia/commit/3f92832df042198b2daefc1f7ca609db38cb8173
         # for `gcd` to be defined on `Rational`.
+        if T == BigInt
+            allocation_test(div, T, MA.div!!, MA.div_to!!, 0)
+        end
         if T == BigInt || T == Rational{BigInt}
             allocation_test(gcd, T, MA.gcd!!, MA.gcd_to!!, 0)
             allocation_test(lcm, T, MA.lcm!!, MA.lcm_to!!, 0)