Skip to content

Commit

Permalink
Require all arrays in eachindex to have the same axes. Fixes #13310 (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
timholy authored Dec 15, 2017
1 parent 15d87e7 commit 9b8e651
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 19 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,9 @@ This section lists changes that do not have deprecation warnings.
points in reverse order; any string type overrides `reverse` to return a different
type of string must also override `reverseind` to compute reversed indices correctly.

* `eachindex(A, B...)` now requires that all inputs have the same number of elements.
When the chosen indexing is Cartesian, they must have the same axes.

Library improvements
--------------------

Expand Down
9 changes: 3 additions & 6 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -843,12 +843,9 @@ end
eachindex(::IndexLinear, A::AbstractArray) = linearindices(A)
function eachindex(::IndexLinear, A::AbstractArray, B::AbstractArray...)
@_inline_meta
1:_maxlength(A, B...)
end
_maxlength(A) = length(A)
function _maxlength(A, B, C...)
@_inline_meta
max(length(A), _maxlength(B, C...))
indsA = linearindices(A)
all(x->linearindices(x) == indsA, B) || throw_eachindex_mismatch(IndexLinear(), A, B...)
indsA
end

isempty(a::AbstractArray) = (_length(a) == 0)
Expand Down
14 changes: 5 additions & 9 deletions base/multidimensional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -228,15 +228,11 @@ module IteratorsMD

eachindex(::IndexCartesian, A::AbstractArray) = CartesianRange(axes(A))

@inline eachindex(::IndexCartesian, A::AbstractArray, B::AbstractArray...) =
CartesianRange(maxsize(A, B...))
maxsize() = ()
@inline maxsize(A) = size(A)
@inline maxsize(A, B...) = maxt(size(A), maxsize(B...))
@inline maxt(a::Tuple{}, b::Tuple{}) = ()
@inline maxt(a::Tuple{}, b::Tuple) = b
@inline maxt(a::Tuple, b::Tuple{}) = a
@inline maxt(a::Tuple, b::Tuple) = (max(a[1], b[1]), maxt(tail(a), tail(b))...)
@inline function eachindex(::IndexCartesian, A::AbstractArray, B::AbstractArray...)
axsA = axes(A)
all(x->axes(x) == axsA, B) || Base.throw_eachindex_mismatch(IndexCartesian(), A, B...)
CartesianRange(axsA)
end

eltype(R::CartesianRange) = eltype(typeof(R))
eltype(::Type{CartesianRange{N}}) where {N} = CartesianIndex{N}
Expand Down
7 changes: 7 additions & 0 deletions base/replutil.jl
Original file line number Diff line number Diff line change
Expand Up @@ -713,3 +713,10 @@ Such a method is usually undesirable to be displayed to the user in the REPL.
function is_default_method(m::Method)
return m.module == Base && m.file == Symbol("sysimg.jl") && m.sig == Tuple{Type{T},Any} where T
end

@noinline function throw_eachindex_mismatch(::IndexLinear, A...)
throw(DimensionMismatch("all inputs to eachindex must have the same indices, got $(join(linearindices.(A), ", ", " and "))"))
end
@noinline function throw_eachindex_mismatch(::IndexCartesian, A...)
throw(DimensionMismatch("all inputs to eachindex must have the same axes, got $(join(axes.(A), ", ", " and "))"))
end
14 changes: 10 additions & 4 deletions test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1636,10 +1636,16 @@ R = CartesianRange((0,3))
R = CartesianRange((3,0))
@test done(R, start(R)) == true

@test @inferred(eachindex(Base.IndexCartesian(),zeros(3),zeros(2,2),zeros(2,2,2),zeros(2,2))) == CartesianRange((3,2,2))
@test @inferred(eachindex(Base.IndexLinear(),zeros(3),zeros(2,2),zeros(2,2,2),zeros(2,2))) == 1:8
@test @inferred(eachindex(zeros(3),view(zeros(3,3),1:2,1:2),zeros(2,2,2),zeros(2,2))) == CartesianRange((3,2,2))
@test @inferred(eachindex(zeros(3),zeros(2,2),zeros(2,2,2),zeros(2,2))) == 1:8
@testset "multi-array eachindex" begin
local a = zeros(2,2)
local b = view(zeros(3,2), 1:2, :)
@test @inferred(eachindex(Base.IndexCartesian(), a, b)) == CartesianRange((2,2))
@test @inferred(eachindex(Base.IndexLinear(), a, b)) == 1:4
@test @inferred(eachindex(a, b)) == CartesianRange((2,2))
@test @inferred(eachindex(a, a)) == 1:4
@test_throws DimensionMismatch eachindex(a, rand(3,3))
@test_throws DimensionMismatch eachindex(b, rand(3,3))
end

@testset "rotates" begin
a = [1 0 0; 0 0 0]
Expand Down

0 comments on commit 9b8e651

Please sign in to comment.