Skip to content

Commit

Permalink
Rework bounds-checking (again)
Browse files Browse the repository at this point in the history
  • Loading branch information
@timholy
timholy committed May 13, 2016
1 parent 9e14448 commit 0d88892
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 79 deletions.
78 changes: 46 additions & 32 deletions base/abstractarray.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ linearindexing(::LinearIndexing, ::LinearIndexing) = LinearSlow()

## Bounds checking ##
@generated function trailingsize{T,N,n}(A::AbstractArray{T,N}, ::Type{Val{n}})
(isa(n, Int) && isa(N, Int)) || error("Must have concrete type")
n > N && return 1
ex = :(size(A, $n))
for m = n+1:N
Expand All @@ -115,52 +116,65 @@ linearindexing(::LinearIndexing, ::LinearIndexing) = LinearSlow()
end

# check along a single dimension
checkbounds(::Type{Bool}, inds::UnitRange, i) = throw(ArgumentError("unable to check bounds for indices of type $(typeof(i))"))
checkbounds(::Type{Bool}, inds::UnitRange, i::Real) = first(inds) <= i <= last(inds)
checkbounds(::Type{Bool}, inds::UnitRange, ::Colon) = true
function checkbounds(::Type{Bool}, inds::UnitRange, r::Range)
checkindex(::Type{Bool}, inds::UnitRange, i) = throw(ArgumentError("unable to check bounds for indices of type $(typeof(i))"))
checkindex(::Type{Bool}, inds::UnitRange, i::Real) = (first(inds) <= i) & (i <= last(inds))
checkindex(::Type{Bool}, inds::UnitRange, ::Colon) = true
function checkindex(::Type{Bool}, inds::UnitRange, r::Range)
@_propagate_inbounds_meta
isempty(r) | (checkbounds(Bool, inds, first(r)) & checkbounds(Bool, inds, last(r)))
isempty(r) | (checkindex(Bool, inds, first(r)) & checkindex(Bool, inds, last(r)))
end
checkbounds{N}(::Type{Bool}, indx::UnitRange, I::AbstractArray{Bool,N}) = N == 1 && indx == indices(I,1)
function checkbounds(::Type{Bool}, inds::UnitRange, I::AbstractArray)
checkindex{N}(::Type{Bool}, indx::UnitRange, I::AbstractArray{Bool,N}) = N == 1 && indx == indices(I,1)
function checkindex(::Type{Bool}, inds::UnitRange, I::AbstractArray)
@_inline_meta
b = true
for i in I
b &= checkbounds(Bool, inds, i)
b &= checkindex(Bool, inds, i)
end
b
end

# check all dimensions
function checkbounds{N,T}(::Type{Bool}, inds::NTuple{N,UnitRange}, I1::T, I...)
# check all indices/dimensions
function checkbounds(::Type{Bool}, A::AbstractArray, I...)
@_inline_meta
checkbounds(Bool, inds[1], I1) & checkbounds(Bool, tail(inds), I...)
# checked::NTuple{M} means we have checked dimensions 1:M-1, now
# need to check dimension M. checked[M] indicates whether all the
# previous ones are in-bounds.
# By growing checked, it allows us to test whether we've processed
# the same number of dimensions as the array, even while
# supporting CartesianIndex
checked = (true,)
_chkbnds(A, (true,), I...)
end
# Single logical array indexing:
_chkbnds(A::AbstractArray, ::NTuple{1,Bool}, I::AbstractArray{Bool}) = indices(A) == indices(I)
_chkbnds(A::AbstractVector, ::NTuple{1,Bool}, I::AbstractVector{Bool}) = indices(A) == indices(I)
_chkbnds(A::AbstractArray, ::NTuple{1,Bool}, I::AbstractVector{Bool}) = length(A) == length(I)
# When all indices have been checked:
_chkbnds{M}(A, checked::NTuple{M,Bool}) = checked[M]
# When the number of indices matches the array dimensionality:
function _chkbnds{T,N}(A::AbstractArray{T,N}, checked::NTuple{N,Bool}, I1)
@_inline_meta
checked[N] & checkindex(Bool, indices(A, N), I1)
end
checkbounds(::Type{Bool}, inds::Tuple{UnitRange}, I1) = (@_inline_meta; checkbounds(Bool, inds[1], I1))
checkbounds{N}(::Type{Bool}, inds::NTuple{N,UnitRange}, I1) = (@_inline_meta; checkbounds(Bool, 1:prod(map(length, inds)), I1)) # TODO: eliminate (partial linear indexing)
checkbounds{N}(::Type{Bool}, inds::NTuple{N,UnitRange}) = (@_inline_meta; checkbounds(Bool, inds, 1)) # for a[]

