From 4151c935c20976393548f24402f9b9f0e2dcfcff Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Thu, 17 Jul 2025 16:51:39 -0400 Subject: [PATCH 01/21] stricter shapes for setindex --- base/indices.jl | 42 +++++++----------------------------------- 1 file changed, 7 insertions(+), 35 deletions(-) diff --git a/base/indices.jl b/base/indices.jl index 88e48a2b331ee..c4b8f55cc3c49 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -215,38 +215,10 @@ end # those are the permutations that preserve the order of the non-singleton # dimensions. function setindex_shape_check(X::AbstractArray, I::Integer...) - li = ndims(X) - lj = length(I) - i = j = 1 - while true - ii = length(axes(X,i)) - jj = I[j] - if i == li || j == lj - while i < li - i += 1 - ii *= length(axes(X,i)) - end - while j < lj - j += 1 - jj *= I[j] - end - if ii != jj - throw_setindex_mismatch(X, I) - end - return - end - if ii == jj - i += 1 - j += 1 - elseif ii == 1 - i += 1 - elseif jj == 1 - j += 1 - else - throw_setindex_mismatch(X, I) - end + for d in 1:max(length(I), ndims(X)) + size(X, d) == get(I, d, 1) || throw_setindex_mismatch(X, I) end -end + end setindex_shape_check(X::AbstractArray) = (length(X)==1 || throw_setindex_mismatch(X,())) @@ -263,14 +235,14 @@ setindex_shape_check(X::AbstractArray{<:Any,1}, i::Integer) = setindex_shape_check(X::AbstractArray{<:Any,1}, i::Integer, j::Integer) = (length(X)==i*j || throw_setindex_mismatch(X, (i,j))) +setindex_shape_check(X::AbstractArray{<:Any,1}, i::Integer...) = + (length(X) == prod(i) || throw_setindex_mismatch(X, i)) + function setindex_shape_check(X::AbstractArray{<:Any,2}, i::Integer, j::Integer) if length(X) != i*j throw_setindex_mismatch(X, (i,j)) end - sx1 = length(axes(X,1)) - if !(i == 1 || i == sx1 || sx1 == 1) - throw_setindex_mismatch(X, (i,j)) - end + ((i, j) == size(X)) || throw_setindex_mismatch(X, (i,j)) end setindex_shape_check(::Any...) = From 1608170744b47f7181f2de42a187f326b454004a Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Fri, 18 Jul 2025 14:39:05 -0400 Subject: [PATCH 02/21] implement semantics from issue discussion --- base/broadcast.jl | 2 +- base/indices.jl | 70 ++++++++++++++++++++++++---------------- base/multidimensional.jl | 13 +++++--- 3 files changed, 51 insertions(+), 34 deletions(-) diff --git a/base/broadcast.jl b/base/broadcast.jl index b86baf08ddfe0..8b04927a85cb1 100644 --- a/base/broadcast.jl +++ b/base/broadcast.jl @@ -614,7 +614,7 @@ Base.@propagate_inbounds _newindex(ax::Tuple{}, I::Tuple{}) = () end Base.@propagate_inbounds function Base.getindex(bc::Broadcasted, Is::Vararg{Union{Integer,CartesianIndex},N}) where {N} - I = to_index(Base.IteratorsMD.flatten(Is)) + I = to_index(Base.flatten(Is)) _getindex(IndexStyle(bc), bc, I) end @inline function _getindex(::IndexStyle, bc, I) diff --git a/base/indices.jl b/base/indices.jl index c4b8f55cc3c49..5b0319e382152 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -202,48 +202,62 @@ function promote_shape(a::Indices, b::Indices) end function throw_setindex_mismatch(X, I) + pI = filter(!isnegative, I) if length(I) == 1 - throw(DimensionMismatch("tried to assign $(length(X)) elements to $(I[1]) destinations")) + throw(DimensionMismatch("tried to assign $(length(X)) elements to $(pI[1]) destinations")) else - throw(DimensionMismatch("tried to assign $(dims2string(size(X))) array to $(dims2string(I)) destination")) + throw(DimensionMismatch("tried to assign $(dims2string(size(X))) array to $(dims2string(pI)) destination")) end end -# check for valid sizes in A[I...] = X where X <: AbstractArray -# we want to allow dimensions that are equal up to permutation, but only -# for permutations that leave array elements in the same linear order. -# those are the permutations that preserve the order of the non-singleton -# dimensions. -function setindex_shape_check(X::AbstractArray, I::Integer...) - for d in 1:max(length(I), ndims(X)) - size(X, d) == get(I, d, 1) || throw_setindex_mismatch(X, I) + + +const IntegerOrTuple = Union{Integer, Tuple{Vararg{Integer}}} + +flatten(::Tuple{}) = () +flatten(I::Tuple{Any}) = Tuple(I[1]) +@inline flatten(I::Tuple) = (Tuple(I[1])..., flatten(tail(I))...) + +_nnprod() = 1 +_nnprod(i::Integer, rest::Integer...) = + isnegative(i) ? _nnprod(rest...) : i * _nnprod(rest...) + +_trailing_one_or_dropped() = true +_trailing_one_or_dropped(i::Integer, rest...) = + isone(abs(i)) && _trailing_one_or_dropped(rest...) + +_shapes_match(::Bool, ::Tuple{}) = true +_shapes_match(::Bool, sz) = _trailing_one_or_dropped(sz...) +_shapes_match(::Bool, ::Tuple{}, I::Integer...) = _trailing_one_or_dropped(I...) +function _shapes_match(isfirstdim, sz, i::Integer, I::Integer...) + if isnegative(i) + return _shapes_match(isfirstdim, sz, I...) + else + if isfirstdim && _trailing_one_or_dropped(sz...) && _nnprod(sz...) == i + return true + else + return (first(sz) == i) && _shapes_match(false, tail(sz), I...) + end end - end +end + +setindex_shape_check(X::AbstractArray, I::Integer...) = + _shapes_match(true, size(X), I...) || throw_setindex_mismatch(X, I) setindex_shape_check(X::AbstractArray) = - (length(X)==1 || throw_setindex_mismatch(X,())) + (length(X) == 1 || throw_setindex_mismatch(X,())) setindex_shape_check(X::AbstractArray, i::Integer) = - (length(X)==i || throw_setindex_mismatch(X, (i,))) - -setindex_shape_check(X::AbstractArray{<:Any, 0}, i::Integer...) = - (length(X) == prod(i) || throw_setindex_mismatch(X, i)) + (length(X) == i || throw_setindex_mismatch(X, (i,))) -setindex_shape_check(X::AbstractArray{<:Any,1}, i::Integer) = - (length(X)==i || throw_setindex_mismatch(X, (i,))) - -setindex_shape_check(X::AbstractArray{<:Any,1}, i::Integer, j::Integer) = - (length(X)==i*j || throw_setindex_mismatch(X, (i,j))) +setindex_shape_check(X::AbstractArray{<:Any,0}, i::Integer...) = + (length(X) == _nnprod(i...) || throw_setindex_mismatch(X, i)) setindex_shape_check(X::AbstractArray{<:Any,1}, i::Integer...) = - (length(X) == prod(i) || throw_setindex_mismatch(X, i)) + (length(X) == _nnprod(i...) || throw_setindex_mismatch(X, i)) -function setindex_shape_check(X::AbstractArray{<:Any,2}, i::Integer, j::Integer) - if length(X) != i*j - throw_setindex_mismatch(X, (i,j)) - end - ((i, j) == size(X)) || throw_setindex_mismatch(X, (i,j)) -end +setindex_shape_check(X::AbstractArray, i::IntegerOrTuple...) = + setindex_shape_check(X, flatten(i)...) setindex_shape_check(::Any...) = throw(ArgumentError("indexed assignment with a single value to possibly many locations is not supported; perhaps use broadcasting `.=` instead?")) diff --git a/base/multidimensional.jl b/base/multidimensional.jl index 83a03fc1b45bf..8913a61710316 100644 --- a/base/multidimensional.jl +++ b/base/multidimensional.jl @@ -86,10 +86,7 @@ module IteratorsMD CartesianIndex{N}() where {N} = CartesianIndex{N}(()) # Un-nest passed CartesianIndexes CartesianIndex{N}(index::CartesianIndex{N}) where {N} = index - CartesianIndex(index::Union{Integer, CartesianIndex}...) = CartesianIndex(flatten(index)) - flatten(::Tuple{}) = () - flatten(I::Tuple{Any}) = Tuple(I[1]) - @inline flatten(I::Tuple) = (Tuple(I[1])..., flatten(tail(I))...) + CartesianIndex(index::Union{Integer, CartesianIndex}...) = CartesianIndex(Base.flatten(index)) CartesianIndex(index::Tuple{Vararg{Union{Integer, CartesianIndex}}}) = CartesianIndex(index...) function show(io::IO, i::CartesianIndex) print(io, "CartesianIndex(") @@ -816,6 +813,12 @@ index_shape() = () @inline index_shape(::Real, rest...) = index_shape(rest...) @inline index_shape(A::AbstractArray, rest...) = (axes(A)..., index_shape(rest...)...) +index_shape2() = () + +# -1 used as signal for dropped dimension. 0 is actually a valid index length +@inline index_shape2(::Real, rest...) = (-1, index_shape2(rest...)...) +@inline index_shape2(A::AbstractArray, rest...) = (size(A), index_shape2(rest...)...) + """ LogicalIndex(mask) @@ -1039,7 +1042,7 @@ function _generate_unsafe_setindex!_body(N::Int) quote x′ = unalias(A, x) @nexprs $N d->(I_d = unalias(A, I[d])) - idxlens = @ncall $N index_lengths I + idxlens = @ncall $N index_shape2 I @ncall $N setindex_shape_check x′ (d->idxlens[d]) X = eachindex(x′) Xy = _prechecked_iterate(X) From b4e4961b9b5ca9de03e298d78f33f83a21dbd53d Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Fri, 18 Jul 2025 14:40:34 -0400 Subject: [PATCH 03/21] rename --- base/multidimensional.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/base/multidimensional.jl b/base/multidimensional.jl index 8913a61710316..dc53570518a4e 100644 --- a/base/multidimensional.jl +++ b/base/multidimensional.jl @@ -813,11 +813,11 @@ index_shape() = () @inline index_shape(::Real, rest...) = index_shape(rest...) @inline index_shape(A::AbstractArray, rest...) = (axes(A)..., index_shape(rest...)...) -index_shape2() = () +index_shape_nested() = () # -1 used as signal for dropped dimension. 0 is actually a valid index length -@inline index_shape2(::Real, rest...) = (-1, index_shape2(rest...)...) -@inline index_shape2(A::AbstractArray, rest...) = (size(A), index_shape2(rest...)...) +@inline index_shape_nested(::Real, rest...) = (-1, index_shape_nested(rest...)...) +@inline index_shape_nested(A::AbstractArray, rest...) = (size(A), index_shape_nested(rest...)...) """ LogicalIndex(mask) @@ -1042,7 +1042,7 @@ function _generate_unsafe_setindex!_body(N::Int) quote x′ = unalias(A, x) @nexprs $N d->(I_d = unalias(A, I[d])) - idxlens = @ncall $N index_shape2 I + idxlens = @ncall $N index_shape_nested I @ncall $N setindex_shape_check x′ (d->idxlens[d]) X = eachindex(x′) Xy = _prechecked_iterate(X) From b09e472bfb259ad4805cb19ad4c0943ce9e66e9d Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Fri, 18 Jul 2025 14:56:23 -0400 Subject: [PATCH 04/21] tidy --- base/indices.jl | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/base/indices.jl b/base/indices.jl index 5b0319e382152..510ba685f5a66 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -210,13 +210,11 @@ function throw_setindex_mismatch(X, I) end end - - const IntegerOrTuple = Union{Integer, Tuple{Vararg{Integer}}} flatten(::Tuple{}) = () flatten(I::Tuple{Any}) = Tuple(I[1]) -@inline flatten(I::Tuple) = (Tuple(I[1])..., flatten(tail(I))...) +flatten(I::Tuple) = (@inline; (Tuple(I[1])..., flatten(tail(I))...)) _nnprod() = 1 _nnprod(i::Integer, rest::Integer...) = From 6b299f2bf387a7dc7cfd8a7fa93a7108a59b287a Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Fri, 18 Jul 2025 15:37:30 -0400 Subject: [PATCH 05/21] fix --- base/indices.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/base/indices.jl b/base/indices.jl index 510ba685f5a66..5770ca173fc2b 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -231,8 +231,8 @@ function _shapes_match(isfirstdim, sz, i::Integer, I::Integer...) if isnegative(i) return _shapes_match(isfirstdim, sz, I...) else - if isfirstdim && _trailing_one_or_dropped(sz...) && _nnprod(sz...) == i - return true + if isfirstdim && _trailing_one_or_dropped(I...) + return _nnprod(sz...) == i else return (first(sz) == i) && _shapes_match(false, tail(sz), I...) end From 50d5cfd78780bc5e85bae363add2470775e8714e Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Sun, 20 Jul 2025 09:41:23 -0400 Subject: [PATCH 06/21] update bitarray --- base/multidimensional.jl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/base/multidimensional.jl b/base/multidimensional.jl index dc53570518a4e..20415354291da 100644 --- a/base/multidimensional.jl +++ b/base/multidimensional.jl @@ -1042,8 +1042,8 @@ function _generate_unsafe_setindex!_body(N::Int) quote x′ = unalias(A, x) @nexprs $N d->(I_d = unalias(A, I[d])) - idxlens = @ncall $N index_shape_nested I - @ncall $N setindex_shape_check x′ (d->idxlens[d]) + idxsigs = @ncall $N index_shape_nested I + @ncall $N setindex_shape_check x′ (d->idxsigs[d]) X = eachindex(x′) Xy = _prechecked_iterate(X) @inbounds @nloops $N i d->I_d begin @@ -1564,7 +1564,8 @@ end N = length(I) quote idxlens = @ncall $N index_lengths I0 d->I[d] - @ncall $N setindex_shape_check X idxlens[1] d->idxlens[d+1] + idxsigs = @ncall $N index_shape_nested I0 d->I[d] + @ncall $N setindex_shape_check X idxsigs[1] d->idxsigs[d+1] isempty(X) && return B f0 = indexoffset(I0)+1 l0 = idxlens[1] From c98341568cee556ebc918fae8001bfded33f10d3 Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Mon, 21 Jul 2025 23:42:01 -0400 Subject: [PATCH 07/21] clean up a bit --- base/indices.jl | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/base/indices.jl b/base/indices.jl index 5770ca173fc2b..6be514de69188 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -217,21 +217,21 @@ flatten(I::Tuple{Any}) = Tuple(I[1]) flatten(I::Tuple) = (@inline; (Tuple(I[1])..., flatten(tail(I))...)) _nnprod() = 1 -_nnprod(i::Integer, rest::Integer...) = - isnegative(i) ? _nnprod(rest...) : i * _nnprod(rest...) +_nnprod(i::Integer, I::Integer...) = + (i == -1) ? _nnprod(I...) : i * _nnprod(I...) -_trailing_one_or_dropped() = true -_trailing_one_or_dropped(i::Integer, rest...) = - isone(abs(i)) && _trailing_one_or_dropped(rest...) +_trailing_dropped(I::Integer...) = true +_trailing_dropped(i::Integer, I::Integer...) = (i == -1) && _trailing_dropped(I...) _shapes_match(::Bool, ::Tuple{}) = true -_shapes_match(::Bool, sz) = _trailing_one_or_dropped(sz...) -_shapes_match(::Bool, ::Tuple{}, I::Integer...) = _trailing_one_or_dropped(I...) +_shapes_match(isfirstdim::Bool, sz) = _shapes_match(isfirstdim, (), sz...) +_shapes_match(isfirstdim::Bool, sz::Tuple{}, i::Integer, I::Integer...) = + isone(abs(i)) && _shapes_match(isfirstdim, sz, I...) function _shapes_match(isfirstdim, sz, i::Integer, I::Integer...) - if isnegative(i) + if i == -1 return _shapes_match(isfirstdim, sz, I...) else - if isfirstdim && _trailing_one_or_dropped(I...) + if isfirstdim && _trailing_dropped(I...) return _nnprod(sz...) == i else return (first(sz) == i) && _shapes_match(false, tail(sz), I...) @@ -239,23 +239,23 @@ function _shapes_match(isfirstdim, sz, i::Integer, I::Integer...) end end -setindex_shape_check(X::AbstractArray, I::Integer...) = - _shapes_match(true, size(X), I...) || throw_setindex_mismatch(X, I) - setindex_shape_check(X::AbstractArray) = - (length(X) == 1 || throw_setindex_mismatch(X,())) + (length(X) == 1 || throw_setindex_mismatch(X, ())) setindex_shape_check(X::AbstractArray, i::Integer) = (length(X) == i || throw_setindex_mismatch(X, (i,))) -setindex_shape_check(X::AbstractArray{<:Any,0}, i::Integer...) = - (length(X) == _nnprod(i...) || throw_setindex_mismatch(X, i)) +setindex_shape_check(X::AbstractArray{<:Any,0}, I::Integer...) = + (length(X) == _nnprod(I...) || throw_setindex_mismatch(X, I)) -setindex_shape_check(X::AbstractArray{<:Any,1}, i::Integer...) = - (length(X) == _nnprod(i...) || throw_setindex_mismatch(X, i)) +setindex_shape_check(X::AbstractArray{<:Any,1}, I::Integer...) = + (length(X) == _nnprod(I...) || throw_setindex_mismatch(X, I)) -setindex_shape_check(X::AbstractArray, i::IntegerOrTuple...) = - setindex_shape_check(X, flatten(i)...) +setindex_shape_check(X::AbstractArray, I::IntegerOrTuple...) = + setindex_shape_check(X, flatten(I)...) + +setindex_shape_check(X::AbstractArray, I::Integer...) = + _shapes_match(true, size(X), I...) || throw_setindex_mismatch(X, I) setindex_shape_check(::Any...) = throw(ArgumentError("indexed assignment with a single value to possibly many locations is not supported; perhaps use broadcasting `.=` instead?")) From 3784ef0e43b9236a3edb9c89eeee4fdeaa10ad7d Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Tue, 22 Jul 2025 09:10:58 -0400 Subject: [PATCH 08/21] add test and restore flatten --- base/multidimensional.jl | 4 ++-- test/arrayops.jl | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/base/multidimensional.jl b/base/multidimensional.jl index 20415354291da..9d7c6db115274 100644 --- a/base/multidimensional.jl +++ b/base/multidimensional.jl @@ -10,7 +10,7 @@ module IteratorsMD import .Base: +, -, *, (:) import .Base: simd_outer_range, simd_inner_length, simd_index, setindex import Core: Tuple - using .Base: to_index, fill_to_length, tail, safe_tail + using .Base: to_index, fill_to_length, tail, safe_tail, flatten using .Base: IndexLinear, IndexCartesian, AbstractCartesianIndex, ReshapedArray, ReshapedArrayLF, OneTo, Fix1 using .Base.Iterators: Reverse, PartitionIterator @@ -86,7 +86,7 @@ module IteratorsMD CartesianIndex{N}() where {N} = CartesianIndex{N}(()) # Un-nest passed CartesianIndexes CartesianIndex{N}(index::CartesianIndex{N}) where {N} = index - CartesianIndex(index::Union{Integer, CartesianIndex}...) = CartesianIndex(Base.flatten(index)) + CartesianIndex(index::Union{Integer, CartesianIndex}...) = CartesianIndex(flatten(index)) CartesianIndex(index::Tuple{Vararg{Union{Integer, CartesianIndex}}}) = CartesianIndex(index...) function show(io::IO, i::CartesianIndex) print(io, "CartesianIndex(") diff --git a/test/arrayops.jl b/test/arrayops.jl index 84b9c8e7b2f6a..ebec4eb160a69 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -3219,6 +3219,45 @@ end @test setindex!(zeros(2,2), fill(1.0), CI0, 1, 1) == [1.0 0.0; 0.0 0.0] end +@testset "setindex! at mismatched shapes" begin + idx_options = ( + 1, + 1:1, + 2:2, + 1:0, + 1:3 + ) + A = zeros(3, 3, 3) + for (i, j, k) in Iterators.product(Iterators.repeated(idx_options, 3)...) + for islinear in (false, true) + I = islinear ? i : (i, j, k) + + # don't test scalar setindex + all(x -> x isa Int, I) && continue + + X = ones((length(i) for i in I if i != 1)...) + X_trail = reshape(X, size(X)..., 1) + X_prepend = reshape(X, 1, size(X)...) + vX = vec(X) + cX = reshape(vX, length(vX), 1) + + for _X in (X, X_trail, X_prepend, vX, cX) + AI = A[I...] + + if ((isone(ndims(_X)) || isone(ndims(AI))) && (length(_X) == length(AI))) || + all(d -> axes(_X, d) == axes(AI, d), 1:max(ndims(_X), ndims(AI))) + @test any(isone, setindex!(A, _X, I...)) + @test any(isone, setindex!(A, _X, I..., 1)) + else + @test_throws DimensionMismatch setindex!(A, _X, I...) + @test_throws DimensionMismatch setindex!(A, _X, I..., 1) + @test_throws DimensionMismatch setindex!(A, _X, I..., :) + end + end + end + end +end + # Throws ArgumentError for negative dimensions in Array @test_throws ArgumentError fill('a', -10) From d0d902de33815fc8ece79d3b9cb009d502ea4b8e Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Tue, 22 Jul 2025 09:28:09 -0400 Subject: [PATCH 09/21] move flatten --- base/indices.jl | 4 ---- base/tuple.jl | 4 ++++ test/arrayops.jl | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/base/indices.jl b/base/indices.jl index 6be514de69188..5c7ad92c2c3d6 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -212,10 +212,6 @@ end const IntegerOrTuple = Union{Integer, Tuple{Vararg{Integer}}} -flatten(::Tuple{}) = () -flatten(I::Tuple{Any}) = Tuple(I[1]) -flatten(I::Tuple) = (@inline; (Tuple(I[1])..., flatten(tail(I))...)) - _nnprod() = 1 _nnprod(i::Integer, I::Integer...) = (i == -1) ? _nnprod(I...) : i * _nnprod(I...) diff --git a/base/tuple.jl b/base/tuple.jl index ac42c667269e6..54593287e8fb7 100644 --- a/base/tuple.jl +++ b/base/tuple.jl @@ -267,6 +267,10 @@ end first(::Tuple{}) = throw(ArgumentError("tuple must be non-empty")) first(t::Tuple) = t[1] +flatten(::Tuple{}) = () +flatten(I::Tuple{Any}) = Tuple(I[1]) +flatten(I::Tuple) = (@inline; (Tuple(I[1])..., flatten(tail(I))...)) + # eltype eltype(::Type{Tuple{}}) = Bottom diff --git a/test/arrayops.jl b/test/arrayops.jl index ebec4eb160a69..b68de86b4b4a9 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -3232,7 +3232,7 @@ end for islinear in (false, true) I = islinear ? i : (i, j, k) - # don't test scalar setindex + # don't test scalar setindex here all(x -> x isa Int, I) && continue X = ones((length(i) for i in I if i != 1)...) From 037459cd35030355ad4380341ff07288db8f20d5 Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Tue, 22 Jul 2025 09:38:35 -0400 Subject: [PATCH 10/21] add more indices --- test/arrayops.jl | 51 ++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/test/arrayops.jl b/test/arrayops.jl index b68de86b4b4a9..83b0237b94bd6 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -3225,34 +3225,35 @@ end 1:1, 2:2, 1:0, - 1:3 + 1:3, + [1,2,3], + [1 2 3], + [1;2;;;] ) A = zeros(3, 3, 3) for (i, j, k) in Iterators.product(Iterators.repeated(idx_options, 3)...) - for islinear in (false, true) - I = islinear ? i : (i, j, k) - - # don't test scalar setindex here - all(x -> x isa Int, I) && continue - - X = ones((length(i) for i in I if i != 1)...) - X_trail = reshape(X, size(X)..., 1) - X_prepend = reshape(X, 1, size(X)...) - vX = vec(X) - cX = reshape(vX, length(vX), 1) - - for _X in (X, X_trail, X_prepend, vX, cX) - AI = A[I...] - - if ((isone(ndims(_X)) || isone(ndims(AI))) && (length(_X) == length(AI))) || - all(d -> axes(_X, d) == axes(AI, d), 1:max(ndims(_X), ndims(AI))) - @test any(isone, setindex!(A, _X, I...)) - @test any(isone, setindex!(A, _X, I..., 1)) - else - @test_throws DimensionMismatch setindex!(A, _X, I...) - @test_throws DimensionMismatch setindex!(A, _X, I..., 1) - @test_throws DimensionMismatch setindex!(A, _X, I..., :) - end + I = (i, j, k) + + # don't test scalar setindex here + all(x -> x isa Int, I) && continue + + X = ones((length(i) for i in I if i != 1)...) + X_trail = reshape(X, size(X)..., 1) + X_prepend = reshape(X, 1, size(X)...) + vX = vec(X) + cX = reshape(vX, length(vX), 1) + + for _X in (X, X_trail, X_prepend, vX, cX) + AI = A[I...] + + if ((isone(ndims(_X)) || isone(ndims(AI))) && (length(_X) == length(AI))) || + all(d -> axes(_X, d) == axes(AI, d), 1:max(ndims(_X), ndims(AI))) + @test any(isone, setindex!(A, _X, I...)) + @test any(isone, setindex!(A, _X, I..., 1)) + else + @test_throws DimensionMismatch setindex!(A, _X, I...) + @test_throws DimensionMismatch setindex!(A, _X, I..., 1) + @test_throws DimensionMismatch setindex!(A, _X, I..., :) end end end From 397eec4b608889599683424e870fc4840599975c Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Tue, 22 Jul 2025 10:25:51 -0400 Subject: [PATCH 11/21] _nnprod --> prod --- base/indices.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/base/indices.jl b/base/indices.jl index 5c7ad92c2c3d6..61ea344d9f858 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -228,7 +228,8 @@ function _shapes_match(isfirstdim, sz, i::Integer, I::Integer...) return _shapes_match(isfirstdim, sz, I...) else if isfirstdim && _trailing_dropped(I...) - return _nnprod(sz...) == i + # sz comes from a call to size(X) and never contains negatives + return prod(sz) == i else return (first(sz) == i) && _shapes_match(false, tail(sz), I...) end From 3c119d349e0f7b36acdd65ea82b2ec55a2e1ae2f Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Tue, 22 Jul 2025 16:00:01 -0400 Subject: [PATCH 12/21] outline error path while we're at it --- base/indices.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/indices.jl b/base/indices.jl index 61ea344d9f858..9b9c258a6e620 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -201,7 +201,7 @@ function promote_shape(a::Indices, b::Indices) return a end -function throw_setindex_mismatch(X, I) +@noinline function throw_setindex_mismatch(X, I) pI = filter(!isnegative, I) if length(I) == 1 throw(DimensionMismatch("tried to assign $(length(X)) elements to $(pI[1]) destinations")) From ee7ffe6bce00769c52e037f0a3e645e4abad6d1a Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Wed, 23 Jul 2025 07:45:08 -0400 Subject: [PATCH 13/21] typo --- base/indices.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/indices.jl b/base/indices.jl index 9b9c258a6e620..183c54279d191 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -203,7 +203,7 @@ end @noinline function throw_setindex_mismatch(X, I) pI = filter(!isnegative, I) - if length(I) == 1 + if length(pI) == 1 throw(DimensionMismatch("tried to assign $(length(X)) elements to $(pI[1]) destinations")) else throw(DimensionMismatch("tried to assign $(dims2string(size(X))) array to $(dims2string(pI)) destination")) From 56d2b74215715a8909bb5d448ff4b7d9fe14ac31 Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Wed, 23 Jul 2025 07:57:26 -0400 Subject: [PATCH 14/21] `@noinline` has bootstrap issues --- base/indices.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/indices.jl b/base/indices.jl index 183c54279d191..8829c630be47a 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -201,7 +201,7 @@ function promote_shape(a::Indices, b::Indices) return a end -@noinline function throw_setindex_mismatch(X, I) +function throw_setindex_mismatch(X, I) pI = filter(!isnegative, I) if length(pI) == 1 throw(DimensionMismatch("tried to assign $(length(X)) elements to $(pI[1]) destinations")) From 1978b6db0987901388a09a2a7ab042c19863ab86 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Thu, 31 Jul 2025 15:37:36 -0400 Subject: [PATCH 15/21] an attempt at documenting these behaviors Just something to get the docs discussion started --- doc/src/manual/arrays.md | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/doc/src/manual/arrays.md b/doc/src/manual/arrays.md index ba2d261301b40..7e63e77f485ab 100644 --- a/doc/src/manual/arrays.md +++ b/doc/src/manual/arrays.md @@ -603,17 +603,23 @@ overwritten with the value of `X`, [`convert`](@ref)ing to the [`eltype`](@ref) of `A` if necessary. -If any index `I_k` is itself an array, then the right hand side `X` must also be an -array with the same shape as the result of indexing `A[I_1, I_2, ..., I_n]` or a vector with -the same number of elements. The value in location `I_1[i_1], I_2[i_2], ..., I_n[i_n]` of -`A` is overwritten with the value `X[i_1, i_2, ..., i_n]`, converting if necessary. The -element-wise assignment operator `.=` may be used to [broadcast](@ref Broadcasting) `X` -across the selected locations: - - -``` -A[I_1, I_2, ..., I_n] .= X -``` +If any index `I_k` is itself an array, then they select multiple locations to be assigned. +The right hand side `X` must also be an +array with the same number of elements as the indices select and a compatible shape. +Each value in `X` is assigned into the corresponding location in `A[I_1, I_2, ..., I_n]`, +following the rules of [linear indexing and omitted/extra indices](@ref man-number-of-indices). +That is, `X` must be the same shape as the result of indexing `A[I_1, I_2, ..., I_n]` or +either may be a vector. In the simple case where all the supplied indices are vectors and +the shapes match, each value in location `I_1[i_1], I_2[i_2], ..., I_n[i_n]` of +`A` is overwritten with the value `X[i_1, i_2, ..., i_n]`, converting if necessary. If either +side is a vector, then the elements are assigned with column-major [linear indexing](@ref man-linear-indexing). + +The element-wise assignment operator `.=` may be used to [broadcast](@ref Broadcasting) `X` +across multiple selected locations with `A[I_1, I_2, ..., I_n] .= X`. In the common case where the +concatenated axes of the indices matches the axes of `X`, this behaves like the nonscalar +indexed assignment described above (that is, `=` without the broadcasting `.`). Unlike indexed +assignment, however, any mismatched axes will be broadcast (in accordance with broadcast's rules) +and linear indexing is not supported. Just as in [Indexing](@ref man-array-indexing), the `end` keyword may be used to represent the last index of each dimension within the indexing brackets, as @@ -838,7 +844,7 @@ julia> x[vec(mask)] == x[mask] # we can also index with a single Boolean vector true ``` -### Number of indices +### [Number of indices](@id man-number-of-indices) #### Cartesian indexing @@ -847,7 +853,7 @@ index selects the position(s) in its particular dimension. For example, in the t array `A = rand(4, 3, 2)`, `A[2, 3, 1]` will select the number in the second row of the third column in the first "page" of the array. This is often referred to as _cartesian indexing_. -#### Linear indexing +#### [Linear indexing](@id man-linear-indexing) When exactly one index `i` is provided, that index no longer represents a location in a particular dimension of the array. Instead, it selects the `i`th element using the From f3217e010c830eef5f6b80f5f5fb0c5487a61d79 Mon Sep 17 00:00:00 2001 From: Andy Dienes <51664769+adienes@users.noreply.github.com> Date: Thu, 31 Jul 2025 15:44:42 -0400 Subject: [PATCH 16/21] Update test/arrayops.jl --- test/arrayops.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/arrayops.jl b/test/arrayops.jl index 83b0237b94bd6..90ff599340ed7 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -3247,7 +3247,7 @@ end AI = A[I...] if ((isone(ndims(_X)) || isone(ndims(AI))) && (length(_X) == length(AI))) || - all(d -> axes(_X, d) == axes(AI, d), 1:max(ndims(_X), ndims(AI))) + all(d -> size(_X, d) == size(AI, d), 1:max(ndims(_X), ndims(AI))) @test any(isone, setindex!(A, _X, I...)) @test any(isone, setindex!(A, _X, I..., 1)) else From 175c592b0598a94fd08ba0349f81d80e5e015c19 Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Sun, 3 Aug 2025 10:13:14 -0400 Subject: [PATCH 17/21] docchange v2 --- doc/src/manual/arrays.md | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/doc/src/manual/arrays.md b/doc/src/manual/arrays.md index 7e63e77f485ab..857235fc5c799 100644 --- a/doc/src/manual/arrays.md +++ b/doc/src/manual/arrays.md @@ -598,16 +598,18 @@ where each `I_k` may be a scalar integer, an array of integers, or any other ranges of the form `a:c` or `a:b:c` to select contiguous or strided subsections, and arrays of booleans to select elements at their `true` indices. -If all indices `I_k` are integers, then the value in location `I_1, I_2, ..., I_n` of `A` is -overwritten with the value of `X`, [`convert`](@ref)ing to the -[`eltype`](@ref) of `A` if necessary. +Just as [indexing](@ref man-array-indexing) `A[I_1, I_2, ..., I_n]` returns either a single +element (if all indices are scalar) or an array (if any index is nonscalar), the indexed +assignment of `A[I_1, I_2, ..., I_n] = X` assigns either a single value to a single location +or an array of value(s) to an array of location(s) within the array `A`. + +If all indices `I_k` are scalar, then the value in location `I_1, I_2, ..., I_n` of `A` is assigned +to the value of `X`, [`convert`](@ref)ing to the [`eltype`](@ref) of `A` if necessary. Otherwise +if any index is nonscalar, the right hand side `X` must be an array with the same number of +elements as the number of locations selected by the indices and it must have a compatible shape. +Each value in `X` is assigned into the corresponding location in `A[I_1, I_2, ..., I_n]`, following +the rules of [linear indexing and omitted/extra indices](@ref man-number-of-indices). - -If any index `I_k` is itself an array, then they select multiple locations to be assigned. -The right hand side `X` must also be an -array with the same number of elements as the indices select and a compatible shape. -Each value in `X` is assigned into the corresponding location in `A[I_1, I_2, ..., I_n]`, -following the rules of [linear indexing and omitted/extra indices](@ref man-number-of-indices). That is, `X` must be the same shape as the result of indexing `A[I_1, I_2, ..., I_n]` or either may be a vector. In the simple case where all the supplied indices are vectors and the shapes match, each value in location `I_1[i_1], I_2[i_2], ..., I_n[i_n]` of From d8517d027fa70b37a972a8c0260433a8e24140ee Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Sun, 3 Aug 2025 10:22:30 -0400 Subject: [PATCH 18/21] restore inner noinline --- base/indices.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/base/indices.jl b/base/indices.jl index 8829c630be47a..51843d56de034 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -202,6 +202,7 @@ function promote_shape(a::Indices, b::Indices) end function throw_setindex_mismatch(X, I) + @noinline pI = filter(!isnegative, I) if length(pI) == 1 throw(DimensionMismatch("tried to assign $(length(X)) elements to $(pI[1]) destinations")) From 5fb3e6a1c6e1440bca456d3d233f813235fcc3f3 Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Fri, 8 Aug 2025 09:45:54 +0800 Subject: [PATCH 19/21] commit to offset behavior in setindex --- base/array.jl | 7 ++----- test/arrayops.jl | 6 ++++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/base/array.jl b/base/array.jl index d62436ca30497..6fc2b7f8a2948 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1025,13 +1025,10 @@ function setindex!(A::Array, X::AbstractArray, I::AbstractVector{Int}) @_propagate_inbounds_meta @boundscheck setindex_shape_check(X, length(I)) @boundscheck checkbounds(A, I) - require_one_based_indexing(X) X′ = unalias(A, X) I′ = unalias(A, I) - count = 1 - for i in I′ - @inbounds A[i] = X′[count] - count += 1 + for (j, i) in zip(eachindex(X′), I′) + @inbounds A[i] = X′[j] end return A end diff --git a/test/arrayops.jl b/test/arrayops.jl index 90ff599340ed7..bd7d9b0904e1e 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -3228,7 +3228,8 @@ end 1:3, [1,2,3], [1 2 3], - [1;2;;;] + [1;2;;;], + OffsetArray(1:3, -1), ) A = zeros(3, 3, 3) for (i, j, k) in Iterators.product(Iterators.repeated(idx_options, 3)...) @@ -3242,8 +3243,9 @@ end X_prepend = reshape(X, 1, size(X)...) vX = vec(X) cX = reshape(vX, length(vX), 1) + oX = OffsetArray(X, (size(X) .- 2*(ndims(X) - 1))...) - for _X in (X, X_trail, X_prepend, vX, cX) + for _X in (X, X_trail, X_prepend, vX, cX, oX) AI = A[I...] if ((isone(ndims(_X)) || isone(ndims(AI))) && (length(_X) == length(AI))) || From b7198d554c6ffca8b5d2de75f0a510b6daa47419 Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Fri, 8 Aug 2025 09:59:42 +0800 Subject: [PATCH 20/21] speed up testset --- test/arrayops.jl | 49 +++++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/test/arrayops.jl b/test/arrayops.jl index bd7d9b0904e1e..3d484b05ff1c2 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -3232,30 +3232,33 @@ end OffsetArray(1:3, -1), ) A = zeros(3, 3, 3) + seen = Set{UInt}() for (i, j, k) in Iterators.product(Iterators.repeated(idx_options, 3)...) - I = (i, j, k) - - # don't test scalar setindex here - all(x -> x isa Int, I) && continue - - X = ones((length(i) for i in I if i != 1)...) - X_trail = reshape(X, size(X)..., 1) - X_prepend = reshape(X, 1, size(X)...) - vX = vec(X) - cX = reshape(vX, length(vX), 1) - oX = OffsetArray(X, (size(X) .- 2*(ndims(X) - 1))...) - - for _X in (X, X_trail, X_prepend, vX, cX, oX) - AI = A[I...] - - if ((isone(ndims(_X)) || isone(ndims(AI))) && (length(_X) == length(AI))) || - all(d -> size(_X, d) == size(AI, d), 1:max(ndims(_X), ndims(AI))) - @test any(isone, setindex!(A, _X, I...)) - @test any(isone, setindex!(A, _X, I..., 1)) - else - @test_throws DimensionMismatch setindex!(A, _X, I...) - @test_throws DimensionMismatch setindex!(A, _X, I..., 1) - @test_throws DimensionMismatch setindex!(A, _X, I..., :) + id = hash(sort([i, j, k]; by=hash)) + in!(id, seen) && continue + for I in ((i, j, k), (k, j, i)) + # don't test scalar setindex here + all(x -> x isa Int, I) && continue + + X = ones((length(i) for i in I if i != 1)...) + X_trail = reshape(X, size(X)..., 1) + X_prepend = reshape(X, 1, size(X)...) + vX = vec(X) + cX = reshape(vX, length(vX), 1) + oX = OffsetArray(X, (size(X) .- 2*(ndims(X) - 1))...) + + for _X in (X, X_trail, X_prepend, vX, cX, oX) + AI = view(A, I...) + + if ((isone(ndims(_X)) || isone(ndims(AI))) && (length(_X) == length(AI))) || + all(d -> size(_X, d) == size(AI, d), 1:max(ndims(_X), ndims(AI))) + @test any(isone, setindex!(A, _X, I...)) + @test any(isone, setindex!(A, _X, I..., 1)) + else + @test_throws DimensionMismatch setindex!(A, _X, I...) + @test_throws DimensionMismatch setindex!(A, _X, I..., 1) + @test_throws DimensionMismatch setindex!(A, _X, I..., :) + end end end end From cfdc1d64938f04e3b272d2b87d8864ffd9690c2d Mon Sep 17 00:00:00 2001 From: Andy Dienes Date: Fri, 29 Aug 2025 10:46:26 -0400 Subject: [PATCH 21/21] fixup testset --- test/arrayops.jl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/arrayops.jl b/test/arrayops.jl index 5f914c1ad1f22..50d8702a22fdd 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -3253,8 +3253,10 @@ end if ((isone(ndims(_X)) || isone(ndims(AI))) && (length(_X) == length(AI))) || all(d -> size(_X, d) == size(AI, d), 1:max(ndims(_X), ndims(AI))) - @test any(isone, setindex!(A, _X, I...)) - @test any(isone, setindex!(A, _X, I..., 1)) + @test any(isone, setindex!(A, _X, I...)) || isempty(_X) + fill!(A, 0) + @test any(isone, setindex!(A, _X, I..., 1)) || isempty(_X) + fill!(A, 0) else @test_throws DimensionMismatch setindex!(A, _X, I...) @test_throws DimensionMismatch setindex!(A, _X, I..., 1)