Skip to content

Commit

Permalink
fix #37199, indexing for 1-d views with offset ranges (#37204)
Browse files Browse the repository at this point in the history
We had been assuming that IdentityUnitRange matched the indices of the parent (like Slices) but they define their own offset axes. Further, other `AbstractRanges` may be similarly offset. We do not need to consider other offset `AbstractArrays` as this code only applies to `IndexLinear` `SubArray`s.
  • Loading branch information
mbauman committed Aug 27, 2020
1 parent 0ae8f95 commit 701885b
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 5 deletions.
13 changes: 8 additions & 5 deletions base/subarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -366,17 +366,20 @@ end
# The running sum is `f`; the cumulative stride product is `s`.
# If the parent is a vector, then we offset the parent's own indices with parameters of I
compute_offset1(parent::AbstractVector, stride1::Integer, I::Tuple{AbstractRange}) =
(@_inline_meta; first(I[1]) - first(axes1(I[1]))*stride1)
(@_inline_meta; first(I[1]) - stride1*first(axes1(I[1])))
# If the result is one-dimensional and it's a Colon, then linear
# indexing uses the indices along the given dimension. Otherwise
# linear indexing always starts with 1.
# indexing uses the indices along the given dimension.
# If the result is one-dimensional and it's a range, then linear
# indexing might be offset if the index itself is offset
# Otherwise linear indexing always starts with 1.
compute_offset1(parent, stride1::Integer, I::Tuple) =
(@_inline_meta; compute_offset1(parent, stride1, find_extended_dims(1, I...), find_extended_inds(I...), I))
compute_offset1(parent, stride1::Integer, dims::Tuple{Int}, inds::Tuple{Union{Slice, IdentityUnitRange}}, I::Tuple) =
compute_offset1(parent, stride1::Integer, dims::Tuple{Int}, inds::Tuple{Slice}, I::Tuple) =
(@_inline_meta; compute_linindex(parent, I) - stride1*first(axes(parent, dims[1]))) # index-preserving case
compute_offset1(parent, stride1::Integer, dims, inds::Tuple{AbstractRange}, I::Tuple) =
(@_inline_meta; compute_linindex(parent, I) - stride1*first(axes1(inds[1]))) # potentially index-offsetting case
compute_offset1(parent, stride1::Integer, dims, inds, I::Tuple) =
(@_inline_meta; compute_linindex(parent, I) - stride1) # linear indexing starts with 1

function compute_linindex(parent, I::NTuple{N,Any}) where N
@_inline_meta
IP = fill_to_length(axes(parent), OneTo(1), Val(N))
Expand Down
27 changes: 27 additions & 0 deletions test/subarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,33 @@ end
@test _test_27632(view(ones(Int64, (1, 1, 1)), 1, 1, 1)) === nothing
end

@testset "issue #37199: 1-d views with offset range indices" begin
b = zeros(6, 3)
b[Base.IdentityUnitRange(4:6), 2] .= 3
@test b == [zeros(6, 1) [0,0,0,3,3,3] zeros(6,1)]
b[4, Base.IdentityUnitRange(2:3)] .= 4
@test b == [zeros(6,1) [0,0,0,4,3,3] [0,0,0,4,0,0]]
b[Base.IdentityUnitRange(2:3), :] .= 5
@test b == [zeros(1, 3); fill(5, 2, 3); [zeros(3) [4,3,3] [4,0,0]]]
b[:, Base.IdentityUnitRange(3:3)] .= 6
@test b == [[zeros(1, 2); fill(5, 2, 2); [zeros(3) [4,3,3]]] fill(6, 6)]

A = reshape(1:5*7*11, 11, 7, 5)
inds = (1:4, 2:5, 2, :, fill(3))
offset(x) = x
offset(r::UnitRange) = Base.IdentityUnitRange(r)
for i1 in inds
for i2 in inds
for i3 in inds
vo = @view A[offset(i1), offset(i2), offset(i3)]
v = @view A[i1, i2, i3]
@test first(vo) == first(v) == first(A[i1, i2, i3])
@test collect(A[i1, i2, i3]) == collect(vo) == collect(v)
end
end
end
end

@testset "issue #29608; contiguousness" begin
@test Base.iscontiguous(view(ones(1), 1))
@test Base.iscontiguous(view(ones(10), 1:10))
Expand Down

0 comments on commit 701885b

Please sign in to comment.