Skip to content

Commit

Permalink
Convert to_index before checkbounds in scalar indexing
Browse files Browse the repository at this point in the history 
I think the behavior here is more flexible and gives better error messages at the same time:

```jl
julia> A[[]]
0-element Array{Int64,1}

julia> A[im]
ERROR: indexing Array{Int64,3} with types Tuple{Complex{Bool}} is not supported
 in error at ./error.jl:21
 in getindex at abstractarray.jl:463

julia> A[[im]]
ERROR: ArgumentError: unable to check bounds for indices of type Complex{Bool}
 in getindex at abstractarray.jl:463

julia> A[Vector[[1,2],[3,4]]]
ERROR: ArgumentError: invalid index: Array{T,1}[[1,2],[3,4]]
 in getindex at abstractarray.jl:463
```
  • Loading branch information
@mbauman
mbauman committed Jul 26, 2015
1 parent 9e1ce9d commit 47e24ec
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 38 deletions.
39 changes: 21 additions & 18 deletions base/abstractarray.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,11 @@ 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
_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
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...)
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
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
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) = 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

0 comments on commit 47e24ec

Please sign in to comment.