Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multidimensional array indexing with any eltype #12273

Merged
merged 2 commits into from
Jul 27, 2015
+31 −39
Merged

Allow multidimensional array indexing with any eltype #12273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e1ce9d
Allow multidimensional array indexing with any eltype
mbauman Jul 23, 2015
47e24ec
Convert to_index before checkbounds in scalar indexing
mbauman Jul 26, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 22 additions & 19 deletions base/abstractarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,12 @@ end
Expr(:block, Expr(:meta, :inline), ex)
end

_checkbounds(sz, i::Integer) = 1 <= i <= sz
_checkbounds(sz, i::Real) = 1 <= to_index(i) <= sz
_checkbounds(sz, I::AbstractVector{Bool}) = length(I) == sz
_checkbounds(sz, r::Range{Int}) = (@_inline_meta; isempty(r) || (minimum(r) >= 1 && maximum(r) <= sz))
_checkbounds{T<:Real}(sz, r::Range{T}) = (@_inline_meta; _checkbounds(sz, to_index(r)))
_checkbounds(sz, i) = throw(ArgumentError("unable to check bounds for indices of type $(typeof(i))"))
_checkbounds(sz, i::Real) = 1 <= i <= sz
_checkbounds(sz, ::Colon) = true
function _checkbounds{T <: Real}(sz, I::AbstractArray{T})
_checkbounds(sz, r::Range) = (@_inline_meta; isempty(r) || (_checkbounds(sz, minimum(r)) && _checkbounds(sz,maximum(r))))
_checkbounds(sz, I::AbstractVector{Bool}) = length(I) == sz
function _checkbounds(sz, I::AbstractArray)
@_inline_meta
b = true
for i in I
Expand Down Expand Up @@ -487,8 +486,9 @@ _getindex(::LinearFast, A::AbstractArray, I::Int) = error("indexing not defined
function _getindex(::LinearFast, A::AbstractArray, I::Real...)
@_inline_meta
# We must check bounds for sub2ind; so we can then call unsafe_getindex
checkbounds(A, I...)
unsafe_getindex(A, sub2ind(size(A), to_indexes(I...)...))
J = to_indexes(I...)
checkbounds(A, J...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing the check before calling to_indexes is better-optimized for logical indexing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, but this is scalar indexing so I<:Tuple{Vararg{Real}}. It's something that I think we can get rid of once float indices are no longer supported, but that will take some thought about what to do with non-Int integers. We'll still want to convert small unsigned integers to Int before doing sub2ind arithmetic... and ideally also allow larger integers, too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point---I was still remembering the original thinking behind this code and not noticing what it had become. Great work as always.

unsafe_getindex(A, sub2ind(size(A), J...))
end
_unsafe_getindex(::LinearFast, A::AbstractArray, I::Int) = (@_inline_meta; getindex(A, I))
function _unsafe_getindex(::LinearFast, A::AbstractArray, I::Real...)
Expand All @@ -512,19 +512,20 @@ end
end
else
# Expand the last index into the appropriate number of indices
Isplat = Expr[:(to_index(I[$d])) for d = 1:N-1]
Jsplat = Expr[:(J[$d]) for d = 1:N-1]
i = 0
for d=N:AN
push!(Isplat, :(s[$(i+=1)]))
push!(Jsplat, :(s[$(i+=1)]))
end
sz = Expr(:tuple)
sz.args = Expr[:(size(A, $d)) for d=N:AN]
quote
$(Expr(:meta, :inline))
# ind2sub requires all dimensions to be nonzero, so checkbounds first
checkbounds(A, I...)
s = ind2sub($sz, to_index(I[$N]))
unsafe_getindex(A, $(Isplat...))
J = to_indexes(I...)
checkbounds(A, J...)
s = ind2sub($sz, J[$N])
unsafe_getindex(A, $(Jsplat...))
end
end
end
Expand Down Expand Up @@ -583,8 +584,9 @@ _setindex!(::LinearFast, A::AbstractArray, v, I::Int) = error("indexed assignmen
function _setindex!(::LinearFast, A::AbstractArray, v, I::Real...)
@_inline_meta
# We must check bounds for sub2ind; so we can then call unsafe_setindex!
checkbounds(A, I...)
unsafe_setindex!(A, v, sub2ind(size(A), to_indexes(I...)...))
J = to_indexes(I...)
checkbounds(A, J...)
unsafe_setindex!(A, v, sub2ind(size(A), J...))
end
_unsafe_setindex!(::LinearFast, A::AbstractArray, v, I::Int) = (@_inline_meta; setindex!(A, v, I))
function _unsafe_setindex!(::LinearFast, A::AbstractArray, v, I::Real...)
Expand All @@ -608,18 +610,19 @@ end
end
else
# Expand the last index into the appropriate number of indices
Isplat = Expr[:(to_index(I[$d])) for d = 1:N-1]
Jsplat = Expr[:(J[$d]) for d = 1:N-1]
i = 0
for d=N:AN
push!(Isplat, :(s[$(i+=1)]))
push!(Jsplat, :(s[$(i+=1)]))
end
sz = Expr(:tuple)
sz.args = Expr[:(size(A, $d)) for d=N:AN]
quote
$(Expr(:meta, :inline))
checkbounds(A, I...)
J = to_indexes(I...)
checkbounds(A, J...)
s = ind2sub($sz, to_index(I[$N]))
unsafe_setindex!(A, v, $(Isplat...))
unsafe_setindex!(A, v, $(Jsplat...))
end
end
end
Expand Down
11 changes: 1 addition & 10 deletions base/deprecated.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,16 +417,7 @@ function to_index(i::Real)
to_index_nodep(i)
end

function to_index{T<:Real}(r::UnitRange{T})
depwarn("indexing with non Integer UnitRanges is deprecated", :to_index)
to_index_nodep(first(r)):to_index_nodep(last(r))
end

function to_index{T<:Real}(r::StepRange{T})
depwarn("indexing with non Integer StepRanges is deprecated", :to_index)
to_index_nodep(first(r)):to_index_nodep(step(r)):to_index_nodep(last(r))
end

to_index{T<:Integer}(A::AbstractArray{T}) = A
function to_index{T<:Real}(A::AbstractArray{T})
depwarn("indexing with non Integer AbstractArrays is deprecated", :to_index)
Int[to_index_nodep(x) for x in A]
Expand Down
15 changes: 5 additions & 10 deletions base/operators.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,20 +303,15 @@ function setindex_shape_check{T}(X::AbstractArray{T,2}, i::Int, j::Int)
end
setindex_shape_check(X, I::Int...) = nothing # Non-arrays broadcast to all idxs

# convert to integer index
# convert to a supported index type (Array, Colon, or Int)
to_index(i::Int) = i
to_index(i::Integer) = convert(Int,i)::Int
to_index(r::UnitRange{Int}) = r
to_index(r::Range{Int}) = r
to_index(I::UnitRange{Bool}) = find(I)
to_index(I::Range{Bool}) = find(I)
to_index{T<:Integer}(r::UnitRange{T}) = to_index(first(r)):to_index(last(r))
to_index{T<:Integer}(r::StepRange{T}) = to_index(first(r)):to_index(step(r)):to_index(last(r))
to_index(c::Colon) = c
to_index(I::AbstractArray{Bool}) = find(I)
to_index(A::AbstractArray{Int}) = A
to_index{T<:Integer}(A::AbstractArray{T}) = [to_index(x) for x in A]
to_index(i) = error("invalid index: $i")
to_index(A::AbstractArray) = A
to_index{T<:AbstractArray}(A::AbstractArray{T}) = throw(ArgumentError("invalid index: $A"))
to_index(A::AbstractArray{Colon}) = throw(ArgumentError("invalid index: $A"))
to_index(i) = throw(ArgumentError("invalid index: $i"))

to_indexes() = ()
to_indexes(i1) = (to_index(i1),)
Expand Down
3 changes: 3 additions & 0 deletions test/abstractarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,9 @@ function test_vector_indexing{T}(::Type{T}, shape, ::Type{TestAbstractArray})
@test B[1:end] == A[1:end] == collect(1:N)
@test B[:,:] == A[:,:] == reshape(1:N, shape[1], prod(shape[2:end]))
@test B[1:end,1:end] == A[1:end,1:end] == reshape(1:N, shape[1], prod(shape[2:end]))
# Test with containers that aren't Int[]
@test B[[]] == A[[]] == []
@test B[convert(Array{Any}, idxs)] == A[convert(Array{Any}, idxs)] == idxs
end

function test_primitives{T}(::Type{T}, shape, ::Type{TestAbstractArray})
Expand Down