Skip to content

Commit cf93a71

Browse files
authored
Fix type instability for exponent iterator with no variable (#343)
* Fix type instability for exponent iterator with no variable * Add tests * Fix format * Switch to SizeUnknown * Ref to Cycle PR * Use SizeUnknown * Add test
1 parent 014104d commit cf93a71

File tree

2 files changed

+27
-9
lines changed

2 files changed

+27
-9
lines changed

src/comparison.jl

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -459,11 +459,6 @@ struct ExponentsIterator{M,D<:Union{Nothing,Int},O}
459459
),
460460
)
461461
end
462-
if length(object) == 0 && isnothing(maxdegree)
463-
# Otherwise, it will incorrectly think that the iterator is infinite
464-
# while it actually has zero elements
465-
maxdegree = mindegree
466-
end
467462
return new{M,typeof(maxdegree),typeof(object)}(
468463
object,
469464
mindegree,
@@ -474,24 +469,41 @@ struct ExponentsIterator{M,D<:Union{Nothing,Int},O}
474469
end
475470

476471
Base.eltype(::Type{ExponentsIterator{M,D,O}}) where {M,D,O} = O
472+
# `IteratorSize` may actually be finite if the list of variables is empty.
473+
# The same issue happens for `Iterators.Cycle`:
474+
# https://github.com/JuliaLang/julia/pull/54187
475+
# With `Base.IsInfinite`, some tests fail but with `Base.SizeUnknown`, all
476+
# tests pass.
477477
function Base.IteratorSize(::Type{<:ExponentsIterator{M,Nothing}}) where {M}
478-
return Base.IsInfinite()
478+
return Base.SizeUnknown()
479479
end
480480
function Base.IteratorSize(::Type{<:ExponentsIterator{M,Int}}) where {M}
481481
return Base.HasLength()
482482
end
483483

484-
function Base.length(it::ExponentsIterator{M,Int}) where {M}
485-
if it.maxdegree < it.mindegree
484+
function _length(it::ExponentsIterator, maxdegree)
485+
if maxdegree < it.mindegree
486486
return 0
487487
end
488-
len = binomial(nvariables(it) + it.maxdegree, nvariables(it))
488+
len = binomial(nvariables(it) + maxdegree, nvariables(it))
489489
if it.mindegree > 0
490490
len -= binomial(nvariables(it) + it.mindegree - 1, nvariables(it))
491491
end
492492
return len
493493
end
494494

495+
function Base.length(it::ExponentsIterator{M,Int}) where {M}
496+
return _length(it, it.maxdegree)
497+
end
498+
499+
function Base.length(it::ExponentsIterator{M,Nothing}) where {M}
500+
if isempty(it.object)
501+
return _length(it, it.mindegree)
502+
else
503+
error("The iterator is infinity because `maxdegree` is `nothing`.")
504+
end
505+
end
506+
495507
nvariables(it::ExponentsIterator) = length(it.object)
496508

497509
_last_lex_index(n, ::Type{LexOrder}) = n

test/comparison.jl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ function _test(object, M; kws...)
77
it = ExponentsIterator{M}(object; kws...)
88
v = collect(Iterators.take(it, 20))
99
@test issorted(v, lt = (a, b) -> cmp(M(), a, b) < 0)
10+
@test issorted(v, lt = M())
1011
end
1112

1213
function _test(nvars::Int, M; kws...)
@@ -26,6 +27,11 @@ function test_errors()
2627
"Ordering `$M` is not a valid ordering, use `Graded{$M}` instead.",
2728
)
2829
@test_throws err ExponentsIterator{M}([0], maxdegree = 2)
30+
exps = ExponentsIterator{LexOrder}([0])
31+
err = ErrorException(
32+
"The iterator is infinity because `maxdegree` is `nothing`.",
33+
)
34+
@test_throws err length(exps)
2935
end
3036

3137
function test_exponents_iterator()

0 commit comments

Comments
 (0)