Skip to content

Commit fe47a00

Browse files
timholytimholy
committed
Rework bounds-checking (again)
1 parent 7f3981f commit fe47a00

File tree

4 files changed

+73
-77
lines changed

4 files changed

+73
-77
lines changed

‎base/abstractarray.jl

+46-32
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ linearindexing(::LinearIndexing, ::LinearIndexing) = LinearSlow()
106106

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

117118
# check along a single dimension
118-
checkbounds(::Type{Bool}, inds::UnitRange, i) = throw(ArgumentError("unable to check bounds for indices of type $(typeof(i))"))
119-
checkbounds(::Type{Bool}, inds::UnitRange, i::Real) = first(inds) <= i <= last(inds)
120-
checkbounds(::Type{Bool}, inds::UnitRange, ::Colon) = true
121-
function checkbounds(::Type{Bool}, inds::UnitRange, r::Range)
119+
checkindex(::Type{Bool}, inds::UnitRange, i) = throw(ArgumentError("unable to check bounds for indices of type $(typeof(i))"))
120+
checkindex(::Type{Bool}, inds::UnitRange, i::Real) = (first(inds) <= i) & (i <= last(inds))
121+
checkindex(::Type{Bool}, inds::UnitRange, ::Colon) = true
122+
function checkindex(::Type{Bool}, inds::UnitRange, r::Range)
122123
@_propagate_inbounds_meta
123-
isempty(r) | (checkbounds(Bool, inds, first(r)) & checkbounds(Bool, inds, last(r)))
124+
isempty(r) | (checkindex(Bool, inds, first(r)) & checkindex(Bool, inds, last(r)))
124125
end
125-
checkbounds{N}(::Type{Bool}, indx::UnitRange, I::AbstractArray{Bool,N}) = N == 1 && indx == indices(I,1)
126-
function checkbounds(::Type{Bool}, inds::UnitRange, I::AbstractArray)
126+
checkindex{N}(::Type{Bool}, indx::UnitRange, I::AbstractArray{Bool,N}) = N == 1 && indx == indices(I,1)
127+
function checkindex(::Type{Bool}, inds::UnitRange, I::AbstractArray)
127128
@_inline_meta
128129
b = true
129130
for i in I
130-
b &= checkbounds(Bool, inds, i)
131+
b &= checkindex(Bool, inds, i)
131132
end
132133
b
133134
end
134135

135-
# check all dimensions
136-
function checkbounds{N,T}(::Type{Bool}, inds::NTuple{N,UnitRange}, I1::T, I...)
136+
# check all indices/dimensions
137+
function checkbounds(::Type{Bool}, A::AbstractArray, I...)
137138
@_inline_meta
138-
checkbounds(Bool, inds[1], I1) & checkbounds(Bool, tail(inds), I...)
139+
# checked::NTuple{M} means we have checked dimensions 1:M-1, now
140+
# need to check dimension M. checked[M] indicates whether all the
141+
# previous ones are in-bounds.
142+
# By growing checked, it allows us to test whether we've processed
143+
# the same number of dimensions as the array, even while
144+
# supporting CartesianIndex
145+
checked = (true,)
146+
_chkbnds(A, (true,), I...)
147+
end
148+
# Single logical array indexing:
149+
_chkbnds(A::AbstractArray, ::NTuple{1,Bool}, I::AbstractArray{Bool}) = indices(A) == indices(I)
150+
_chkbnds(A::AbstractVector, ::NTuple{1,Bool}, I::AbstractVector{Bool}) = indices(A) == indices(I)
151+
_chkbnds(A::AbstractArray, ::NTuple{1,Bool}, I::AbstractVector{Bool}) = length(A) == length(I)
152+
# When all indices have been checked:
153+
_chkbnds{M}(A, checked::NTuple{M,Bool}) = checked[M]
154+
# When the number of indices matches the array dimensionality:
155+
function _chkbnds{T,N}(A::AbstractArray{T,N}, checked::NTuple{N,Bool}, I1)
156+
@_inline_meta
157+
checked[N] & checkindex(Bool, indices(A, N), I1)
139158
end
140-
checkbounds(::Type{Bool}, inds::Tuple{UnitRange}, I1) = (@_inline_meta; checkbounds(Bool, inds[1], I1))
141-
checkbounds{N}(::Type{Bool}, inds::NTuple{N,UnitRange}, I1) = (@_inline_meta; checkbounds(Bool, 1:prod(map(length, inds)), I1)) # TODO: eliminate (partial linear indexing)
142-
checkbounds{N}(::Type{Bool}, inds::NTuple{N,UnitRange}) = (@_inline_meta; checkbounds(Bool, inds, 1)) # for a[]
143-
144-
checkbounds(::Type{Bool}, inds::Tuple{}, i) = (@_inline_meta; checkbounds(Bool, 1:1, i))
145-
function checkbounds(::Type{Bool}, inds::Tuple{}, i, I...)
159+
# When the last checked dimension is not equal to the array dimensionality:
160+
# TODO: when deprecating partial linear indexing, change to 1:trailingsize(...) to 1:1
161+
function _chkbnds{T,N,M}(A::AbstractArray{T,N}, checked::NTuple{M,Bool}, I1)
146162
@_inline_meta
147-
checkbounds(Bool, 1:1, i) & checkbounds(Bool, (), I...)
163+
checked[M] & checkindex(Bool, 1:trailingsize(A, Val{M}), I1)
148164
end
149-
# Prevent allocation of a GC frame by hiding the BoundsError in a noinline function
165+
# Checking an interior dimension:
166+
function _chkbnds{T,N,M}(A::AbstractArray{T,N}, checked::NTuple{M,Bool}, I1, I...)
167+
@_inline_meta
168+
# grow checked by one
169+
newchecked = (checked..., checked[M] & checkindex(Bool, indices(A, M), I1))
170+
_chkbnds(A, newchecked, I...)
171+
end
172+
150173
throw_boundserror(A, I) = (@_noinline_meta; throw(BoundsError(A, I)))
151174

