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

Make no-index A[] check bounds like everyone else #24236

Merged
merged 3 commits into from
Oct 23, 2017
Merged

Make no-index A[] check bounds like everyone else #24236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
94e0fb6
Make no-index A[] check bounds like everyone else
mbauman Oct 20, 2017
c3789a5
Fixup tests
mbauman Oct 20, 2017
2ad31e9
Whitespace (SQUASH ME!)
mbauman Oct 20, 2017
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
29 changes: 8 additions & 21 deletions base/abstractarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,6 @@ function checkbounds(A::AbstractArray, I...)
checkbounds(Bool, A, I...) || throw_boundserror(A, I)
nothing
end
checkbounds(A::AbstractArray) = checkbounds(A, 1) # 0-d case

"""
checkbounds_indices(Bool, IA, I)
Expand Down Expand Up @@ -943,24 +942,18 @@ _getindex(::IndexStyle, A::AbstractArray, I...) =

## IndexLinear Scalar indexing: canonical method is one Int
_getindex(::IndexLinear, A::AbstractArray, i::Int) = (@_propagate_inbounds_meta; getindex(A, i))
_getindex(::IndexLinear, A::AbstractArray) = (@_propagate_inbounds_meta; getindex(A, _to_linear_index(A)))
function _getindex(::IndexLinear, A::AbstractArray, I::Vararg{Int,M}) where M
@_inline_meta
@boundscheck checkbounds(A, I...) # generally _to_linear_index requires bounds checking
@inbounds r = getindex(A, _to_linear_index(A, I...))
r
end
_to_linear_index(A::AbstractArray, i::Int) = i
_to_linear_index(A::AbstractVector, i::Int, I::Int...) = i # TODO: DEPRECATE FOR #14770
_to_linear_index(A::AbstractArray{T,N}, I::Vararg{Int,N}) where {T,N} = (@_inline_meta; sub2ind(A, I...))
_to_linear_index(A::AbstractArray) = 1 # TODO: DEPRECATE FOR #14770
_to_linear_index(A::AbstractArray, I::Int...) = (@_inline_meta; sub2ind(A, I...)) # TODO: DEPRECATE FOR #14770
_to_linear_index(A::AbstractVector, i::Int, I::Int...) = i
_to_linear_index(A::AbstractArray) = 1
_to_linear_index(A::AbstractArray, I::Int...) = (@_inline_meta; sub2ind(A, I...))

## IndexCartesian Scalar indexing: Canonical method is full dimensionality of Ints
function _getindex(::IndexCartesian, A::AbstractArray)
@_propagate_inbounds_meta
getindex(A, _to_subscript_indices(A)...)
end
function _getindex(::IndexCartesian, A::AbstractArray, I::Vararg{Int,M}) where M
@_inline_meta
@boundscheck checkbounds(A, I...) # generally _to_subscript_indices requires bounds checking
Expand All @@ -972,11 +965,11 @@ function _getindex(::IndexCartesian, A::AbstractArray{T,N}, I::Vararg{Int, N}) w
getindex(A, I...)
end
_to_subscript_indices(A::AbstractArray, i::Int) = (@_inline_meta; _unsafe_ind2sub(A, i))
_to_subscript_indices(A::AbstractArray{T,N}) where {T,N} = (@_inline_meta; fill_to_length((), 1, Val(N))) # TODO: DEPRECATE FOR #14770
_to_subscript_indices(A::AbstractArray{T,0}) where {T} = () # TODO: REMOVE FOR #14770
_to_subscript_indices(A::AbstractArray{T,0}, i::Int) where {T} = () # TODO: REMOVE FOR #14770
_to_subscript_indices(A::AbstractArray{T,0}, I::Int...) where {T} = () # TODO: DEPRECATE FOR #14770
function _to_subscript_indices(A::AbstractArray{T,N}, I::Int...) where {T,N} # TODO: DEPRECATE FOR #14770
_to_subscript_indices(A::AbstractArray{T,N}) where {T,N} = (@_inline_meta; fill_to_length((), 1, Val(N)))
_to_subscript_indices(A::AbstractArray{T,0}) where {T} = ()
_to_subscript_indices(A::AbstractArray{T,0}, i::Int) where {T} = ()
_to_subscript_indices(A::AbstractArray{T,0}, I::Int...) where {T} = ()
function _to_subscript_indices(A::AbstractArray{T,N}, I::Int...) where {T,N}
@_inline_meta
J, Jrem = IteratorsMD.split(I, Val(N))
_to_subscript_indices(A, J, Jrem)
Expand Down Expand Up @@ -1019,7 +1012,6 @@ _setindex!(::IndexStyle, A::AbstractArray, v, I...) =

## IndexLinear Scalar indexing
_setindex!(::IndexLinear, A::AbstractArray, v, i::Int) = (@_propagate_inbounds_meta; setindex!(A, v, i))
_setindex!(::IndexLinear, A::AbstractArray, v) = (@_propagate_inbounds_meta; setindex!(A, v, _to_linear_index(A)))
function _setindex!(::IndexLinear, A::AbstractArray, v, I::Vararg{Int,M}) where M
@_inline_meta
@boundscheck checkbounds(A, I...)
Expand All @@ -1032,10 +1024,6 @@ function _setindex!(::IndexCartesian, A::AbstractArray{T,N}, v, I::Vararg{Int, N
@_propagate_inbounds_meta
setindex!(A, v, I...)
end
function _setindex!(::IndexCartesian, A::AbstractArray, v)
@_propagate_inbounds_meta
setindex!(A, v, _to_subscript_indices(A)...)
end
function _setindex!(::IndexCartesian, A::AbstractArray, v, I::Vararg{Int,M}) where M
@_inline_meta
@boundscheck checkbounds(A, I...)
Expand Down Expand Up @@ -1072,7 +1060,6 @@ end

get(A::AbstractArray, I::AbstractRange, default) = get!(similar(A, typeof(default), index_shape(I)), A, I, default)

# TODO: DEPRECATE FOR #14770 (just the partial linear indexing part)
function get!(X::AbstractArray{T}, A::AbstractArray, I::RangeVecIntList, default::T) where T
fill!(X, default)
dst, src = indcopy(size(A), I)
Expand Down
5 changes: 2 additions & 3 deletions base/array.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ function getindex end

# This is more complicated than it needs to be in order to get Win64 through bootstrap
@eval getindex(A::Array, i1::Int) = arrayref($(Expr(:boundscheck)), A, i1)
@eval getindex(A::Array, i1::Int, i2::Int, I::Int...) = (@_inline_meta; arrayref($(Expr(:boundscheck)), A, i1, i2, I...)) # TODO: REMOVE FOR #14770
@eval getindex(A::Array, i1::Int, i2::Int, I::Int...) = (@_inline_meta; arrayref($(Expr(:boundscheck)), A, i1, i2, I...))

# Faster contiguous indexing using copy! for UnitRange and Colon
function getindex(A::Array, I::UnitRange{Int})
Expand Down Expand Up @@ -795,7 +795,7 @@ function setindex! end

@eval setindex!(A::Array{T}, x, i1::Int) where {T} = arrayset($(Expr(:boundscheck)), A, convert(T,x)::T, i1)
@eval setindex!(A::Array{T}, x, i1::Int, i2::Int, I::Int...) where {T} =
(@_inline_meta; arrayset($(Expr(:boundscheck)), A, convert(T,x)::T, i1, i2, I...)) # TODO: REMOVE FOR #14770
(@_inline_meta; arrayset($(Expr(:boundscheck)), A, convert(T,x)::T, i1, i2, I...))

# These are redundant with the abstract fallbacks but needed for bootstrap
function setindex!(A::Array, x, I::AbstractVector{Int})
Expand Down Expand Up @@ -2231,7 +2231,6 @@ end
findin(a, b) = _findin(a, b)

# Copying subregions
# TODO: DEPRECATE FOR #14770
function indcopy(sz::Dims, I::Vector)
n = length(I)
s = sz[n]
Expand Down
2 changes: 1 addition & 1 deletion base/subarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ parentindexes(a::AbstractArray) = ntuple(i->OneTo(size(a,i)), ndims(a))
_maybe_reshape_parent(A::AbstractArray, ::NTuple{1, Bool}) = reshape(A, Val(1))
_maybe_reshape_parent(A::AbstractArray{<:Any,1}, ::NTuple{1, Bool}) = reshape(A, Val(1))
_maybe_reshape_parent(A::AbstractArray{<:Any,N}, ::NTuple{N, Bool}) where {N} = A
_maybe_reshape_parent(A::AbstractArray, ::NTuple{N, Bool}) where {N} = reshape(A, Val(N)) # TODO: DEPRECATE FOR #14770
_maybe_reshape_parent(A::AbstractArray, ::NTuple{N, Bool}) where {N} = reshape(A, Val(N))
"""
view(A, inds...)

