Skip to content

Commit

Permalink
Array(::AbstractRange) should return an Array (#50568)
Browse files Browse the repository at this point in the history
Currently, `Array(r::AbstractRange)` falls back to `vcat(r)`, but
certain ranges may choose to specialize `vcat(r::AbstractRange)` to not
return an `Array`. This PR ensures that `Array(r)` always returns an
`Array`.

At present, there's some code overlap with `vcat` (just above the
`Array` method added in this PR). Perhaps some of these may be replaced
by `unsafe_copyto!`, but the tests for ranges include some special cases
that don't support `getindex`, which complicates things a bit. I've not
done this for now. In any case, the common bit of code is pretty simple,
so perhaps the duplication is harmless.
  • Loading branch information
jishnub authored Jul 24, 2023
1 parent 76a9772 commit 3cc0590
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
17 changes: 15 additions & 2 deletions base/range.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1380,8 +1380,21 @@ function vcat(rs::AbstractRange{T}...) where T
return a
end

Array{T,1}(r::AbstractRange{T}) where {T} = vcat(r)
collect(r::AbstractRange) = vcat(r)
# This method differs from that for AbstractArrays as it
# use iteration instead of indexing. This works even if certain
# non-standard ranges don't support indexing.
# See https://github.com/JuliaLang/julia/pull/27302
# Similarly, collect(r::AbstractRange) uses iteration
function Array{T,1}(r::AbstractRange{T}) where {T}
a = Vector{T}(undef, length(r))
i = 1
for x in r
@inbounds a[i] = x
i += 1
end
return a
end
collect(r::AbstractRange) = Array(r)

_reverse(r::OrdinalRange, ::Colon) = (:)(last(r), negate(step(r)), first(r))
function _reverse(r::StepRangeLen, ::Colon)
Expand Down
21 changes: 21 additions & 0 deletions test/ranges.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1802,6 +1802,7 @@ Base.div(x::Displacement, y::Displacement) = Displacement(div(x.val, y.val))
# 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
Base.Int(x::Displacement) = x.val

# Unsigned complement, for testing checked_length
struct UPosition <: Unsigned
Expand Down Expand Up @@ -2499,11 +2500,31 @@ end
@test (-10:2:typemax(Int))[typemax(Int)÷2+2] == typemax(Int)-9
end

@testset "collect with specialized vcat" begin
struct OneToThree <: AbstractUnitRange{Int} end
Base.size(r::OneToThree) = (3,)
Base.first(r::OneToThree) = 1
Base.length(r::OneToThree) = 3
Base.last(r::OneToThree) = 3
function Base.getindex(r::OneToThree, i::Int)
checkbounds(r, i)
i
end
Base.vcat(r::OneToThree) = r
r = OneToThree()
a = Array(r)
@test a isa Vector{Int}
@test a == r
@test collect(r) isa Vector{Int}
@test collect(r) == r
end

@testset "isassigned" begin
for (r, val) in ((1:3, 3), (1:big(2)^65, big(2)^65))
@test isassigned(r, lastindex(r))
# test that the indexing actually succeeds
@test r[end] == val
@test_throws ArgumentError isassigned(r, true)
end

end

0 comments on commit 3cc0590

Please sign in to comment.