Skip to content

Commit e39e77f

Browse files
authored
Fix unaliascopy(::SubArray) with indices of Array{<:CartesianIndex} (#47779)
This PR fixes the bug caused by the trimming trick. `Base.index_lengths` is not a proper tool to calculate the trimmed shape as `indices` might consume more than 1 dim. And we can avoid the unneeded "`repeat`" when we meet `Array{CartesianIndex{0}}` Test added. Close #47644.
1 parent b71523e commit e39e77f

File tree

2 files changed

+47
-9
lines changed

2 files changed

+47
-9
lines changed

base/subarray.jl

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -110,16 +110,41 @@ unaliascopy(A::SubArray) = typeof(A)(unaliascopy(A.parent), map(unaliascopy, A.i
110110

111111
# When the parent is an Array we can trim the size down a bit. In the future this
112112
# could possibly be extended to any mutable array.
113-
function unaliascopy(V::SubArray{T,N,A,I,LD}) where {T,N,A<:Array,I<:Tuple{Vararg{Union{Real,AbstractRange,Array}}},LD}
114-
dest = Array{T}(undef, index_lengths(V.indices...))
115-
copyto!(dest, V)
113+
function unaliascopy(V::SubArray{T,N,A,I,LD}) where {T,N,A<:Array,I<:Tuple{Vararg{Union{ScalarIndex,AbstractRange{<:ScalarIndex},Array{<:Union{ScalarIndex,AbstractCartesianIndex}}}}},LD}
114+
dest = Array{T}(undef, _trimmedshape(V.indices...))
115+
trimmedpind = _trimmedpind(V.indices...)
116+
vdest = trimmedpind isa Tuple{Vararg{Union{Slice,Colon}}} ? dest : view(dest, trimmedpind...)
117+
copyto!(vdest, view(V, _trimmedvind(V.indices...)...))
116118
SubArray{T,N,A,I,LD}(dest, map(_trimmedindex, V.indices), 0, Int(LD))
117119
end
120+
# Get the proper trimmed shape
121+
_trimmedshape(::ScalarIndex, rest...) = (1, _trimmedshape(rest...)...)
122+
_trimmedshape(i::AbstractRange, rest...) = (maximum(i), _trimmedshape(rest...)...)
123+
_trimmedshape(i::Union{UnitRange,StepRange,OneTo}, rest...) = (length(i), _trimmedshape(rest...)...)
124+
_trimmedshape(i::AbstractArray{<:ScalarIndex}, rest...) = (length(i), _trimmedshape(rest...)...)
125+
_trimmedshape(i::AbstractArray{<:AbstractCartesianIndex{0}}, rest...) = _trimmedshape(rest...)
126+
_trimmedshape(i::AbstractArray{<:AbstractCartesianIndex{N}}, rest...) where {N} = (length(i), ntuple(Returns(1), Val(N - 1))..., _trimmedshape(rest...)...)
127+
_trimmedshape() = ()
128+
# We can avoid the repeation from `AbstractArray{CartesianIndex{0}}`
129+
_trimmedpind(i, rest...) = (map(Returns(:), axes(i))..., _trimmedpind(rest...)...)
130+
_trimmedpind(i::AbstractRange, rest...) = (i, _trimmedpind(rest...)...)
131+
_trimmedpind(i::Union{UnitRange,StepRange,OneTo}, rest...) = ((:), _trimmedpind(rest...)...)
132+
_trimmedpind(i::AbstractArray{<:AbstractCartesianIndex{0}}, rest...) = _trimmedpind(rest...)
133+
_trimmedpind() = ()
134+
_trimmedvind(i, rest...) = (map(Returns(:), axes(i))..., _trimmedvind(rest...)...)
135+
_trimmedvind(i::AbstractArray{<:AbstractCartesianIndex{0}}, rest...) = (map(first, axes(i))..., _trimmedvind(rest...)...)
136+
_trimmedvind() = ()
118137
# Transform indices to be "dense"
119-
_trimmedindex(i::Real) = oftype(i, 1)
120-
_trimmedindex(i::AbstractUnitRange) = oftype(i, oneto(length(i)))
121-
_trimmedindex(i::AbstractArray) = oftype(i, reshape(eachindex(IndexLinear(), i), axes(i)))
122-
138+
_trimmedindex(i::ScalarIndex) = oftype(i, 1)
139+
_trimmedindex(i::AbstractRange) = i
140+
_trimmedindex(i::Union{UnitRange,StepRange,OneTo}) = oftype(i, oneto(length(i)))
141+
_trimmedindex(i::AbstractArray{<:ScalarIndex}) = oftype(i, reshape(eachindex(IndexLinear(), i), axes(i)))
142+
_trimmedindex(i::AbstractArray{<:AbstractCartesianIndex{0}}) = oftype(i, copy(i))
143+
function _trimmedindex(i::AbstractArray{<:AbstractCartesianIndex{N}}) where {N}
144+
padding = ntuple(Returns(1), Val(N - 1))
145+
ax1 = eachindex(IndexLinear(), i)
146+
return oftype(i, reshape(CartesianIndices((ax1, padding...)), axes(i)))
147+
end
123148
## SubArray creation
124149
# We always assume that the dimensionality of the parent matches the number of
125150
# indices that end up getting passed to it, so we store the parent as a

test/subarray.jl

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -660,8 +660,21 @@ end
660660
@testset "unaliascopy trimming; Issue #26263" begin
661661
A = rand(5,5,5,5)
662662
V = view(A, 2:5, :, 2:5, 1:2:5)
663-
@test @inferred(Base.unaliascopy(V)) == V == A[2:5, :, 2:5, 1:2:5]
664-
@test @inferred(sum(Base.unaliascopy(V))) sum(V) sum(A[2:5, :, 2:5, 1:2:5])
663+
V′ = @inferred(Base.unaliascopy(V))
664+
@test size(V′.parent) == size(V)
665+
@test V′::typeof(V) == V == A[2:5, :, 2:5, 1:2:5]
666+
@test @inferred(sum(V′)) sum(V) sum(A[2:5, :, 2:5, 1:2:5])
667+
V = view(A, Base.IdentityUnitRange(2:4), :, Base.StepRangeLen(1,1,3), 1:2:5)
668+
V′ = @inferred(Base.unaliascopy(V))
669+
@test size(V.parent) != size(V′.parent)
670+
@test V′ == V && V′ isa typeof(V)
671+
i1 = collect(CartesianIndices((2:5)))
672+
i2 = [CartesianIndex(), CartesianIndex()]
673+
i3 = collect(CartesianIndices((2:5, 1:2:5)))
674+
V = view(A, i1, 1:5, i2, i3)
675+
@test @inferred(Base.unaliascopy(V))::typeof(V) == V == A[i1, 1:5, i2, i3]
676+
V = view(A, i1, 1:5, i3, i2)
677+
@test @inferred(Base.unaliascopy(V))::typeof(V) == V == A[i1, 1:5, i3, i2]
665678
end
666679

667680
@testset "issue #27632" begin

0 commit comments

Comments
 (0)