From 1c951f7fe8e5e923cc5f5b18c91e461b61641b14 Mon Sep 17 00:00:00 2001 From: Sebastian Stock <42280794+sostock@users.noreply.github.com> Date: Wed, 28 Jul 2021 11:53:28 +0200 Subject: [PATCH] Fix `length(::AbstractUnitRange)` and speed up `length(::AbstractUnitRange{<:Rational})` (#41479) * Fix length(::AbstractUnitRange), faster length(::AbstractUnitRange{<:Rational}) --- base/range.jl | 17 ++++++++--------- base/rational.jl | 18 ++++++++++++++++++ test/ranges.jl | 15 ++++++++++++++- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/base/range.jl b/base/range.jl index b6a8e333cf52e..4ddecd7bdd91c 100644 --- a/base/range.jl +++ b/base/range.jl @@ -663,8 +663,8 @@ 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 @@ -672,8 +672,8 @@ function checked_length(r::AbstractUnitRange{T}) where T 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 @@ -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)) diff --git a/base/rational.jl b/base/rational.jl index 23c9298962f53..1c83443d10df3 100644 --- a/base/rational.jl +++ b/base/rational.jl @@ -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 diff --git a/test/ranges.jl b/test/ranges.jl index d4ff83b81fe2d..1c31585ece45e 100644 --- a/test/ranges.jl +++ b/test/ranges.jl @@ -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 @@ -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