Expand Down
28 changes: 18 additions & 10 deletions test/abstractarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ function test_scalar_indexing(::Type{T}, shape, ::Type{TestAbstractArray}) where
end
end
# Test zero-dimensional accesses
@test A[] == B[] == A[1] == B[1] == 1
@test A[1] == B[1] == 1
# Test multidimensional scalar indexed assignment
C = T(Int, shape)
D1 = T(Int, shape)
Expand Down Expand Up @@ -361,9 +361,17 @@ function test_scalar_indexing(::Type{T}, shape, ::Type{TestAbstractArray}) where
end
@test C == B == A
# Test zero-dimensional setindex
A[] = 0; B[] = 0
@test A[] == B[] == 0
@test A == B
if length(A) == 1
A[] = 0; B[] = 0
@test A[] == B[] == 0
@test A == B
else
# TODO: Re-enable after PLI deprecation
# @test_throws BoundsError A[] = 0
# @test_throws BoundsError B[] = 0
# @test_throws BoundsError A[]
# @test_throws BoundsError B[]
end
end

function test_vector_indexing(::Type{T}, shape, ::Type{TestAbstractArray}) where T
Expand Down Expand Up @@ -489,10 +497,10 @@ function test_getindex_internals(::Type{T}, shape, ::Type{TestAbstractArray}) wh
A = reshape(collect(1:N), shape)
B = T(A)

