From 1424822aed22bc630acdb22edd341a2af0eb719f Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Mon, 5 Apr 2021 15:18:51 -0400 Subject: [PATCH 1/3] deprecate unsafe_length for length This seems to be a fairly arbitrary case for throwing exceptions, when the user might often use this value in arithmetic afterwards, which is not checked. It leads to awkward complexity in the API however, where it may be unclear which function to reach for, with no particular justification for why a particular usage is "safe". And it inhibits optimization and performance due to the additional checks and error cases (and is not even entirely type-stable). --- base/abstractarray.jl | 15 ++-- base/broadcast.jl | 2 +- base/checked.jl | 12 ++- base/deprecated.jl | 3 + base/indices.jl | 8 +- base/multidimensional.jl | 2 +- base/range.jl | 127 ++++++++++++++++++++--------- base/subarray.jl | 10 +-- stdlib/Dates/test/ranges.jl | 2 +- test/ranges.jl | 60 ++++++++------ test/testhelpers/Furlongs.jl | 15 ++-- test/testhelpers/InfiniteArrays.jl | 8 +- test/testhelpers/OffsetArrays.jl | 2 +- 13 files changed, 161 insertions(+), 105 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 16fdb61e1d3fe..53a162c958c5b 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -116,9 +116,6 @@ axes1(A::AbstractArray{<:Any,0}) = OneTo(1) axes1(A::AbstractArray) = (@_inline_meta; axes(A)[1]) axes1(iter) = oneto(length(iter)) -unsafe_indices(A) = axes(A) -unsafe_indices(r::AbstractRange) = (oneto(unsafe_length(r)),) # Ranges use checked_sub for size - """ keys(a::AbstractArray) @@ -580,14 +577,14 @@ end function trailingsize(inds::Indices, n) s = 1 for i=n:length(inds) - s *= unsafe_length(inds[i]) + s *= length(inds[i]) end return s end # This version is type-stable even if inds is heterogeneous function trailingsize(inds::Indices) @_inline_meta - prod(map(unsafe_length, inds)) + prod(map(length, inds)) end ## Bounds checking ## @@ -688,7 +685,7 @@ function checkbounds_indices(::Type{Bool}, ::Tuple{}, I::Tuple) @_inline_meta checkindex(Bool, OneTo(1), I[1])::Bool & checkbounds_indices(Bool, (), tail(I)) end -checkbounds_indices(::Type{Bool}, IA::Tuple, ::Tuple{}) = (@_inline_meta; all(x->unsafe_length(x)==1, IA)) +checkbounds_indices(::Type{Bool}, IA::Tuple, ::Tuple{}) = (@_inline_meta; all(x->length(x)==1, IA)) checkbounds_indices(::Type{Bool}, ::Tuple{}, ::Tuple{}) = true throw_boundserror(A, I) = (@_noinline_meta; throw(BoundsError(A, I))) @@ -2472,8 +2469,8 @@ function _sub2ind_recurse(inds, L, ind, i::Integer, I::Integer...) end nextL(L, l::Integer) = L*l -nextL(L, r::AbstractUnitRange) = L*unsafe_length(r) -nextL(L, r::Slice) = L*unsafe_length(r.indices) +nextL(L, r::AbstractUnitRange) = L*length(r) +nextL(L, r::Slice) = L*length(r.indices) offsetin(i, l::Integer) = i-1 offsetin(i, r::AbstractUnitRange) = i-first(r) @@ -2499,7 +2496,7 @@ end _lookup(ind, d::Integer) = ind+1 _lookup(ind, r::AbstractUnitRange) = ind+first(r) _div(ind, d::Integer) = div(ind, d), 1, d -_div(ind, r::AbstractUnitRange) = (d = unsafe_length(r); (div(ind, d), first(r), d)) +_div(ind, r::AbstractUnitRange) = (d = length(r); (div(ind, d), first(r), d)) # Vectorized forms function _sub2ind(inds::Indices{1}, I1::AbstractVector{T}, I::AbstractVector{T}...) where T<:Integer diff --git a/base/broadcast.jl b/base/broadcast.jl index 7c87fef3f19e6..c6651e28489a3 100644 --- a/base/broadcast.jl +++ b/base/broadcast.jl @@ -566,7 +566,7 @@ an `Int`. """ Base.@propagate_inbounds newindex(arg, I::CartesianIndex) = CartesianIndex(_newindex(axes(arg), I.I)) Base.@propagate_inbounds newindex(arg, I::Integer) = CartesianIndex(_newindex(axes(arg), (I,))) -Base.@propagate_inbounds _newindex(ax::Tuple, I::Tuple) = (ifelse(Base.unsafe_length(ax[1])==1, ax[1][1], I[1]), _newindex(tail(ax), tail(I))...) +Base.@propagate_inbounds _newindex(ax::Tuple, I::Tuple) = (ifelse(length(ax[1]) == 1, ax[1][1], I[1]), _newindex(tail(ax), tail(I))...) Base.@propagate_inbounds _newindex(ax::Tuple{}, I::Tuple) = () Base.@propagate_inbounds _newindex(ax::Tuple, I::Tuple{}) = (ax[1][1], _newindex(tail(ax), ())...) Base.@propagate_inbounds _newindex(ax::Tuple{}, I::Tuple{}) = () diff --git a/base/checked.jl b/base/checked.jl index 840015861923f..65f3d4234045e 100644 --- a/base/checked.jl +++ b/base/checked.jl @@ -6,14 +6,14 @@ module Checked export checked_neg, checked_abs, checked_add, checked_sub, checked_mul, checked_div, checked_rem, checked_fld, checked_mod, checked_cld, - add_with_overflow, sub_with_overflow, mul_with_overflow + checked_length, add_with_overflow, sub_with_overflow, mul_with_overflow import Core.Intrinsics: checked_sadd_int, checked_ssub_int, checked_smul_int, checked_sdiv_int, checked_srem_int, checked_uadd_int, checked_usub_int, checked_umul_int, checked_udiv_int, checked_urem_int -import ..no_op_err, ..@_inline_meta, ..@_noinline_meta +import ..no_op_err, ..@_inline_meta, ..@_noinline_meta, ..checked_length # define promotion behavior for checked operations checked_add(x::Integer, y::Integer) = checked_add(promote(x,y)...) @@ -349,4 +349,12 @@ The overflow protection may impose a perceptible performance penalty. """ checked_cld(x::T, y::T) where {T<:Integer} = cld(x, y) # Base.cld already checks +""" + Base.checked_length(r) + +Calculates `length(r)`, but may check for overflow errors where applicable when +the result doesn't fit into `Union{eltype(r),Int}`. +""" +checked_length(r) = length(r) # for most things, length doesn't error + end diff --git a/base/deprecated.jl b/base/deprecated.jl index d60cc3393662c..3ed9ffa9c34e8 100644 --- a/base/deprecated.jl +++ b/base/deprecated.jl @@ -240,6 +240,9 @@ end @deprecate cat_shape(dims, shape::Tuple{}, shapes::Tuple...) cat_shape(dims, shapes) false cat_shape(dims, shape::Tuple{}) = () # make sure `cat_shape(dims, ())` do not recursively calls itself +@deprecate unsafe_indices(A) axes(A) false +@deprecate unsafe_length(r) length(r) false + # END 1.6 deprecations # BEGIN 1.7 deprecations diff --git a/base/indices.jl b/base/indices.jl index ef9e2c52ca6a3..817d9d435522b 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -352,17 +352,14 @@ struct Slice{T<:AbstractUnitRange} <: AbstractUnitRange{Int} end Slice(S::Slice) = S axes(S::Slice) = (IdentityUnitRange(S.indices),) -unsafe_indices(S::Slice) = (IdentityUnitRange(S.indices),) axes1(S::Slice) = IdentityUnitRange(S.indices) axes(S::Slice{<:OneTo}) = (S.indices,) -unsafe_indices(S::Slice{<:OneTo}) = (S.indices,) axes1(S::Slice{<:OneTo}) = S.indices first(S::Slice) = first(S.indices) last(S::Slice) = last(S.indices) size(S::Slice) = (length(S.indices),) length(S::Slice) = length(S.indices) -unsafe_length(S::Slice) = unsafe_length(S.indices) getindex(S::Slice, i::Int) = (@_inline_meta; @boundscheck checkbounds(S, i); i) getindex(S::Slice, i::AbstractUnitRange{<:Integer}) = (@_inline_meta; @boundscheck checkbounds(S, i); i) getindex(S::Slice, i::StepRange{<:Integer}) = (@_inline_meta; @boundscheck checkbounds(S, i); i) @@ -383,17 +380,14 @@ end IdentityUnitRange(S::IdentityUnitRange) = S # IdentityUnitRanges are offset and thus have offset axes, so they are their own axes axes(S::IdentityUnitRange) = (S,) -unsafe_indices(S::IdentityUnitRange) = (S,) axes1(S::IdentityUnitRange) = S axes(S::IdentityUnitRange{<:OneTo}) = (S.indices,) -unsafe_indices(S::IdentityUnitRange{<:OneTo}) = (S.indices,) axes1(S::IdentityUnitRange{<:OneTo}) = S.indices first(S::IdentityUnitRange) = first(S.indices) last(S::IdentityUnitRange) = last(S.indices) size(S::IdentityUnitRange) = (length(S.indices),) length(S::IdentityUnitRange) = length(S.indices) -unsafe_length(S::IdentityUnitRange) = unsafe_length(S.indices) getindex(S::IdentityUnitRange, i::Int) = (@_inline_meta; @boundscheck checkbounds(S, i); i) getindex(S::IdentityUnitRange, i::AbstractUnitRange{<:Integer}) = (@_inline_meta; @boundscheck checkbounds(S, i); i) getindex(S::IdentityUnitRange, i::StepRange{<:Integer}) = (@_inline_meta; @boundscheck checkbounds(S, i); i) @@ -479,7 +473,7 @@ convert(::Type{LinearIndices{N,R}}, inds::LinearIndices{N}) where {N,R} = # AbstractArray implementation IndexStyle(::Type{<:LinearIndices}) = IndexLinear() axes(iter::LinearIndices) = map(axes1, iter.indices) -size(iter::LinearIndices) = map(unsafe_length, iter.indices) +size(iter::LinearIndices) = map(length, iter.indices) function getindex(iter::LinearIndices, i::Int) @_inline_meta @boundscheck checkbounds(iter, i) diff --git a/base/multidimensional.jl b/base/multidimensional.jl index 2f277af36cdc1..8104cddb34387 100644 --- a/base/multidimensional.jl +++ b/base/multidimensional.jl @@ -849,7 +849,7 @@ function _unsafe_getindex(::IndexStyle, A::AbstractArray, I::Vararg{Union{Real, # This is specifically not inlined to prevent excessive allocations in type unstable code shape = index_shape(I...) dest = similar(A, shape) - map(unsafe_length, axes(dest)) == map(unsafe_length, shape) || throw_checksize_error(dest, shape) + map(length, axes(dest)) == map(length, shape) || throw_checksize_error(dest, shape) _unsafe_getindex!(dest, A, I...) # usually a generated function, don't allow it to impact inference result return dest end diff --git a/base/range.jl b/base/range.jl index 8051b0cdb7ccb..4c4237cad0637 100644 --- a/base/range.jl +++ b/base/range.jl @@ -585,9 +585,11 @@ end ## interface implementations +length(r::AbstractRange) = error("length implementation missing") # catch mistakes size(r::AbstractRange) = (length(r),) isempty(r::StepRange) = + # steprange_last_empty(r.start, r.step, r.stop) == r.stop (r.start != r.stop) & ((r.step > zero(r.step)) != (r.stop > r.start)) isempty(r::AbstractUnitRange) = first(r) > last(r) isempty(r::StepRangeLen) = length(r) == 0 @@ -614,7 +616,7 @@ julia> step(range(2.5, stop=10.9, length=85)) ``` """ step(r::StepRange) = r.step -step(r::AbstractUnitRange{T}) where{T} = oneunit(T) - zero(T) +step(r::AbstractUnitRange{T}) where {T} = oneunit(T) - zero(T) step(r::StepRangeLen) = r.step step(r::StepRangeLen{T}) where {T<:AbstractFloat} = T(r.step) step(r::LinRange) = (last(r)-first(r))/r.lendiv @@ -622,60 +624,107 @@ step(r::LinRange) = (last(r)-first(r))/r.lendiv step_hp(r::StepRangeLen) = r.step step_hp(r::AbstractRange) = step(r) -unsafe_length(r::AbstractRange) = length(r) # generic fallback - -function unsafe_length(r::StepRange) - n = Integer(div((r.stop - r.start) + r.step, r.step)) - isempty(r) ? zero(n) : n -end -length(r::StepRange) = unsafe_length(r) -unsafe_length(r::AbstractUnitRange) = Integer(last(r) - first(r) + step(r)) -unsafe_length(r::OneTo) = Integer(r.stop - zero(r.stop)) -length(r::AbstractUnitRange) = unsafe_length(r) -length(r::OneTo) = unsafe_length(r) -length(r::StepRangeLen) = r.len -length(r::LinRange) = r.len +axes(r::AbstractRange) = (oneto(length(r)),) # Needed to fold the `firstindex` call in SimdLoop.simd_index firstindex(::UnitRange) = 1 firstindex(::StepRange) = 1 firstindex(::LinRange) = 1 -function length(r::StepRange{T}) where T<:Union{Int,UInt,Int64,UInt64,Int128,UInt128} - isempty(r) && return zero(T) - if r.step > 1 - return checked_add(convert(T, div(unsigned(r.stop - r.start), r.step)), one(T)) - elseif r.step < -1 - return checked_add(convert(T, div(unsigned(r.start - r.stop), -r.step)), one(T)) - elseif r.step > 0 - return checked_add(div(checked_sub(r.stop, r.start), r.step), one(T)) - else - return checked_add(div(checked_sub(r.start, r.stop), -r.step), one(T)) +# n.b. checked_length for these is defined iff checked_add and checked_sub are +# defined between the relevant types +function checked_length(r::OrdinalRange{T}) where T + s = step(r) + # s != 0, by construction, but avoids the division error later + start = first(r) + if s == zero(s) || isempty(r) + return Integer(start - start + zero(s)) + end + return Integer(div(checked_add(checked_sub(last(r), start), s, s))) +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 a + one(a) +end + +function length(r::OrdinalRange{T}) where T + s = step(r) + # s != 0, by construction, but avoids the division error later + start = first(r) + if s == zero(s) || isempty(r) + return Integer(start - start + zero(s)) end + return Integer(div(last(r) - start + s, s)) end -function length(r::AbstractUnitRange{T}) where T<:Union{Int,Int64,Int128} +function length(r::AbstractUnitRange{T}) where T @_inline_meta - checked_add(checked_sub(last(r), first(r)), one(T)) + a = Integer(last(r) - first(r)) # even when isempty, by construction (with overflow) + return a + one(a) end -length(r::OneTo{T}) where {T<:Union{Int,Int64}} = T(r.stop) -length(r::AbstractUnitRange{T}) where {T<:Union{UInt,UInt64,UInt128}} = - r.stop < r.start ? zero(T) : checked_add(last(r) - first(r), one(T)) +length(r::OneTo) = Integer(r.stop - zero(r.stop)) +length(r::StepRangeLen) = r.len +length(r::LinRange) = r.len -# some special cases to favor default Int type -let smallint = (Int === Int64 ? - Union{Int8,UInt8,Int16,UInt16,Int32,UInt32} : - Union{Int8,UInt8,Int16,UInt16}) +let bigints = Union{Int, UInt, Int64, UInt64, Int128, UInt128} global length - - function length(r::StepRange{<:smallint}) - isempty(r) && return Int(0) - div(Int(r.stop)+Int(r.step) - Int(r.start), Int(r.step)) + # compile optimization for which promote_type(T, Int) == T + length(r::OneTo{T}) where {T<:bigints} = r.stop + # slightly more accurate length and checked_length in extreme cases + # (near typemax) for types with known `unsigned` functions + function length(r::OrdinalRange{T}) where T<:bigints + s = step(r) + s == zero(s) && return zero(T) # unreachable, by construction, but avoids the error case here later + isempty(r) && return zero(T) + diff = last(r) - first(r) + # if |s| > 1, diff might have overflowed, but unsigned(diff)÷s should + # therefore still be valid (if the result is representable at all) + # n.b. !(s isa T) + if s isa Unsigned || -1 <= s <= 1 || s == -s + a = div(diff, s) + elseif s < 0 + a = div(unsigned(-diff), -s) % typeof(diff) + else + a = div(unsigned(diff), s) % typeof(diff) + end + return Integer(a) + one(a) end + function checked_length(r::OrdinalRange{T}) where T<:bigints + s = step(r) + s == zero(s) && return zero(T) # unreachable, by construction, but avoids the error case here later + isempty(r) && return zero(T) + stop, start = last(r), first(r) + # n.b. !(s isa T) + if s > 1 + diff = stop - start + a = convert(T, div(unsigned(diff), s)) + elseif s < -1 + diff = start - stop + a = convert(T, div(unsigned(diff), -s)) + elseif s > 0 + a = div(checked_sub(stop, start), s) + else + a = div(checked_sub(start, stop), -s) + end + return checked_add(a, one(a)) + end +end - length(r::AbstractUnitRange{<:smallint}) = Int(last(r)) - Int(first(r)) + 1 - length(r::OneTo{<:smallint}) = Int(r.stop) +# some special cases to favor default Int type +let smallints = (Int === Int64 ? + Union{Int8, UInt8, Int16, UInt16, Int32, UInt32} : + Union{Int8, UInt8, Int16, UInt16}) + global length + # n.b. !(step isa T) + length(r::OrdinalRange{<:smallints}) = div(Int(last(r)) - Int(first(r)) + step(r), step(r)) + length(r::AbstractUnitRange{<:smallints}) = Int(last(r)) - Int(first(r)) + 1 + length(r::OneTo{<:smallints}) = Int(r.stop) end first(r::OrdinalRange{T}) where {T} = convert(T, r.start) diff --git a/base/subarray.jl b/base/subarray.jl index 32262058cb55b..89a4db4d65790 100644 --- a/base/subarray.jl +++ b/base/subarray.jl @@ -60,7 +60,7 @@ viewindexing(I::Tuple{Vararg{Any}}) = IndexCartesian() viewindexing(I::Tuple{AbstractArray, Vararg{Any}}) = IndexCartesian() # Simple utilities -size(V::SubArray) = (@_inline_meta; map(unsafe_length, axes(V))) +size(V::SubArray) = (@_inline_meta; map(length, axes(V))) similar(V::SubArray, T::Type, dims::Dims) = similar(V.parent, T, dims) @@ -362,7 +362,7 @@ compute_stride1(parent::AbstractArray, I::NTuple{N,Any}) where {N} = compute_stride1(s, inds, I::Tuple{}) = s compute_stride1(s, inds, I::Tuple{Vararg{ScalarIndex}}) = s compute_stride1(s, inds, I::Tuple{ScalarIndex, Vararg{Any}}) = - (@_inline_meta; compute_stride1(s*unsafe_length(inds[1]), tail(inds), tail(I))) + (@_inline_meta; compute_stride1(s*length(inds[1]), tail(inds), tail(I))) compute_stride1(s, inds, I::Tuple{AbstractRange, Vararg{Any}}) = s*step(I[1]) compute_stride1(s, inds, I::Tuple{Slice, Vararg{Any}}) = s compute_stride1(s, inds, I::Tuple{Any, Vararg{Any}}) = throw(ArgumentError("invalid strided index type $(typeof(I[1]))")) @@ -407,12 +407,12 @@ end function compute_linindex(f, s, IP::Tuple, I::Tuple{ScalarIndex, Vararg{Any}}) @_inline_meta Δi = I[1]-first(IP[1]) - compute_linindex(f + Δi*s, s*unsafe_length(IP[1]), tail(IP), tail(I)) + compute_linindex(f + Δi*s, s*length(IP[1]), tail(IP), tail(I)) end function compute_linindex(f, s, IP::Tuple, I::Tuple{Any, Vararg{Any}}) @_inline_meta Δi = first(I[1])-first(IP[1]) - compute_linindex(f + Δi*s, s*unsafe_length(IP[1]), tail(IP), tail(I)) + compute_linindex(f + Δi*s, s*length(IP[1]), tail(IP), tail(I)) end compute_linindex(f, s, IP::Tuple, I::Tuple{}) = f @@ -447,5 +447,5 @@ _indices_sub(::Real, I...) = (@_inline_meta; _indices_sub(I...)) _indices_sub() = () function _indices_sub(i1::AbstractArray, I...) @_inline_meta - (unsafe_indices(i1)..., _indices_sub(I...)...) + (axes(i1)..., _indices_sub(I...)...) end diff --git a/stdlib/Dates/test/ranges.jl b/stdlib/Dates/test/ranges.jl index 6eb6371376867..52416fc95ec0c 100644 --- a/stdlib/Dates/test/ranges.jl +++ b/stdlib/Dates/test/ranges.jl @@ -515,7 +515,7 @@ end @test length(Dates.Year(1):Dates.Year(1):Dates.Year(10)) == 10 @test length(Dates.Year(10):Dates.Year(-1):Dates.Year(1)) == 10 @test length(Dates.Year(10):Dates.Year(-2):Dates.Year(1)) == 5 -@test_throws OverflowError length(typemin(Dates.Year):Dates.Year(1):typemax(Dates.Year)) +@test length(typemin(Dates.Year):Dates.Year(1):typemax(Dates.Year)) == 0 # overflow @test_throws MethodError Dates.Date(0):Dates.DateTime(2000) @test_throws MethodError Dates.Date(0):Dates.Year(10) @test length(range(Dates.Date(2000), step=Dates.Day(1), length=366)) == 366 diff --git a/test/ranges.jl b/test/ranges.jl index b06bae88b02a7..c37011cf561f3 100644 --- a/test/ranges.jl +++ b/test/ranges.jl @@ -499,19 +499,28 @@ end @test length(1:4:typemax(Int)) == div(typemax(Int),4) + 1 @testset "overflow in length" begin - Tset = Int === Int64 ? (Int,UInt,Int128,UInt128) : - (Int,UInt,Int64,UInt64,Int128, UInt128) + Tset = Int === Int64 ? (Int, UInt, Int128, UInt128) : + (Int, UInt, Int64, UInt64, Int128, UInt128) for T in Tset - @test_throws OverflowError length(zero(T):typemax(T)) - @test_throws OverflowError length(typemin(T):typemax(T)) - @test_throws OverflowError length(zero(T):one(T):typemax(T)) - @test_throws OverflowError length(typemin(T):one(T):typemax(T)) + @test length(zero(T):typemax(T)) == typemin(T) + @test length(typemin(T):typemax(T)) == T(0) + @test length(zero(T):one(T):typemax(T)) == typemin(T) + @test length(typemin(T):one(T):typemax(T)) == T(0) + @test length(one(T):typemax(T)) == typemax(T) if T <: Signed - @test_throws OverflowError length(-one(T):typemax(T)-one(T)) - @test_throws OverflowError length(-one(T):one(T):typemax(T)-one(T)) + @test length(-one(T):typemax(T)-one(T)) == typemin(T) + @test length(-one(T):one(T):typemax(T)-one(T)) == typemin(T) + @test length(-one(T):typemax(T)) == typemin(T) + T(1) + @test length(zero(T):typemin(T):typemin(T)) == 2 + @test length(one(T):typemin(T):typemin(T)) == 2 + @test length(typemax(T):typemin(T):typemin(T)) == 2 + @test length(-one(T):typemin(T):typemin(T)) == 1 + @test length(zero(T):typemin(T):zero(T)) == 1 + @test length(zero(T):typemin(T):one(T)) == 0 end end end + @testset "loops involving typemin/typemax" begin n = 0 s = 0 @@ -866,17 +875,18 @@ end end # issue #2959 @test 1.0:1.5 == 1.0:1.0:1.5 == 1.0:1.0 -#@test 1.0:(.3-.1)/.1 == 1.0:2.0 +@test_broken 1.0:(.3-.1)/.1 == 1.0:2.0 # (this is just shy of 2.0) @testset "length with typemin/typemax" begin - let r = typemin(Int64):2:typemax(Int64), s = typemax(Int64):-2:typemin(Int64) + let r = typemin(Int64):2:typemax(Int64) @test first(r) == typemin(Int64) - @test last(r) == (typemax(Int64)-1) - @test_throws OverflowError length(r) - + @test last(r) == typemax(Int64) - 1 + @test length(r) == typemin(Int64) + end + let s = typemax(Int64):-2:typemin(Int64) @test first(s) == typemax(Int64) - @test last(s) == (typemin(Int64)+1) - @test_throws OverflowError length(s) + @test last(s) == typemin(Int64) + 1 + @test length(s) == typemin(Int64) end @test length(typemin(Int64):3:typemax(Int64)) == 6148914691236517206 @@ -892,6 +902,7 @@ end @test length(typemax(UInt):UInt(2):(typemax(UInt)-1)) == 0 @test length((typemin(Int)+3):5:(typemin(Int)+1)) == 0 end + # issue #6364 @test length((1:64)*(pi/5)) == 64 @@ -1042,13 +1053,9 @@ end @test reverse(LinRange{Int}(0,3,4)) === LinRange{Int}(3,0,4) @test reverse(LinRange{Float64}(0.,3.,4)) === LinRange{Float64}(3.,0.,4) end -@testset "Issue #11245" begin - io = IOBuffer() - show(io, range(1, stop=2, length=3)) - str = String(take!(io)) -# @test str == "range(1.0, stop=2.0, length=3)" - @test str == "1.0:0.5:2.0" -end + +# issue #11245 +@test repr(range(1, stop=2, length=3)) == "1.0:0.5:2.0" @testset "issue 10950" begin r = 1//2:3 @@ -1263,8 +1270,11 @@ end end r = 1f8-10:1f8 - @test_broken argmin(f) == argmin(collect(r)) - @test_broken argmax(f) == argmax(collect(r)) + rv = collect(r) + @test argmin(r) == argmin(rv) == 1 + @test r[argmax(r)] == r[argmax(rv)] == 1f8 + @test argmax(r) == lastindex(r) + @test argmax(rv) != lastindex(r) end @testset "OneTo" begin @@ -1516,7 +1526,7 @@ Base.Unsigned(x::Displacement) = Unsigned(x.val) Base.rem(x::Displacement, y::Displacement) = Displacement(rem(x.val, y.val)) Base.div(x::Displacement, y::Displacement) = Displacement(div(x.val, y.val)) -# required for collect (summing lengths); alternatively, should unsafe_length return Int by default? +# required for collect (summing lengths); alternatively, should length return Int by default? Base.promote_rule(::Type{Displacement}, ::Type{Int}) = Int Base.convert(::Type{Int}, x::Displacement) = x.val diff --git a/test/testhelpers/Furlongs.jl b/test/testhelpers/Furlongs.jl index 73d23a39d2d7b..f3583a532215a 100644 --- a/test/testhelpers/Furlongs.jl +++ b/test/testhelpers/Furlongs.jl @@ -20,7 +20,7 @@ Furlong{p}(x::Furlong{q}) where {p,q} = (@assert(p==q); Furlong{p,typeof(x.val)} Furlong{p,T}(x::Furlong{q}) where {T,p,q} = (@assert(p==q); Furlong{p,T}(T(x.val))) Base.promote_type(::Type{Furlong{p,T}}, ::Type{Furlong{p,S}}) where {p,T,S} = - (Base.@_pure_meta; Furlong{p,promote_type(T,S)}) + Furlong{p,promote_type(T,S)} Base.one(x::Furlong{p,T}) where {p,T} = one(T) Base.one(::Type{Furlong{p,T}}) where {p,T} = one(T) @@ -38,14 +38,12 @@ Base.floatmax(::Type{Furlong{p,T}}) where {p,T<:AbstractFloat} = Furlong{p}(floa Base.floatmax(::Furlong{p,T}) where {p,T<:AbstractFloat} = floatmax(Furlong{p,T}) Base.conj(x::Furlong{p,T}) where {p,T} = Furlong{p,T}(conj(x.val)) -# convert Furlong exponent p to a canonical form. This -# is not type stable, but it doesn't matter since it is used -# at compile time (in generated functions), not runtime +# convert Furlong exponent p to a canonical form canonical_p(p) = isinteger(p) ? Int(p) : Rational{Int}(p) Base.abs(x::Furlong{p}) where {p} = Furlong{p}(abs(x.val)) -@generated Base.abs2(x::Furlong{p}) where {p} = :(Furlong{$(canonical_p(2p))}(abs2(x.val))) -@generated Base.inv(x::Furlong{p}) where {p} = :(Furlong{$(canonical_p(-p))}(inv(x.val))) +Base.abs2(x::Furlong{p}) where {p} = Furlong{canonical_p(2p)}(abs2(x.val)) +Base.inv(x::Furlong{p}) where {p} = Furlong{canonical_p(-p)}(inv(x.val)) for f in (:isfinite, :isnan, :isreal, :isinf) @eval Base.$f(x::Furlong) = $f(x.val) @@ -64,11 +62,10 @@ end for op in (:(==), :(!=), :<, :<=, :isless, :isequal) @eval $op(x::Furlong{p}, y::Furlong{p}) where {p} = $op(x.val, y.val) end -# generated functions to allow type inference of the value of the exponent: for (f,op) in ((:_plus,:+),(:_minus,:-),(:_times,:*),(:_div,://)) - @eval @generated function $f(v::T, ::Furlong{p}, ::Union{Furlong{q},Val{q}}) where {T,p,q} + @eval function $f(v::T, ::Furlong{p}, ::Union{Furlong{q},Val{q}}) where {T,p,q} s = $op(p, q) - :(Furlong{$(canonical_p(s)),$T}(v)) + Furlong{canonical_p(s),T}(v) end end for (op,eop) in ((:*, :_plus), (:/, :_minus), (://, :_minus), (:div, :_minus)) diff --git a/test/testhelpers/InfiniteArrays.jl b/test/testhelpers/InfiniteArrays.jl index bc6de1afc5503..d69130f4d726a 100644 --- a/test/testhelpers/InfiniteArrays.jl +++ b/test/testhelpers/InfiniteArrays.jl @@ -39,13 +39,11 @@ struct OneToInf{T<:Integer} <: AbstractUnitRange{T} end OneToInf() = OneToInf{Int}() Base.axes(r::OneToInf) = (r,) -Base.unsafe_indices(r::OneToInf) = (r,) -Base.unsafe_length(r::OneToInf) = Infinity() Base.size(r::OneToInf) = (Infinity(),) Base.first(r::OneToInf{T}) where {T} = oneunit(T) -Base.length(r::OneToInf{T}) where {T} = Infinity() -Base.last(r::OneToInf{T}) where {T} = Infinity() +Base.length(r::OneToInf) = Infinity() +Base.last(r::OneToInf) = Infinity() Base.unitrange(r::OneToInf) = r Base.oneto(::Infinity) = OneToInf() -end \ No newline at end of file +end diff --git a/test/testhelpers/OffsetArrays.jl b/test/testhelpers/OffsetArrays.jl index 67de3ef476652..a4a6b5eba7f85 100644 --- a/test/testhelpers/OffsetArrays.jl +++ b/test/testhelpers/OffsetArrays.jl @@ -68,7 +68,7 @@ offset_coerce(::Type{I}, r::AbstractUnitRange) where I<:AbstractUnitRange{T} whe @inline Base.parent(r::IdOffsetRange) = r.parent @inline Base.axes(r::IdOffsetRange) = (Base.axes1(r),) @inline Base.axes1(r::IdOffsetRange) = IdOffsetRange(Base.axes1(r.parent), r.offset) -@inline Base.unsafe_indices(r::IdOffsetRange) = (r,) +#@inline Base.unsafe_indices(r::IdOffsetRange) = (r,) # why did this assume first(r.parent) == 1 previously?? @inline Base.length(r::IdOffsetRange) = length(r.parent) Base.reduced_index(i::IdOffsetRange) = typeof(i)(first(i):first(i)) # Workaround for #92 on Julia < 1.4 From 96a1e5ff98d6a0592ed041aeb94aa2a8bd4edc00 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Tue, 8 Jun 2021 23:32:46 -0400 Subject: [PATCH 2/3] fix method discrepencies --- NEWS.md | 4 + base/checked.jl | 2 +- base/range.jl | 32 +++++-- doc/src/base/collections.md | 1 + test/ranges.jl | 165 +++++++++++++++++++++++++++--------- 5 files changed, 158 insertions(+), 46 deletions(-) diff --git a/NEWS.md b/NEWS.md index c95d6896388c3..7a83ad5bb573a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -143,6 +143,10 @@ Standard library changes * `replace(::String)` now accepts multiple patterns, which will be applied left-to-right simultaneously, so only one pattern will be applied to any character, and the patterns will only be applied to the input text, not the replacements ([#40484]). +* The `length` function on certain ranges of certain specific element types no longer checks for integer + overflow in most cases. The new function `checked_length` is now available, which will try to use checked + arithmetic to error if the result may be wrapping. Or use a package such as SaferIntegers.jl when + constructing the range. ([#40382]) #### Package Manager diff --git a/base/checked.jl b/base/checked.jl index 65f3d4234045e..ba23d4c5acd2b 100644 --- a/base/checked.jl +++ b/base/checked.jl @@ -353,7 +353,7 @@ checked_cld(x::T, y::T) where {T<:Integer} = cld(x, y) # Base.cld already checks Base.checked_length(r) Calculates `length(r)`, but may check for overflow errors where applicable when -the result doesn't fit into `Union{eltype(r),Int}`. +the result doesn't fit into `Union{Integer(eltype(r)),Int}`. """ checked_length(r) = length(r) # for most things, length doesn't error diff --git a/base/range.jl b/base/range.jl index 4c4237cad0637..e7cd7b4a51655 100644 --- a/base/range.jl +++ b/base/range.jl @@ -640,7 +640,15 @@ function checked_length(r::OrdinalRange{T}) where T if s == zero(s) || isempty(r) return Integer(start - start + zero(s)) end - return Integer(div(checked_add(checked_sub(last(r), start), s, s))) + stop = last(r) + if isless(s, zero(s)) + diff = checked_sub(start, stop) + s = -s + else + diff = checked_sub(stop, start) + end + a = Integer(div(diff, s)) + return checked_add(a, one(a)) end function checked_length(r::AbstractUnitRange{T}) where T @@ -649,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 a + one(a) + return checked_add(a, one(a)) end function length(r::OrdinalRange{T}) where T @@ -659,9 +667,18 @@ function length(r::OrdinalRange{T}) where T if s == zero(s) || isempty(r) return Integer(start - start + zero(s)) end - return Integer(div(last(r) - start + s, s)) + stop = last(r) + if isless(s, zero(s)) + diff = start - stop + s = -s + else + diff = stop - start + end + a = Integer(div(diff, s)) + return a + one(a) end + function length(r::AbstractUnitRange{T}) where T @_inline_meta a = Integer(last(r) - first(r)) # even when isempty, by construction (with overflow) @@ -673,7 +690,7 @@ length(r::StepRangeLen) = r.len length(r::LinRange) = r.len let bigints = Union{Int, UInt, Int64, UInt64, Int128, UInt128} - global length + global length, checked_length # compile optimization for which promote_type(T, Int) == T length(r::OneTo{T}) where {T<:bigints} = r.stop # slightly more accurate length and checked_length in extreme cases @@ -720,11 +737,14 @@ end let smallints = (Int === Int64 ? Union{Int8, UInt8, Int16, UInt16, Int32, UInt32} : Union{Int8, UInt8, Int16, UInt16}) - global length + global length, checked_length # n.b. !(step isa T) - length(r::OrdinalRange{<:smallints}) = div(Int(last(r)) - Int(first(r)) + step(r), step(r)) + length(r::OrdinalRange{<:smallints}) = div(Int(last(r)) - Int(first(r)), step(r)) + 1 length(r::AbstractUnitRange{<:smallints}) = Int(last(r)) - Int(first(r)) + 1 length(r::OneTo{<:smallints}) = Int(r.stop) + checked_length(r::OrdinalRange{<:smallints}) = length(r) + checked_length(r::AbstractUnitRange{<:smallints}) = length(r) + checked_length(r::OneTo{<:smallints}) = length(r) end first(r::OrdinalRange{T}) where {T} = convert(T, r.start) diff --git a/doc/src/base/collections.md b/doc/src/base/collections.md index f8ef12071171a..84e5702e0e396 100644 --- a/doc/src/base/collections.md +++ b/doc/src/base/collections.md @@ -66,6 +66,7 @@ Base.LinRange Base.isempty Base.empty! Base.length +Base.checked_length ``` Fully implemented by: diff --git a/test/ranges.jl b/test/ranges.jl index c37011cf561f3..4486a5087586c 100644 --- a/test/ranges.jl +++ b/test/ranges.jl @@ -1,5 +1,7 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license +using Base.Checked: checked_length + @testset "range construction" begin @test_throws ArgumentError range(start=1, step=1, stop=2, length=10) @test_throws ArgumentError range(start=1, step=1, stop=10, length=11) @@ -267,22 +269,28 @@ end end end @testset "length" begin - @test length(.1:.1:.3) == 3 - @test length(1.1:1.1:3.3) == 3 - @test length(1.1:1.3:3) == 2 - @test length(1:1:1.8) == 1 - @test length(1:.2:2) == 6 - @test length(1.:.2:2.) == 6 - @test length(2:-.2:1) == 6 - @test length(2.:-.2:1.) == 6 - @test length(2:.2:1) == 0 + @test length(.1:.1:.3) == checked_length(.1:.1:.3) == 3 + @test length(1.1:1.1:3.3) == checked_length(1.1:1.1:3.3) == 3 + @test length(1.1:1.3:3) == checked_length(1.1:1.3:3) == 2 + @test length(1:1:1.8) == checked_length(1:1:1.8) == 1 + @test length(1:.2:2) == checked_length(1:.2:2) == 6 + @test length(1.:.2:2.) == checked_length(1.:.2:2.) == 6 + @test length(2:-.2:1) == checked_length(2:-.2:1) == 6 + @test length(2.:-.2:1.) == checked_length(2.:-.2:1.) == 6 + @test length(2:.2:1) == checked_length(2:.2:1) == 0 @test length(2.:.2:1.) == 0 - @test length(1:0) == 0 - @test length(0.0:-0.5) == 0 - @test length(1:2:0) == 0 - @test length(Char(0):Char(0x001fffff)) == 2097152 - @test length(typemax(UInt64)//one(UInt64):1:typemax(UInt64)//one(UInt64)) == 1 + @test length(1:0) == checked_length(1:0) == 0 + @test length(0.0:-0.5) == checked_length(0.0:-0.5) == 0 + @test length(1:2:0) == checked_length(1:2:0) == 0 + let r = Char(0):Char(0x001fffff) + @test length(r) == 2097152 + @test_throws MethodError checked_length(r) == 2097152 # this would work if checked_sub is defined on Char + end + let r = typemax(UInt64)//one(UInt64):1:typemax(UInt64)//one(UInt64) + @test length(r) == 1 + @test_throws MethodError checked_length(r) == 1 # this would work if checked_sub is defined on Rational + end end @testset "keys/values" begin keytype_is_correct(r) = keytype(r) == eltype(keys(r)) @@ -496,7 +504,8 @@ for a=AbstractRange[3:6, 0:2:10], b=AbstractRange[0:1, 2:-1:0] end # avoiding intermediate overflow (#5065) -@test length(1:4:typemax(Int)) == div(typemax(Int),4) + 1 +@test length(1:4:typemax(Int)) == div(typemax(Int), 4) + 1 +@test checked_length(1:4:typemax(Int)) == div(typemax(Int), 4) + 1 # computed exactly in modulo arithmetic @testset "overflow in length" begin Tset = Int === Int64 ? (Int, UInt, Int128, UInt128) : @@ -506,7 +515,11 @@ end @test length(typemin(T):typemax(T)) == T(0) @test length(zero(T):one(T):typemax(T)) == typemin(T) @test length(typemin(T):one(T):typemax(T)) == T(0) - @test length(one(T):typemax(T)) == typemax(T) + @test_throws OverflowError checked_length(zero(T):typemax(T)) + @test_throws OverflowError checked_length(typemin(T):typemax(T)) + @test_throws OverflowError checked_length(zero(T):one(T):typemax(T)) + @test_throws OverflowError checked_length(typemin(T):one(T):typemax(T)) + @test length(one(T):typemax(T)) == checked_length(one(T):typemax(T)) == typemax(T) if T <: Signed @test length(-one(T):typemax(T)-one(T)) == typemin(T) @test length(-one(T):one(T):typemax(T)-one(T)) == typemin(T) @@ -517,6 +530,11 @@ end @test length(-one(T):typemin(T):typemin(T)) == 1 @test length(zero(T):typemin(T):zero(T)) == 1 @test length(zero(T):typemin(T):one(T)) == 0 + @test_throws OverflowError checked_length(-one(T):typemax(T)-one(T)) + @test_throws OverflowError checked_length(-one(T):one(T):typemax(T)-one(T)) + @test_throws InexactError checked_length(zero(T):typemin(T):typemin(T)) == 2 # this can be improved + @test_throws InexactError checked_length(one(T):typemin(T):typemin(T)) == 2 # this can be improved + @test_throws InexactError checked_length(typemax(T):typemin(T):typemin(T)) == 2 # this can be improved end end end @@ -882,25 +900,36 @@ end @test first(r) == typemin(Int64) @test last(r) == typemax(Int64) - 1 @test length(r) == typemin(Int64) + @test_throws OverflowError checked_length(r) end - let s = typemax(Int64):-2:typemin(Int64) - @test first(s) == typemax(Int64) - @test last(s) == typemin(Int64) + 1 - @test length(s) == typemin(Int64) + let r = typemax(Int64):-2:typemin(Int64) + @test first(r) == typemax(Int64) + @test last(r) == typemin(Int64) + 1 + @test length(r) == typemin(Int64) + @test_throws OverflowError checked_length(r) end - @test length(typemin(Int64):3:typemax(Int64)) == 6148914691236517206 - @test length(typemax(Int64):-3:typemin(Int64)) == 6148914691236517206 + let r = typemin(Int64):3:typemax(Int64) + @test length(r) == checked_length(r) == 6148914691236517206 + end + let r = typemax(Int64):-3:typemin(Int64) + @test length(r) == checked_length(r) == 6148914691236517206 + end for s in 3:100 - @test length(typemin(Int):s:typemax(Int)) == length(big(typemin(Int)):big(s):big(typemax(Int))) - @test length(typemax(Int):-s:typemin(Int)) == length(big(typemax(Int)):big(-s):big(typemin(Int))) + r = typemin(Int):s:typemax(Int) + br = big(typemin(Int)):big(s):big(typemax(Int)) + @test length(r) == checked_length(r) == length(br) + + r = typemax(Int):-s:typemin(Int) + br = big(typemax(Int)):big(-s):big(typemin(Int)) + @test length(r) == checked_length(r) == length(br) end - @test length(UInt(1):UInt(1):UInt(0)) == 0 - @test length(typemax(UInt):UInt(1):(typemax(UInt)-1)) == 0 - @test length(typemax(UInt):UInt(2):(typemax(UInt)-1)) == 0 - @test length((typemin(Int)+3):5:(typemin(Int)+1)) == 0 + @test length(UInt(1):UInt(1):UInt(0)) == checked_length(UInt(1):UInt(1):UInt(0)) == 0 + @test length(typemax(UInt):UInt(1):(typemax(UInt)-1)) == checked_length(typemax(UInt):UInt(1):(typemax(UInt)-1)) == 0 + @test length(typemax(UInt):UInt(2):(typemax(UInt)-1)) == checked_length(typemax(UInt):UInt(2):(typemax(UInt)-1)) == 0 + @test length((typemin(Int)+3):5:(typemin(Int)+1)) == checked_length((typemin(Int)+3):5:(typemin(Int)+1)) == 0 end # issue #6364 @@ -972,7 +1001,8 @@ end (Int8,UInt8,Int16,UInt16,Int32,UInt32) : (Int8,UInt8,Int16,UInt16)) for T in smallint - @test length(typemin(T):typemax(T)) == 2^(8*sizeof(T)) + s = typemin(T):typemax(T) + @test length(s) == checked_length(s) == 2^(8*sizeof(T)) end end @@ -980,7 +1010,7 @@ end @test (0:1//2:2)[1:2:3] == 0:1//1:1 # issue #12278 -@test length(1:UInt(0)) == 0 +@test length(1:UInt(0)) == checked_length(1:UInt(0)) == 0 @testset "zip" begin i = 0 @@ -1060,6 +1090,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 i = 1 for x in r @test x == i//2 @@ -1280,12 +1311,12 @@ end @testset "OneTo" begin let r = Base.OneTo(-5) @test isempty(r) - @test length(r) == 0 + @test length(r) == checked_length(r) == 0 @test size(r) == (0,) end let r = Base.OneTo(3) @test !isempty(r) - @test length(r) == 3 + @test length(r) == checked_length(r) == 3 @test size(r) == (3,) @test step(r) == 1 @test first(r) == 1 @@ -1382,7 +1413,7 @@ end @testset "issue #20520" begin r = range(1.3173739f0, stop=1.3173739f0, length=3) - @test length(r) == 3 + @test length(r) == checked_length(r) == 3 @test first(r) === 1.3173739f0 @test last(r) === 1.3173739f0 @test r[2] === 1.3173739f0 @@ -1406,7 +1437,7 @@ using .Main.Furlongs @testset "dimensional correctness" begin @test length(Vector(Furlong(2):Furlong(10))) == 9 - @test length(range(Furlong(2), length=9)) == 9 + @test length(range(Furlong(2), length=9)) == checked_length(range(Furlong(2), length=9)) == 9 @test Vector(Furlong(2):Furlong(1):Furlong(10)) == Vector(range(Furlong(2), step=Furlong(1), length=9)) == Furlong.(2:10) @test Vector(Furlong(1.0):Furlong(0.5):Furlong(10.0)) == Vector(Furlong(1):Furlong(0.5):Furlong(10)) == Furlong.(1:0.5:10) @@ -1501,15 +1532,18 @@ module NonStandardIntegerRangeTest using Test +using Base.Checked: checked_length +import Base.Checked: checked_add, checked_sub + struct Position <: Integer val::Int end -Position(x::Position) = x # to resolve ambiguity with boot.jl:728 +Position(x::Position) = x # to resolve ambiguity with boot.jl:770 struct Displacement <: Integer val::Int end -Displacement(x::Displacement) = x # to resolve ambiguity with boot.jl:728 +Displacement(x::Displacement) = x # to resolve ambiguity with boot.jl:770 Base.:-(x::Displacement) = Displacement(-x.val) Base.:-(x::Position, y::Position) = Displacement(x.val - y.val) @@ -1530,10 +1564,63 @@ Base.div(x::Displacement, y::Displacement) = Displacement(div(x.val, y.val)) Base.promote_rule(::Type{Displacement}, ::Type{Int}) = Int Base.convert(::Type{Int}, x::Displacement) = x.val +# Unsigned complement, for testing checked_length +struct UPosition <: Unsigned + val::UInt +end +UPosition(x::UPosition) = x # to resolve ambiguity with boot.jl:770 + +struct UDisplacement <: Unsigned + val::UInt +end +UDisplacement(x::UDisplacement) = x # to resolve ambiguity with boot.jl:770 + +Base.show(io::IO, x::Union{Position, UPosition, Displacement, UDisplacement}) = + # should use show if we were to do this properly (instead of just a test-helper) + print(io, typeof(x).name.name, "(", x.val, ")") + +Base.:-(x::UPosition, y::UPosition) = UDisplacement(x.val - y.val) +Base.:-(x::UPosition, y::UDisplacement) = UPosition(x.val - y.val) +Base.:+(x::UPosition, y::UDisplacement) = UPosition(x.val + y.val) +Base.:+(x::UDisplacement, y::Displacement) = UDisplacement(x.val + y.val) +Base.:+(x::UDisplacement, y::UDisplacement) = UDisplacement(x.val + y.val) +checked_sub(x::UPosition, y::UPosition) = UDisplacement(checked_sub(x.val, y.val)) +checked_sub(x::UPosition, y::UDisplacement) = UPosition(checked_sub(x.val, y.val)) +checked_sub(x::UDisplacement, y::UDisplacement) = UDisplacement(checked_sub(x.val, y.val)) +checked_add(x::UPosition, y::UDisplacement) = UPosition(checked_add(x.val, y.val)) +checked_add(x::UDisplacement, y::UDisplacement) = UDisplacement(checked_add(x.val, y.val)) +Base.:+(x::UPosition, y::Displacement) = UPosition(x.val + y.val) +Base.:(<=)(x::UPosition, y::UPosition) = x.val <= y.val +Base.:(<)(x::UPosition, y::UPosition) = x.val < y.val +Base.:(<)(x::UDisplacement, y::UDisplacement) = x.val < y.val + +# for StepRange computation: +Base.rem(x::UDisplacement, y::Displacement) = UDisplacement(rem(x.val, y.val)) +Base.div(x::UDisplacement, y::Displacement) = UDisplacement(div(x.val, y.val)) +Base.rem(x::UDisplacement, y::UDisplacement) = UDisplacement(rem(x.val, y.val)) +Base.div(x::UDisplacement, y::UDisplacement) = UDisplacement(div(x.val, y.val)) + +#Base.promote_rule(::Type{UDisplacement}, ::Type{Int}) = Int +#Base.convert(::Type{Int}, x::UDisplacement) = Int(x.val) + @testset "Ranges with nonstandard Integers" begin for (start, stop) in [(2, 4), (3, 3), (3, -2)] - @test collect(Position(start) : Position(stop)) == Position.(start : stop) - end + r = Position(start) : Position(stop) + @test length(r) === Displacement(stop >= start ? stop - start + 1 : 0) + start >= 0 && stop >= 0 && @test UDisplacement(length(r).val) === + checked_length(UPosition(start) : UPosition(stop)) === + checked_length(UPosition(start) : Displacement(1) : UPosition(stop)) === + checked_length(UPosition(start) : UDisplacement(1) : UPosition(stop)) + @test collect(r) == Position.(start : stop) + end + + @test length(UPosition(3):Displacement(7):UPosition(100)) === checked_length(UPosition(3):Displacement(7):UPosition(100)) === UDisplacement(14) + @test length(UPosition(100):Displacement(7):UPosition(3)) === checked_length(UPosition(100):Displacement(7):UPosition(3)) === UDisplacement(0) + @test length(UPosition(100):Displacement(-7):UPosition(3)) === checked_length(UPosition(100):Displacement(-7):UPosition(3)) === UDisplacement(14) + @test length(UPosition(3):Displacement(-7):UPosition(100)) === checked_length(UPosition(3):Displacement(-7):UPosition(100)) === UDisplacement(0) + @test_throws OverflowError checked_length(zero(UPosition):UPosition(typemax(UInt))) + @test_throws OverflowError checked_length(zero(UPosition):Displacement(1):UPosition(typemax(UInt))) + @test_throws OverflowError checked_length(UPosition(typemax(UInt)):Displacement(-1):zero(UPosition)) for start in [3, 0, -2] @test collect(Base.OneTo(Position(start))) == Position.(Base.OneTo(start)) @@ -1555,7 +1642,7 @@ end end # module NonStandardIntegerRangeTest @testset "Issue #26619" begin - @test length(UInt(100) : -1 : 1) === UInt(100) + @test length(UInt(100) : -1 : 1) == checked_length(UInt(100) : -1 : 1) === UInt(100) @test collect(UInt(5) : -1 : 3) == [UInt(5), UInt(4), UInt(3)] let r = UInt(5) : -2 : 2 From 7bb6ee15d8470c28f5941456ad22c28bdaa55bb3 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Mon, 21 Jun 2021 16:45:33 -0400 Subject: [PATCH 3/3] Update test/testhelpers/OffsetArrays.jl --- test/testhelpers/OffsetArrays.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/test/testhelpers/OffsetArrays.jl b/test/testhelpers/OffsetArrays.jl index a4a6b5eba7f85..2f650808a12f2 100644 --- a/test/testhelpers/OffsetArrays.jl +++ b/test/testhelpers/OffsetArrays.jl @@ -68,7 +68,6 @@ offset_coerce(::Type{I}, r::AbstractUnitRange) where I<:AbstractUnitRange{T} whe @inline Base.parent(r::IdOffsetRange) = r.parent @inline Base.axes(r::IdOffsetRange) = (Base.axes1(r),) @inline Base.axes1(r::IdOffsetRange) = IdOffsetRange(Base.axes1(r.parent), r.offset) -#@inline Base.unsafe_indices(r::IdOffsetRange) = (r,) # why did this assume first(r.parent) == 1 previously?? @inline Base.length(r::IdOffsetRange) = length(r.parent) Base.reduced_index(i::IdOffsetRange) = typeof(i)(first(i):first(i)) # Workaround for #92 on Julia < 1.4