checkbounds(::Type{Bool}, inds::Tuple{}, i) = (@_inline_meta; checkbounds(Bool, 1:1, i))
function checkbounds(::Type{Bool}, inds::Tuple{}, i, I...)
# When the last checked dimension is not equal to the array dimensionality:
# TODO: when deprecating partial linear indexing, change to 1:trailingsize(...) to 1:1
function _chkbnds{T,N,M}(A::AbstractArray{T,N}, checked::NTuple{M,Bool}, I1)
@_inline_meta
checkbounds(Bool, 1:1, i) & checkbounds(Bool, (), I...)
checked[M] & checkindex(Bool, 1:trailingsize(A, Val{M}), I1)
end
# Prevent allocation of a GC frame by hiding the BoundsError in a noinline function
# Checking an interior dimension:
function _chkbnds{T,N,M}(A::AbstractArray{T,N}, checked::NTuple{M,Bool}, I1, I...)
@_inline_meta
# grow checked by one
newchecked = (checked..., checked[M] & checkindex(Bool, indices(A, M), I1))
_chkbnds(A, newchecked, I...)
end

throw_boundserror(A, I) = (@_noinline_meta; throw(BoundsError(A, I)))

# Don't define index types on checkbounds to make extending easier
checkbounds(A::AbstractArray, I...) = (@_inline_meta; _internal_checkbounds(A, I...))
# The internal function is named _internal_checkbounds since there had been a
# _checkbounds previously that meant something different.
_internal_checkbounds(A::AbstractArray) = true
_internal_checkbounds(A::AbstractArray, I::AbstractArray{Bool}) = indices(A) == indices(I) || throw_boundserror(A, I)
_internal_checkbounds(A::AbstractVector, I::AbstractVector{Bool}) = indices(A) == indices(I) || throw_boundserror(A, I)
_internal_checkbounds(A::AbstractArray, I::AbstractVector{Bool}) = length(A) == length(I) || throw_boundserror(A, I)
function _internal_checkbounds(A::AbstractArray, I1, I...)
# having I1 seems important for good codegen
function checkbounds(A::AbstractArray, I...)
@_inline_meta
checkbounds(Bool, indices(A), I1, I...) || throw_boundserror(A, (I1, I...))
checkbounds(Bool, A, I...) || throw_boundserror(A, I)
end

# See also specializations in multidimensional
Expand Down Expand Up @@ -583,9 +597,9 @@ end

typealias RangeVecIntList{A<:AbstractVector{Int}} Union{Tuple{Vararg{Union{Range, AbstractVector{Int}}}}, AbstractVector{UnitRange{Int}}, AbstractVector{Range{Int}}, AbstractVector{A}}

get(A::AbstractArray, i::Integer, default) = checkbounds(Bool, indices(A), i) ? A[i] : default
get(A::AbstractArray, i::Integer, default) = checkbounds(Bool, A, i) ? A[i] : default
get(A::AbstractArray, I::Tuple{}, default) = similar(A, typeof(default), 0)
get(A::AbstractArray, I::Dims, default) = checkbounds(Bool, indices(A), I...) ? A[I...] : default
get(A::AbstractArray, I::Dims, default) = checkbounds(Bool, A, I...) ? A[I...] : default

function get!{T}(X::AbstractArray{T}, A::AbstractArray, I::Union{Range, AbstractVector{Int}}, default::T)
ind = findin(I, 1:length(A))
Expand Down
10 changes: 7 additions & 3 deletions base/deprecated.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1152,12 +1152,16 @@ isequal(x::Char, y::Integer) = false
isequal(x::Integer, y::Char) = false

function checkbounds(::Type{Bool}, sz::Integer, i)
depwarn("checkbounds(Bool, size(A, d), i) is deprecated, use checkbounds(Bool, indices(A, d), i).", :checkbounds)
depwarn("checkbounds(Bool, size(A, d), i) is deprecated, use checkindex(Bool, indices(A, d), i).", :checkbounds)
checkbounds(Bool, 1:sz, i)
end
immutable FakeArray{T,N} <: AbstractArray{T,N}
dims::NTuple{N,Int}
end
size(A::FakeArray) = A.dims
function checkbounds{N,T}(::Type{Bool}, sz::NTuple{N,Integer}, I1::T, I...)
depwarn("checkbounds(Bool, size(A), I...) is deprecated, use checkbounds(Bool, indices(A), I...).", :checkbounds)
checkbounds(Bool, map(s->1:s, sz), I1, I...)
depwarn("checkbounds(Bool, size(A), I...) is deprecated, use checkbounds(Bool, A, I...).", :checkbounds)
checkbounds(Bool, FakeArray(sz), I1, I...)
end