@test getindex(A) == 1
@test getindex(B) == 1
@test Base.unsafe_getindex(A) == 1
@test Base.unsafe_getindex(B) == 1
@test getindex(A, 1) == 1
@test getindex(B, 1) == 1
@test Base.unsafe_getindex(A, 1) == 1
@test Base.unsafe_getindex(B, 1) == 1
end

function test_getindex_internals(::Type{TestAbstractArray})
Expand All @@ -509,8 +517,8 @@ function test_setindex!_internals(::Type{T}, shape, ::Type{TestAbstractArray}) w
A = reshape(collect(1:N), shape)
B = T(A)

Base.unsafe_setindex!(B, 1)
@test B[1] == 1
Base.unsafe_setindex!(B, 2, 1)
@test B[1] == 2
end

function test_setindex!_internals(::Type{TestAbstractArray})
Expand Down
13 changes: 10 additions & 3 deletions test/bitarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,16 @@ timesofar("constructors")
@testset "Indexing" begin
@testset "0d for size $sz" for (sz,T) in allsizes
b1 = rand!(falses(sz...))
@check_bit_operation getindex(b1) Bool
@check_bit_operation setindex!(b1, true) T
@check_bit_operation setindex!(b1, false) T
if length(b1) == 1
@check_bit_operation getindex(b1) Bool
@check_bit_operation setindex!(b1, true) T
@check_bit_operation setindex!(b1, false) T
else
# TODO: Re-enable after PLI deprecation is removed
# @test_throws getindex(b1)
# @test_throws setindex!(b1, true)
# @test_throws setindex!(b1, false)
end
end

@testset "linear for size $sz" for (sz,T) in allsizes[2:end]
Expand Down
2 changes: 1 addition & 1 deletion test/core.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2332,7 +2332,7 @@ end

# pull request #9534
@test try; a,b,c = 1,2; catch ex; (ex::BoundsError).a === (1,2) && ex.i == 3; end
@test try; [][]; catch ex; isempty((ex::BoundsError).a::Array{Any,1}) && ex.i == (1,); end
# @test try; [][]; catch ex; isempty((ex::BoundsError).a::Array{Any,1}) && ex.i == (1,); end # TODO: Re-enable after PLI
@test try; [][1,2]; catch ex; isempty((ex::BoundsError).a::Array{Any,1}) && ex.i == (1,2); end
@test try; [][10]; catch ex; isempty((ex::BoundsError).a::Array{Any,1}) && ex.i == (10,); end
f9534a() = (a=1+2im; getfield(a, -100))
Expand Down