Skip to content

Commit

Permalink
Correct mutating op fallbacks (#1823)
Browse files Browse the repository at this point in the history
* Fix docstrings of `inv!` and `div!`

* Move methods from `AA.*` to `Base.*`

the `AA.*` should only implement methods for julia types and delegate everything else
  • Loading branch information
lgoettgens authored Oct 2, 2024
1 parent 3fe0405 commit 55c35d9
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 15 deletions.
2 changes: 1 addition & 1 deletion src/Fields.jl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ is_zero_divisor(a::T) where T <: FieldElem = is_zero(a)

Base.divrem(a::T, b::T) where {T <: FieldElem} = divexact(a, b), zero(parent(a))

div(a::T, b::T) where {T <: FieldElem} = divexact(a, b)
Base.div(a::T, b::T) where {T <: FieldElem} = divexact(a, b)

function gcd(x::T, y::T) where {T <: FieldElem}
check_parent(x, y)
Expand Down
5 changes: 0 additions & 5 deletions src/Poly.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1479,11 +1479,6 @@ function Base.divrem(f::PolyRingElem{T}, g::PolyRingElem{T}) where T <: RingElem
return q, f
end

function Base.div(f::PolyRingElem{T}, g::PolyRingElem{T}) where T <: RingElement
q, r = divrem(f, g)
return q
end

##############################################################################
#
# Ad hoc Euclidean division
Expand Down
27 changes: 24 additions & 3 deletions src/fundamental_interface.jl
Original file line number Diff line number Diff line change
Expand Up @@ -328,11 +328,16 @@ neg!(a) = neg!(a, a)
inv!(z, a)
inv!(a)
Return `inv(a)`, possibly modifying the object `z` in the process.
Return `AbstractAlgebra.inv(a)`, possibly modifying the object `z` in the process.
Aliasing is permitted.
The unary version is a shorthand for `inv!(a, a)`.
!!! note
`AbstractAlgebra.inv` and `Base.inv` differ only in their behavior on julia
types like `Integer` and `Rational{Int}`. The former makes it adhere to the
Ring interface.
"""
inv!(z, a) = inv(a)
inv!(z, a) = AbstractAlgebra.inv(a)
inv!(a) = inv!(a, a)

for (name, op) in ((:add!, :+), (:sub!, :-), (:mul!, :*))
Expand Down Expand Up @@ -378,7 +383,7 @@ submul!(z, a, b) = submul!(z, a, b, parent(z)())

function divexact end

for name in (:divexact, :div, :rem, :mod, :gcd, :lcm)
for name in (:divexact, :rem, :mod, :gcd, :lcm)
name_bang = Symbol(name, "!")
@eval begin
@doc """
Expand All @@ -394,6 +399,22 @@ for name in (:divexact, :div, :rem, :mod, :gcd, :lcm)
end
end

@doc """
div!(z, a, b)
div!(a, b)
Return `div(a, b)`, possibly modifying the object `z` in the process.
Aliasing is permitted.
The two argument version is a shorthand for `div(a, a, b)`.
!!! note
`AbstractAlgebra.div` and `Base.div` differ only in their behavior on julia
types like `Integer` and `Rational{Int}`. The former makes it adhere to the
Ring interface.
"""
div!(z, a, b) = AbstractAlgebra.div(a, b)
div!(a, b) = div!(a, a, b)

@doc raw"""
canonical_injection(D, i)
Expand Down
4 changes: 2 additions & 2 deletions src/generic/LaurentMPoly.jl
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ function is_gen(a::LaurentMPolyWrap)
return !iszero(_var_index(a))
end

function inv(a::LaurentMPolyWrap)
function Base.inv(a::LaurentMPolyWrap)
(ap, ad) = _normalize(a)
return LaurentMPolyWrap(parent(a), inv(ap), neg!(ad, ad))
end
Expand Down Expand Up @@ -204,7 +204,7 @@ function gcd(a::LaurentMPolyWrap, b::LaurentMPolyWrap)
return LaurentMPolyWrap(parent(a), gcd(ap, bp), zero!(ad))
end

function divrem(a::LaurentMPolyWrap, b::LaurentMPolyWrap)
function Base.divrem(a::LaurentMPolyWrap, b::LaurentMPolyWrap)
check_parent(a, b)
error("divrem not implemented for LaurentMPoly")
end
Expand Down
4 changes: 0 additions & 4 deletions src/generic/Misc/Localization.jl
Original file line number Diff line number Diff line change
Expand Up @@ -263,10 +263,6 @@ function Base.divrem(a::LocalizedEuclideanRingElem{T}, b::LocalizedEuclideanRing
end
end

function Base.div(a::LocalizedEuclideanRingElem{T}, b::LocalizedEuclideanRingElem{T}) where {T}
return divrem(a, b)[1]
end

function rem(a::LocalizedEuclideanRingElem{T}, b::LocalizedEuclideanRingElem{T}) where {T}
return divrem(a, b)[2]
end
Expand Down

0 comments on commit 55c35d9

Please sign in to comment.