Skip to content

Commit

Permalink
Fix length(::AbstractUnitRange) and speed up `length(::AbstractUnit…
Browse files Browse the repository at this point in the history
…Range{<:Rational})` (#41479)

* Fix length(::AbstractUnitRange), faster length(::AbstractUnitRange{<:Rational})
  • Loading branch information
sostock authored Jul 28, 2021
1 parent 57ce0e6 commit 1c951f7
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 10 deletions.
17 changes: 8 additions & 9 deletions base/range.jl
Original file line number Diff line number Diff line change
Expand Up @@ -663,17 +663,17 @@ function checked_length(r::OrdinalRange{T}) where T
else
diff = checked_sub(stop, start)
end
a = Integer(div(diff, s))
return checked_add(a, oneunit(a))
a = div(diff, s)
return Integer(checked_add(a, oneunit(a)))
end

function checked_length(r::AbstractUnitRange{T}) where T
# compiler optimization: remove dead cases from above
if isempty(r)
return Integer(first(r) - first(r))
end
a = Integer(checked_add(checked_sub(last(r), first(r))))
return checked_add(a, oneunit(a))
a = checked_sub(last(r), first(r))
return Integer(checked_add(a, oneunit(a)))
end

function length(r::OrdinalRange{T}) where T
Expand All @@ -690,15 +690,14 @@ function length(r::OrdinalRange{T}) where T
else
diff = stop - start
end
a = Integer(div(diff, s))
return a + oneunit(a)
a = div(diff, s)
return Integer(a + oneunit(a))
end


function length(r::AbstractUnitRange{T}) where T
@_inline_meta
a = Integer(last(r) - first(r)) # even when isempty, by construction (with overflow)
return a + oneunit(a)
a = last(r) - first(r) # even when isempty, by construction (with overflow)
return Integer(a + oneunit(a))
end

length(r::OneTo) = Integer(r.stop - zero(r.stop))
Expand Down
18 changes: 18 additions & 0 deletions base/rational.jl
Original file line number Diff line number Diff line change
Expand Up @@ -533,3 +533,21 @@ function hash(x::Rational{<:BitInteger64}, h::UInt)
h = hash_integer(num, h)
return h
end

# These methods are only needed for performance. Since `first(r)` and `last(r)` have the
# same denominator (because their difference is an integer), `length(r)` can be calulated
# without calling `gcd`.
function length(r::AbstractUnitRange{T}) where T<:Rational
@_inline_meta
f = first(r)
l = last(r)
return div(l.num - f.num + f.den, f.den)
end
function checked_length(r::AbstractUnitRange{T}) where T<:Rational
f = first(r)
l = last(r)
if isempty(r)
return f.num - f.num
end
return div(checked_add(checked_sub(l.num, f.num), f.den), f.den)
end
15 changes: 14 additions & 1 deletion test/ranges.jl
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,19 @@ end
end
end

# A number type with the overflow behavior of `UInt8`. Conversion to `Integer` returns an
# `Int32`, i.e., a type with different `typemin`/`typemax`. See #41479
struct OverflowingReal <: Real
val::UInt8
end
OverflowingReal(x::OverflowingReal) = x
Base.:<=(x::OverflowingReal, y::OverflowingReal) = x.val <= y.val
Base.:+(x::OverflowingReal, y::OverflowingReal) = OverflowingReal(x.val + y.val)
Base.:-(x::OverflowingReal, y::OverflowingReal) = OverflowingReal(x.val - y.val)
Base.round(x::OverflowingReal, ::RoundingMode) = x
Base.Integer(x::OverflowingReal) = Int32(x.val)
@test length(OverflowingReal(1):OverflowingReal(0)) == 0

@testset "loops involving typemin/typemax" begin
n = 0
s = 0
Expand Down Expand Up @@ -1095,7 +1108,7 @@ end
@testset "issue 10950" begin
r = 1//2:3
@test length(r) == 3
@test_throws MethodError checked_length(r) == 3 # this would work if checked_sub is defined on Rational
@test checked_length(r) == 3
i = 1
for x in r
@test x == i//2
Expand Down

0 comments on commit 1c951f7

Please sign in to comment.