Skip to content

Commit c33aad8

Browse files
jishnubKristofferC
authored andcommitted
Array(::AbstractRange) should return an Array (#50568)
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. (cherry picked from commit 3cc0590)
1 parent e22d308 commit c33aad8

File tree

2 files changed

+33
-9
lines changed

2 files changed

+33
-9
lines changed

base/range.jl

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1376,8 +1376,21 @@ function vcat(rs::AbstractRange{T}...) where T
13761376
return a
13771377
end
13781378

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

13821395
_reverse(r::OrdinalRange, ::Colon) = (:)(last(r), negate(step(r)), first(r))
13831396
function _reverse(r::StepRangeLen, ::Colon)

test/ranges.jl

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1802,6 +1802,7 @@ Base.div(x::Displacement, y::Displacement) = Displacement(div(x.val, y.val))
18021802
# required for collect (summing lengths); alternatively, should length return Int by default?
18031803
Base.promote_rule(::Type{Displacement}, ::Type{Int}) = Int
18041804
Base.convert(::Type{Int}, x::Displacement) = x.val
1805+
Base.Int(x::Displacement) = x.val
18051806

18061807
# Unsigned complement, for testing checked_length
18071808
struct UPosition <: Unsigned
@@ -2491,11 +2492,21 @@ end
24912492
@test Core.Compiler.is_foldable(Base.infer_effects(check_ranges, (UnitRange{Int},UnitRange{Int})))
24922493
# TODO JET.@test_opt check_ranges(1:2, 3:4)
24932494

2494-
@testset "isassigned" begin
2495-
for (r, val) in ((1:3, 3), (1:big(2)^65, big(2)^65))
2496-
@test isassigned(r, lastindex(r))
2497-
# test that the indexing actually succeeds
2498-
@test r[end] == val
2499-
@test_throws ArgumentError isassigned(r, true)
2500-
end
2495+
@testset "collect with specialized vcat" begin
2496+
struct OneToThree <: AbstractUnitRange{Int} end
2497+
Base.size(r::OneToThree) = (3,)
2498+
Base.first(r::OneToThree) = 1
2499+
Base.length(r::OneToThree) = 3
2500+
Base.last(r::OneToThree) = 3
2501+
function Base.getindex(r::OneToThree, i::Int)
2502+
checkbounds(r, i)
2503+
i
2504+
end
2505+
Base.vcat(r::OneToThree) = r
2506+
r = OneToThree()
2507+
a = Array(r)
2508+
@test a isa Vector{Int}
2509+
@test a == r
2510+
@test collect(r) isa Vector{Int}
2511+
@test collect(r) == r
25012512
end

0 commit comments

Comments
 (0)