152-
# Don't define index types on checkbounds to make extending easier
153-
checkbounds(A::AbstractArray, I...) = (@_inline_meta; _internal_checkbounds(A, I...))
154-
# The internal function is named _internal_checkbounds since there had been a
155-
# _checkbounds previously that meant something different.
156-
_internal_checkbounds(A::AbstractArray) = true
157-
_internal_checkbounds(A::AbstractArray, I::AbstractArray{Bool}) = indices(A) == indices(I) || throw_boundserror(A, I)
158-
_internal_checkbounds(A::AbstractVector, I::AbstractVector{Bool}) = indices(A) == indices(I) || throw_boundserror(A, I)
159-
_internal_checkbounds(A::AbstractArray, I::AbstractVector{Bool}) = length(A) == length(I) || throw_boundserror(A, I)
160-
function _internal_checkbounds(A::AbstractArray, I1, I...)
161-
# having I1 seems important for good codegen
175+
function checkbounds(A::AbstractArray, I...)
162176
@_inline_meta
163-
checkbounds(Bool, indices(A), I1, I...) || throw_boundserror(A, (I1, I...))
177+
checkbounds(Bool, A, I...) || throw_boundserror(A, I)
164178
end
165179

166180
# See also specializations in multidimensional
@@ -584,9 +598,9 @@ end
584598

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

587-
get(A::AbstractArray, i::Integer, default) = checkbounds(Bool, indices(A), i) ? A[i] : default
601+
get(A::AbstractArray, i::Integer, default) = checkbounds(Bool, A, i) ? A[i] : default
588602
get(A::AbstractArray, I::Tuple{}, default) = similar(A, typeof(default), 0)
589-
get(A::AbstractArray, I::Dims, default) = checkbounds(Bool, indices(A), I...) ? A[I...] : default
603+
get(A::AbstractArray, I::Dims, default) = checkbounds(Bool, A, I...) ? A[I...] : default
590604

591605
function get!{T}(X::AbstractArray{T}, A::AbstractArray, I::Union{Range, AbstractVector{Int}}, default::T)
592606
ind = findin(I, 1:length(A))

‎base/deprecated.jl

+7-3
Original file line numberDiff line numberDiff line change
@@ -1152,12 +1152,16 @@ isequal(x::Char, y::Integer) = false
11521152
isequal(x::Integer, y::Char) = false
11531153

11541154
function checkbounds(::Type{Bool}, sz::Integer, i)
1155-
depwarn("checkbounds(Bool, size(A, d), i) is deprecated, use checkbounds(Bool, indices(A, d), i).", :checkbounds)
1155+
depwarn("checkbounds(Bool, size(A, d), i) is deprecated, use checkindex(Bool, indices(A, d), i).", :checkbounds)
11561156
checkbounds(Bool, 1:sz, i)
11571157
end
1158+
immutable FakeArray{T,N} <: AbstractArray{T,N}
1159+
dims::NTuple{N,Int}
1160+
end
1161+
size(A::FakeArray) = A.dims
11581162
function checkbounds{N,T}(::Type{Bool}, sz::NTuple{N,Integer}, I1::T, I...)
1159-
depwarn("checkbounds(Bool, size(A), I...) is deprecated, use checkbounds(Bool, indices(A), I...).", :checkbounds)
1160-
checkbounds(Bool, map(s->1:s, sz), I1, I...)
1163+
depwarn("checkbounds(Bool, size(A), I...) is deprecated, use checkbounds(Bool, A, I...).", :checkbounds)
1164+
checkbounds(Bool, FakeArray(sz), I1, I...)
11611165
end
11621166

11631167
# During the 0.5 development cycle, do not add any deprecations below this line

