From 6fd25769d2b119d50728e26ef15fdcdf4990f3c7 Mon Sep 17 00:00:00 2001 From: Sebastian Stock <42280794+sostock@users.noreply.github.com> Date: Tue, 6 Jul 2021 10:39:33 +0200 Subject: [PATCH 1/9] Fix length(::AbstractUnitRange), faster length(::AbstractUnitRange{<:Rational}) --- base/range.jl | 4 ++-- base/rational.jl | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/base/range.jl b/base/range.jl index 8845f363cd38c..45785811ef2c8 100644 --- a/base/range.jl +++ b/base/range.jl @@ -681,8 +681,8 @@ 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 + one(a) + a = last(r) - first(r) # even when isempty, by construction (with overflow) + return Integer(a + one(a)) end length(r::OneTo) = Integer(r.stop - zero(r.stop)) diff --git a/base/rational.jl b/base/rational.jl index ab4e7d6d0abbd..04471524f8551 100644 --- a/base/rational.jl +++ b/base/rational.jl @@ -533,3 +533,10 @@ function hash(x::Rational{<:BitInteger64}, h::UInt) h = hash_integer(num, h) return h end + +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 From 206f154404f5add758b557df34ebe9c417eab8d6 Mon Sep 17 00:00:00 2001 From: Sebastian Stock <42280794+sostock@users.noreply.github.com> Date: Tue, 6 Jul 2021 16:22:13 +0200 Subject: [PATCH 2/9] Replace `one` by `oneunit` Co-authored-by: Jan Weidner --- base/range.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/range.jl b/base/range.jl index 45785811ef2c8..794e4f5c787d3 100644 --- a/base/range.jl +++ b/base/range.jl @@ -682,7 +682,7 @@ end function length(r::AbstractUnitRange{T}) where T @_inline_meta a = last(r) - first(r) # even when isempty, by construction (with overflow) - return Integer(a + one(a)) + return Integer(a + oneunit(a)) end length(r::OneTo) = Integer(r.stop - zero(r.stop)) From c8e04d6c6067316671c6bae6e10e478f88dd2c9e Mon Sep 17 00:00:00 2001 From: Sebastian Stock <42280794+sostock@users.noreply.github.com> Date: Tue, 6 Jul 2021 17:06:40 +0200 Subject: [PATCH 3/9] Add test --- test/ranges.jl | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/ranges.jl b/test/ranges.jl index b622fc05b3ca6..aecd09c6302e3 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`. +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 From 4a49d28100df790f376f09bd4a1628289b6c0106 Mon Sep 17 00:00:00 2001 From: Sebastian Stock <42280794+sostock@users.noreply.github.com> Date: Wed, 7 Jul 2021 11:37:14 +0200 Subject: [PATCH 4/9] Add reference to PR Co-authored-by: Jan Weidner --- test/ranges.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ranges.jl b/test/ranges.jl index aecd09c6302e3..5a84e046ec54f 100644 --- a/test/ranges.jl +++ b/test/ranges.jl @@ -545,7 +545,7 @@ 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`. +# `Int32`, i.e., a type with different `typemin`/`typemax`. See #41479 struct OverflowingReal <: Real val::UInt8 end From 6a4a9a8bd4d281e1f1050e9c0bceb3608b9bfad9 Mon Sep 17 00:00:00 2001 From: Sebastian Stock <42280794+sostock@users.noreply.github.com> Date: Tue, 20 Jul 2021 17:49:58 +0200 Subject: [PATCH 5/9] Add comment that specialized method is only needed for performance --- base/rational.jl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/base/rational.jl b/base/rational.jl index 04471524f8551..26d34d1cfb9ba 100644 --- a/base/rational.jl +++ b/base/rational.jl @@ -534,6 +534,9 @@ function hash(x::Rational{<:BitInteger64}, h::UInt) return h end +# This method is 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) From 0078f1e7e8d07549b80bcf30eb921335c1d9415f Mon Sep 17 00:00:00 2001 From: Sebastian Stock <42280794+sostock@users.noreply.github.com> Date: Tue, 20 Jul 2021 18:52:08 +0200 Subject: [PATCH 6/9] More one(a) -> oneunit(a) --- base/range.jl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/base/range.jl b/base/range.jl index 794e4f5c787d3..3d26059fddca7 100644 --- a/base/range.jl +++ b/base/range.jl @@ -648,7 +648,7 @@ function checked_length(r::OrdinalRange{T}) where T diff = checked_sub(stop, start) end a = Integer(div(diff, s)) - return checked_add(a, one(a)) + return checked_add(a, oneunit(a)) end function checked_length(r::AbstractUnitRange{T}) where T @@ -657,7 +657,7 @@ function checked_length(r::AbstractUnitRange{T}) where T return Integer(first(r) - first(r)) end a = Integer(checked_add(checked_sub(last(r), first(r)))) - return checked_add(a, one(a)) + return checked_add(a, oneunit(a)) end function length(r::OrdinalRange{T}) where T @@ -675,7 +675,7 @@ function length(r::OrdinalRange{T}) where T diff = stop - start end a = Integer(div(diff, s)) - return a + one(a) + return a + oneunit(a) end @@ -710,7 +710,7 @@ let bigints = Union{Int, UInt, Int64, UInt64, Int128, UInt128} else a = div(unsigned(diff), s) % typeof(diff) end - return Integer(a) + one(a) + return Integer(a) + oneunit(a) end function checked_length(r::OrdinalRange{T}) where T<:bigints s = step(r) @@ -729,7 +729,7 @@ let bigints = Union{Int, UInt, Int64, UInt64, Int128, UInt128} else a = div(checked_sub(start, stop), -s) end - return checked_add(a, one(a)) + return checked_add(a, oneunit(a)) end end From 926029ddf37a1ee71ab04bd2ddb15c9d19b1b8e4 Mon Sep 17 00:00:00 2001 From: Sebastian Stock <42280794+sostock@users.noreply.github.com> Date: Thu, 22 Jul 2021 20:48:59 +0200 Subject: [PATCH 7/9] Consistent use of Integer() --- base/range.jl | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/base/range.jl b/base/range.jl index de166c83e0190..87d5598d1469a 100644 --- a/base/range.jl +++ b/base/range.jl @@ -647,8 +647,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 @@ -656,8 +656,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 @@ -674,11 +674,10 @@ 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 = last(r) - first(r) # even when isempty, by construction (with overflow) From 875a0fa941b93a37266ce85dbc8c767073544401 Mon Sep 17 00:00:00 2001 From: Sebastian Stock <42280794+sostock@users.noreply.github.com> Date: Sat, 24 Jul 2021 22:21:34 +0200 Subject: [PATCH 8/9] Add checked_length(::AbstractUnitRange{<:Rational}) --- base/rational.jl | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/base/rational.jl b/base/rational.jl index b01e52e9736b8..1c83443d10df3 100644 --- a/base/rational.jl +++ b/base/rational.jl @@ -534,12 +534,20 @@ function hash(x::Rational{<:BitInteger64}, h::UInt) return h end -# This method is 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`. +# 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 From e6528e12700581e5f6c4bd266798ea5b625389cb Mon Sep 17 00:00:00 2001 From: Sebastian Stock <42280794+sostock@users.noreply.github.com> Date: Sat, 24 Jul 2021 23:38:01 +0200 Subject: [PATCH 9/9] Fix a test --- test/ranges.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ranges.jl b/test/ranges.jl index acc01581d4cbb..1c31585ece45e 100644 --- a/test/ranges.jl +++ b/test/ranges.jl @@ -1108,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