From 0d88892fade7cb8182dcaf0a43df3b3bec6b4a9b Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Thu, 12 May 2016 22:18:03 -0500 Subject: [PATCH] Rework bounds-checking (again) --- base/abstractarray.jl | 78 +++++++++++++++++++++++----------------- base/deprecated.jl | 10 ++++-- base/multidimensional.jl | 42 ++-------------------- test/abstractarray.jl | 23 +++++++++--- 4 files changed, 74 insertions(+), 79 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 0d171663ac180..148703d33a770 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -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 @@ -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 @@ -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)) diff --git a/base/deprecated.jl b/base/deprecated.jl index cd12f64a06272..2d0ad0524f80a 100644 --- a/base/deprecated.jl +++ b/base/deprecated.jl @@ -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 diff --git a/base/multidimensional.jl b/base/multidimensional.jl index 9473d27bd6523..3a33ef8800dcd 100644 --- a/base/multidimensional.jl +++ b/base/multidimensional.jl @@ -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 diff --git a/test/abstractarray.jl b/test/abstractarray.jl index 81a02c03034f3..95c9e4453842a 100644 --- a/test/abstractarray.jl +++ b/test/abstractarray.jl @@ -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 @@ -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