‎base/multidimensional.jl

+1-38
Original file line numberDiff line numberDiff line change
@@ -193,45 +193,8 @@ end # IteratorsMD
193193

194194
using .IteratorsMD
195195

196-
# Bounds-checking specialization
197-
# Specializing for a fixed number of arguments provides a ~25%
198-
# improvement over the general definitions in abstractarray.jl
199-
for N = 1:5
200-
args = [:($(Symbol(:I, d))) for d = 1:N]
201-
targs = [:($(Symbol(:I, d))::Union{Colon,Number,AbstractArray}) for d = 1:N] # prevent co-opting the CartesianIndex version
202-
exs = [:(checkbounds(Bool, indices(A, $d), $(args[d]))) for d = 1:N]
203-
cbexpr = exs[1]
204-
for d = 2:N
205-
cbexpr = :($(exs[d]) & $cbexpr)
206-
end
207-
@eval begin
208-
function checkbounds(A::AbstractArray, $(args...))
209-
@_inline_meta
210-
_internal_checkbounds(A, $(args...))
211-
end
212-
function _internal_checkbounds{T}(A::AbstractArray{T,$N}, $(targs...))
213-
@_inline_meta
214-
($cbexpr) || throw_boundserror(A, ($(args...),))
215-
end
216-
end
217-
end
218-
# This is annoying, but we must also define logical indexing to avoid ambiguities
219-
_internal_checkbounds(A::AbstractVector, I::AbstractArray{Bool}) = size(A) == size(I) || throw_boundserror(A, I)
220-
_internal_checkbounds(A::AbstractVector, I::AbstractVector{Bool}) = length(A) == length(I) || throw_boundserror(A, I)
221-
222196
# Bounds-checking with CartesianIndex
223-
@inline function checkbounds(::Type{Bool}, ::Tuple{}, I1::CartesianIndex)
224-
checkbounds(Bool, (), I1.I...)
225-
end
226-
@inline function checkbounds(::Type{Bool}, inds::Tuple{}, I1::CartesianIndex, I...)
227-
checkbounds(Bool, (), I1.I..., I...)
228-
end
229-
@inline function checkbounds(::Type{Bool}, inds::Tuple{UnitRange}, I1::CartesianIndex)
230-
checkbounds(Bool, inds, I1.I..., I...)
231-
end
232-
@inline function checkbounds{N}(::Type{Bool}, inds::NTuple{N,UnitRange}, I1::CartesianIndex, I...)
233-
checkbounds(Bool, inds, I1.I..., I...)
234-
end
197+
@inline _chkbnds{T,N,M}(A::AbstractArray{T,N}, checked::NTuple{M,Bool}, I1::CartesianIndex, I...) = _chkbnds(A, checked, I1.I..., I...)
235198

236199
# Recursively compute the lengths of a list of indices, without dropping scalars
237200
# These need to be inlined for more than 3 indexes

‎test/abstractarray.jl

+19-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,19 @@
11
# This file is a part of Julia. License is MIT: http://julialang.org/license
22

3+
# Bounds checking
4+
A = rand(3,3,3)
5+
@test checkbounds(Bool, A, 1, 1, 1) == true
6+
@test checkbounds(Bool, A, 1, 3, 3) == true
7+
@test checkbounds(Bool, A, 1, 4, 3) == false
8+
@test checkbounds(Bool, A, 1, -1, 3) == false
9+
@test checkbounds(Bool, A, 1, 9) == true # partial linear indexing
10+
@test checkbounds(Bool, A, 1, 10) == false # partial linear indexing
11+
@test checkbounds(Bool, A, 1) == true
12+
@test checkbounds(Bool, A, 27) == true
13+
@test checkbounds(Bool, A, 28) == false
14+
@test checkbounds(Bool, A, 2, 2, 2, 1) == true
15+
@test checkbounds(Bool, A, 2, 2, 2, 2) == false
16+
317
# token type on which to dispatch testing methods in order to avoid potential
418
# name conflicts elsewhere in the base test suite
519
type TestAbstractArray end
@@ -254,12 +268,13 @@ end
254268

255269
function test_in_bounds(::Type{TestAbstractArray})
256270
n = rand(2:5)
257-
inds = ntuple(d->1:rand(2:5), n)
258-
len = prod(map(length, inds))
271+
sz = rand(2:5, n)
272+
len = prod(sz)
273+
A = zeros(sz...)
259274
for i in 1:len
260-
@test checkbounds(Bool, inds, i) == true
275+
@test checkbounds(Bool, A, i) == true
261276
end
262-
@test checkbounds(Bool, inds, len + 1) == false
277+
@test checkbounds(Bool, A, len + 1) == false
263278
end
264279

265280
type UnimplementedFastArray{T, N} <: AbstractArray{T, N} end

0 commit comments

Comments
 (0)