# During the 0.5 development cycle, do not add any deprecations below this line
Expand Down
42 changes: 2 additions & 40 deletions base/multidimensional.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,47 +193,9 @@ end # IteratorsMD

using .IteratorsMD

# Bounds-checking specialization
# Specializing for a fixed number of arguments provides a ~25%
# improvement over the general definitions in abstractarray.jl

# This is annoying, but we must first define logical indexing to avoid ambiguities
_internal_checkbounds(A::AbstractVector, I::AbstractVector{Bool}) = length(A) == length(I) || throw_boundserror(A, I)
_internal_checkbounds(A::AbstractVector, I::AbstractArray{Bool}) = size(A) == size(I) || throw_boundserror(A, I)

for N = 1:5
args = [:($(Symbol(:I, d))) for d = 1:N]
targs = [:($(Symbol(:I, d))::Union{Colon,Number,AbstractArray}) for d = 1:N] # prevent co-opting the CartesianIndex version
exs = [:(checkbounds(Bool, indices(A, $d), $(args[d]))) for d = 1:N]
cbexpr = exs[1]
for d = 2:N
cbexpr = :($(exs[d]) & $cbexpr)
end
@eval begin
function checkbounds(A::AbstractArray, $(args...))
@_inline_meta
_internal_checkbounds(A, $(args...))
end
function _internal_checkbounds{T}(A::AbstractArray{T,$N}, $(targs...))
@_inline_meta
($cbexpr) || throw_boundserror(A, ($(args...),))
end
end
end

# Bounds-checking with CartesianIndex
@inline function checkbounds(::Type{Bool}, ::Tuple{}, I1::CartesianIndex)
checkbounds(Bool, (), I1.I...)
end
@inline function checkbounds(::Type{Bool}, inds::Tuple{}, I1::CartesianIndex, I...)
checkbounds(Bool, (), I1.I..., I...)
end
@inline function checkbounds(::Type{Bool}, inds::Tuple{UnitRange}, I1::CartesianIndex)
checkbounds(Bool, inds, I1.I..., I...)
end
@inline function checkbounds{N}(::Type{Bool}, inds::NTuple{N,UnitRange}, I1::CartesianIndex, I...)
checkbounds(Bool, inds, I1.I..., I...)
end
@inline _chkbnds{T,N}(A::AbstractArray{T,N}, checked::NTuple{N,Bool}, I1::CartesianIndex, I...) = _chkbnds(A, checked, I1.I..., I...)
@inline _chkbnds{T,N,M}(A::AbstractArray{T,N}, checked::NTuple{M,Bool}, I1::CartesianIndex, I...) = _chkbnds(A, checked, I1.I..., I...)

# Recursively compute the lengths of a list of indices, without dropping scalars
# These need to be inlined for more than 3 indexes
Expand Down
23 changes: 19 additions & 4 deletions test/abstractarray.jl
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
# This file is a part of Julia. License is MIT: http://julialang.org/license

# Bounds checking
A = rand(3,3,3)
@test checkbounds(Bool, A, 1, 1, 1) == true
@test checkbounds(Bool, A, 1, 3, 3) == true
@test checkbounds(Bool, A, 1, 4, 3) == false
@test checkbounds(Bool, A, 1, -1, 3) == false
@test checkbounds(Bool, A, 1, 9) == true # partial linear indexing
@test checkbounds(Bool, A, 1, 10) == false # partial linear indexing
@test checkbounds(Bool, A, 1) == true
@test checkbounds(Bool, A, 27) == true
@test checkbounds(Bool, A, 28) == false
@test checkbounds(Bool, A, 2, 2, 2, 1) == true
@test checkbounds(Bool, A, 2, 2, 2, 2) == false

# token type on which to dispatch testing methods in order to avoid potential
# name conflicts elsewhere in the base test suite
type TestAbstractArray end
Expand Down Expand Up @@ -254,12 +268,13 @@ end

function test_in_bounds(::Type{TestAbstractArray})
n = rand(2:5)
inds = ntuple(d->1:rand(2:5), n)
len = prod(map(length, inds))
sz = rand(2:5, n)
len = prod(sz)
A = zeros(sz...)
for i in 1:len
@test checkbounds(Bool, inds, i) == true
@test checkbounds(Bool, A, i) == true
end
@test checkbounds(Bool, inds, len + 1) == false
@test checkbounds(Bool, A, len + 1) == false
end

type UnimplementedFastArray{T, N} <: AbstractArray{T, N} end
Expand Down

0 comments on commit 0d88892

Please sign in to comment.