Skip to content

Commit 0eb3ed5

Browse files
authored
Address some issues reported by JET (#2248)
* Type param for FracField is RingElem, not RingElement * Type param for TotFracRing is RingElem, not RingElement * Fac type param is RingElement Of course this restriction is effectively enforced by the new{T} inside, but Julia is concerned about the Dict instantiation & resulting conversion anyway. * Turn printer outer constructor into inner This ensures instance invariants hold that make JET happy * Fix JET warning about deg being undefined * Fix JET warnings triggered by parse_cycles This avoids JET warnings in Base * Fix JET warning in Partition constructor JET is concerned about e.g. UnitRange arguments, on which the code before looked as if it might invoke sort! or issorted. * Merge sqrt_classical(_char2) and Generic.sqrt_classical(_char2) * Fix JET complaint in sqrt_classical JET is concerned `g2` might be used without initialization. However, in case of the method taking a `AbsPowerSeriesRingElem` that access is in a loop over `1:prec - aval2 - 1` which is empty unless `prec - aval2 - 1 >= 1`, i.e., `prec > aval2 + 1`. Therefore `prec > aval2` is a necessary precondition, and so we may move that loop inside the `if prec > aval2` block; or equivalently, skip the code if `prec <= aval2`. It is similar in the case of the `LaurentSeriesElem` and `RelPowerSeriesRingElem` methods.
1 parent 88c673c commit 0eb3ed5

File tree

10 files changed

+58
-54
lines changed

10 files changed

+58
-54
lines changed

src/AbsSeries.jl

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -850,18 +850,20 @@ function sqrt_classical(a::AbsPowerSeriesRingElem; check::Bool=true)
850850
asqrt = parent(a)()
851851
fit!(asqrt, prec)
852852
asqrt = _set_precision_raw!(asqrt, prec)
853+
if prec <= aval2
854+
asqrt = set_length!(asqrt, normalise(asqrt, prec))
855+
return true, asqrt
856+
end
853857
for n = 1:aval2
854858
asqrt = setcoeff!(asqrt, n - 1, R())
855859
end
856-
if prec > aval2
857-
c = coeff(a, aval)
858-
if check && !is_square(c)
859-
return false, zero(S)
860-
end
861-
g = sqrt(c; check=check)
862-
asqrt = setcoeff!(asqrt, aval2, g)
863-
g2 = g + g
860+
c = coeff(a, aval)
861+
if check && !is_square(c)
862+
return false, zero(S)
864863
end
864+
g = sqrt(c; check=check)
865+
asqrt = setcoeff!(asqrt, aval2, g)
866+
g2 = g + g
865867
p = R()
866868
for n = 1:prec - aval2 - 1
867869
c = R()

src/Factor.jl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,15 @@ mutable struct Fac{T <: RingElement}
2323
unit::T
2424
arr::Vector{Pair{T, Int}}
2525

26-
function Fac{T}() where {T}
26+
function Fac{T}() where {T <: RingElement}
2727
return new{T}(Dict{T, Int}())
2828
end
2929

30-
function Fac{T}(u::T, d::Dict{T, Int}) where {T}
30+
function Fac{T}(u::T, d::Dict{T, Int}) where {T <: RingElement}
3131
return new{T}(d, u)
3232
end
3333

34-
function Fac{T}(u::T, a::Vector{Pair{T, Int}}) where {T}
34+
function Fac{T}(u::T, a::Vector{Pair{T, Int}}) where {T <: RingElement}
3535
z = new{T}()
3636
z.unit = u
3737
z.arr = a

src/PrettyPrinting.jl

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -568,12 +568,12 @@ mutable struct printer{IOT <: IO}
568568
terse_level::Int
569569
size_limit_stack::Vector{Int} # >= 0 for loosely-defined limit before ...
570570
# < 0 for unrestricted output
571-
end
572-
573-
function printer(io::IO)
574-
terse_level = get(io, :terse_level, 0)
575-
size_limit = get(io, :size_limit, -1)
576-
return printer{typeof(io)}(io, String[], terse_level, Int[size_limit])
571+
572+
function printer(io::IO)
573+
terse_level = get(io, :terse_level, 0)
574+
size_limit = get(io, :size_limit, -1)
575+
return new{typeof(io)}(io, String[], terse_level, Int[size_limit])
576+
end
577577
end
578578

579579
# TODO since the subexpressions are not changing much, cache the leaf_count

src/RelSeries.jl

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,25 +1108,22 @@ function sqrt_classical(a::RelPowerSeriesRingElem; check::Bool=true)
11081108
end
11091109
aval2 = div(aval, 2)
11101110
prec = precision(a) - aval
1111-
if prec == 0
1112-
asqrt = parent(a)()
1111+
asqrt = parent(a)()
1112+
if prec <= 0
11131113
asqrt = _set_precision_raw!(asqrt, aval2)
11141114
asqrt = set_valuation!(asqrt, aval2)
11151115
return true, asqrt
11161116
end
1117-
asqrt = parent(a)()
11181117
fit!(asqrt, prec)
11191118
asqrt = _set_precision_raw!(asqrt, prec + aval2)
11201119
asqrt = set_valuation!(asqrt, aval2)
1121-
if prec > 0
1122-
c = polcoeff(a, 0)
1123-
if check && !is_square(c)
1124-
return false, zero(S)
1125-
end
1126-
g = sqrt(c; check=check)
1127-
asqrt = setcoeff!(asqrt, 0, g)
1128-
g2 = g + g
1120+
c = polcoeff(a, 0)
1121+
if check && !is_square(c)
1122+
return false, zero(S)
11291123
end
1124+
g = sqrt(c; check=check)
1125+
asqrt = setcoeff!(asqrt, 0, g)
1126+
g2 = g + g
11301127
p = R()
11311128
for n = 1:prec - 1
11321129
c = R()

src/algorithms/LaurentPoly.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ end
8888
# return the degree of the unique term, throw if not a term
8989
function term_degree(p::LaurentPolyRingElem)
9090
isnull = true
91-
local deg
91+
deg = -1
9292
mds = terms_degrees(p)
9393
isempty(mds) && throw(DomainError(p, "not a term"))
9494
for d in mds
@@ -98,7 +98,7 @@ function term_degree(p::LaurentPolyRingElem)
9898
isnull = false
9999
end
100100
end
101-
deg
101+
return deg
102102
end
103103

104104
function leading_coefficient(p::LaurentPolyRingElem)

src/generic/GenericTypes.jl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,10 @@ struct Partition{T} <: AbstractVector{T}
9393
Partition(part::AbstractVector{<:Integer}, check::Bool=true) =
9494
Partition(sum(part), part, check)
9595

96-
function Partition(n::Integer, part::AbstractVector{T}, check::Bool=true) where T
96+
Partition(n::Integer, part::AbstractVector, check::Bool=true) =
97+
Partition(n, collect(part), check)
98+
99+
function Partition(n::Integer, part::Vector{T}, check::Bool=true) where T
97100
if check
98101
issorted(part, rev=true) || sort!(part, rev=true)
99102
if length(part) > 0

src/generic/LaurentSeries.jl

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,28 +1330,25 @@ function sqrt_classical(a::LaurentSeriesElem; check::Bool=true)
13301330
end
13311331
aval2 = div(aval, 2)
13321332
prec = precision(a) - aval
1333-
if prec == 0
1334-
asqrt = parent(a)()
1333+
asqrt = parent(a)()
1334+
if prec <= 0
13351335
asqrt = set_precision!(asqrt, aval2)
13361336
asqrt = set_valuation!(asqrt, aval2)
13371337
asqrt = set_scale!(asqrt, 1)
13381338
return true, asqrt
13391339
end
1340-
asqrt = parent(a)()
13411340
s = scale(a)
13421341
zlen = div(prec + s - 1, s)
13431342
fit!(asqrt, zlen)
13441343
asqrt = set_precision!(asqrt, prec + aval2)
13451344
asqrt = set_valuation!(asqrt, aval2)
1346-
if prec > 0
1347-
c = polcoeff(a, 0)
1348-
if check && !is_square(c)
1349-
return false, zero(S)
1350-
end
1351-
g = sqrt(c; check=check)
1352-
asqrt = setcoeff!(asqrt, 0, g)
1353-
g2 = g + g
1345+
c = polcoeff(a, 0)
1346+
if check && !is_square(c)
1347+
return false, zero(S)
13541348
end
1349+
g = sqrt(c; check=check)
1350+
asqrt = setcoeff!(asqrt, 0, g)
1351+
g2 = g + g
13551352
p = R()
13561353
for n = 1:zlen - 1
13571354
c = R()

src/generic/PermGroups.jl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,10 @@ end
817817
###############################################################################
818818

819819
function parse_cycles(str::AbstractString)
820+
return parse_cycles(string(str))
821+
end
822+
823+
function parse_cycles(str::Union{String, SubString})
820824
ccycles = Int[]
821825
cptrs = Int[1]
822826
if startswith(str, "Cycle Decomposition: ")

src/generic/TotalFraction.jl

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ base_ring(a::TotFracRing{T}) where T <: RingElem = a.base_ring::parent_type(T)
2626

2727
parent(a::TotFrac) = a.parent
2828

29-
function is_domain_type(::Type{T}) where {S <: RingElement, T <: TotFrac{S}}
29+
function is_domain_type(::Type{T}) where {S <: RingElem, T <: TotFrac{S}}
3030
return is_domain_type(S)
3131
end
3232

33-
function is_exact_type(a::Type{T}) where {S <: RingElement, T <: TotFrac{S}}
33+
function is_exact_type(a::Type{T}) where {S <: RingElem, T <: TotFrac{S}}
3434
return is_exact_type(S)
3535
end
3636

@@ -530,7 +530,6 @@ rand(S::TotFracRing, v...) = rand(Random.default_rng(), S, v...)
530530
#
531531
###############################################################################
532532

533-
promote_rule(::Type{TotFrac{T}}, ::Type{TotFrac{T}}) where T <: RingElement = TotFrac{T}
534533
promote_rule(::Type{TotFrac{T}}, ::Type{TotFrac{T}}) where T <: RingElem = TotFrac{T}
535534

536535
function promote_rule(::Type{TotFrac{T}}, ::Type{U}) where {T <: RingElem, U <: RingElem}
@@ -543,26 +542,26 @@ end
543542
#
544543
###############################################################################
545544

546-
function (a::TotFracRing{T})(b::RingElement) where {T <: RingElement}
545+
function (a::TotFracRing{T})(b::RingElement) where {T <: RingElem}
547546
return a(base_ring(a)(b))
548547
end
549548

550-
function (a::TotFracRing{T})() where {T <: RingElement}
549+
function (a::TotFracRing{T})() where {T <: RingElem}
551550
R = base_ring(a)
552551
z = TotFrac{T}(zero(R), one(R))
553552
z.parent = a
554553
return z
555554
end
556555

557-
function (a::TotFracRing{T})(b::T) where {T <: RingElement}
556+
function (a::TotFracRing{T})(b::T) where {T <: RingElem}
558557
R = base_ring(a)
559558
parent(b) != R && error("Could not coerce to fraction")
560559
z = TotFrac{T}(b, one(R))
561560
z.parent = a
562561
return z
563562
end
564563

565-
function (a::TotFracRing{T})(b::T, c::T, check::Bool=true) where {T <: RingElement}
564+
function (a::TotFracRing{T})(b::T, c::T, check::Bool=true) where {T <: RingElem}
566565
R = base_ring(a)
567566
parent(b) != R && error("Could not coerce to fraction")
568567
parent(c) != R && error("Could not coerce to fraction")
@@ -575,23 +574,23 @@ function (a::TotFracRing{T})(b::T, c::T, check::Bool=true) where {T <: RingEleme
575574
return z
576575
end
577576

578-
function (a::TotFracRing{T})(b::T, c::Union{Integer, Rational}, check::Bool=true) where {T <: RingElement}
577+
function (a::TotFracRing{T})(b::T, c::Union{Integer, Rational}, check::Bool=true) where {T <: RingElem}
579578
a(b, base_ring(a)(c), check)
580579
end
581580

582-
function (a::TotFracRing{T})(b::Union{Integer, Rational}, c::T, check::Bool=true) where {T <: RingElement}
581+
function (a::TotFracRing{T})(b::Union{Integer, Rational}, c::T, check::Bool=true) where {T <: RingElem}
583582
a(base_ring(a)(b), c, check)
584583
end
585584

586-
function (a::TotFracRing{T})(b::Union{Integer, Rational}) where {T <: RingElement}
585+
function (a::TotFracRing{T})(b::Union{Integer, Rational}) where {T <: RingElem}
587586
a(base_ring(a)(b))
588587
end
589588

590-
function (a::TotFracRing{T})(b::Integer, c::Integer, check::Bool=true) where {T <: RingElement}
589+
function (a::TotFracRing{T})(b::Integer, c::Integer, check::Bool=true) where {T <: RingElem}
591590
a(base_ring(a)(b), base_ring(a)(c), check)
592591
end
593592

594-
function (a::TotFracRing{T})(b::TotFrac{T}) where {T <: RingElement}
593+
function (a::TotFracRing{T})(b::TotFrac{T}) where {T <: RingElem}
595594
a != parent(b) && error("Could not coerce to fraction")
596595
return b
597596
end

src/generic/imports.jl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,8 @@ import ..AbstractAlgebra: shift_left
218218
import ..AbstractAlgebra: shift_right
219219
import ..AbstractAlgebra: snf
220220
import ..AbstractAlgebra: sqrt
221+
import ..AbstractAlgebra: sqrt_classical
222+
import ..AbstractAlgebra: sqrt_classical_char2
221223
import ..AbstractAlgebra: sub!
222224
import ..AbstractAlgebra: submul!
223225
import ..AbstractAlgebra: symbols

0 commit comments

Comments
 (0)