Skip to content

Commit

Permalink
Fully deprecate partial linear indexing. Fixes #14770.
Browse files Browse the repository at this point in the history
  • Loading branch information
timholy committed Feb 13, 2017
1 parent 2bb2727 commit a99f379
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 94 deletions.
64 changes: 34 additions & 30 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ unsafe_indices(r::Range) = (OneTo(unsafe_length(r)),) # Ranges use checked_sub f
"""
linearindices(A)
Returns a `UnitRange` specifying the valid range of indices for `A[i]`
Returns a `AbstractUnitRange` specifying the valid range of indices for `A[i]`
where `i` is an `Int`. For arrays with conventional indexing (indices
start at 1), or any multidimensional array, this is `1:length(A)`;
however, for one-dimensional arrays with unconventional indices, this
Expand All @@ -89,6 +89,9 @@ julia> extrema(b)
"""
linearindices(A) = (@_inline_meta; OneTo(_length(A)))
linearindices(A::AbstractVector) = (@_inline_meta; indices1(A))
linearindices(inds::Tuple{Vararg{AbstractUnitRange}}) =
(@_inline_meta; OneTo(prod(map(unsafe_length, inds))))

eltype(::Type{<:AbstractArray{E}}) where {E} = E
elsize{T}(::AbstractArray{T}) = sizeof(T)

Expand Down Expand Up @@ -363,21 +366,7 @@ function checkbounds_indices(::Type{Bool}, IA::Tuple{Any}, I::Tuple{Any})
end
function checkbounds_indices(::Type{Bool}, IA::Tuple, I::Tuple{Any})
@_inline_meta
checkbounds_linear_indices(Bool, IA, I[1])
end
function checkbounds_linear_indices(::Type{Bool}, IA::Tuple, i)
@_inline_meta
if checkindex(Bool, IA[1], i)
return true
elseif checkindex(Bool, OneTo(trailingsize(IA)), i) # partial linear indexing
partial_linear_indexing_warning_lookup(length(IA))
return true # TODO: Return false after the above function is removed in deprecated.jl
end
return false
end
function checkbounds_linear_indices(::Type{Bool}, IA::Tuple, i::Union{Slice,Colon})
partial_linear_indexing_warning_lookup(length(IA))
true
checkbounds_indices(Bool, (linearindices(IA),), I)
end
checkbounds_indices(::Type{Bool}, ::Tuple, ::Tuple{}) = true

Expand Down Expand Up @@ -824,18 +813,32 @@ _getindex(::LinearIndexing, A::AbstractArray, I...) = error("indexing $(typeof(A

## LinearFast Scalar indexing: canonical method is one Int
_getindex(::LinearFast, A::AbstractArray, i::Int) = (@_propagate_inbounds_meta; getindex(A, i))
_getindex(::LinearFast, A::AbstractArray) = (@_propagate_inbounds_meta; getindex(A, _to_linear_index(A)))
_getindex(::LinearFast, A::AbstractArray) = (@_propagate_inbounds_meta; getindex(A, 1))
function _getindex(::LinearFast, A::AbstractArray, I::Int...)
@_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::AbstractVector) = (@_inline_meta; first(indices1(A)))
_to_linear_index(A::AbstractArray) = 1
_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{T,N}(A::AbstractArray{T,N}, I::Vararg{Int,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
function _to_linear_index{T,N}(A::AbstractArray{T,N}, I::Int...)
@_inline_meta
J, Jrem = IteratorsMD.split(I, Val{N})
_to_linear_index(A, J, Jrem)
end
# It's safe to drop Jrem, because we've already bounds-checked
_to_linear_index(A::AbstractVector, J::Tuple, Jrem::Tuple) = (@_inline_meta; J[1])
_to_linear_index(A::AbstractArray, J::Tuple, Jrem::Tuple) = (@_inline_meta; sub2ind(A, J...))
# TODO: uncomment when deprecation period for partial linear indexing has ended
# function _to_linear_index(A::AbstractVector, J::Tuple, Jrem::Tuple{})
# throw(ArgumentError("partial linear indexing is not allowed. Use `reshape(A, Val{$(length(J))})` to make the dimensionality of the array match the number of indices"))
# end
# function _to_linear_index(A::AbstractArray, J::Tuple, Jrem::Tuple{})
# throw(ArgumentError("partial linear indexing is not allowed. Use `reshape(A, Val{$(length(J))})` to make the dimensionality of the array match the number of indices"))
# end

## LinearSlow Scalar indexing: Canonical method is full dimensionality of Ints
_getindex(::LinearSlow, A::AbstractArray) = (@_propagate_inbounds_meta; getindex(A, _to_subscript_indices(A)...))
Expand All @@ -847,16 +850,18 @@ function _getindex(::LinearSlow, A::AbstractArray, I::Int...)
end
_getindex{T,N}(::LinearSlow, A::AbstractArray{T,N}, I::Vararg{Int, N}) = (@_propagate_inbounds_meta; getindex(A, I...))
_to_subscript_indices(A::AbstractArray, i::Int) = (@_inline_meta; _unsafe_ind2sub(A, i))
_to_subscript_indices{T,N}(A::AbstractArray{T,N}) = (@_inline_meta; fill_to_length((), 1, Val{N})) # TODO: DEPRECATE FOR #14770
_to_subscript_indices{T}(A::AbstractArray{T,0}) = () # TODO: REMOVE FOR #14770
_to_subscript_indices{T}(A::AbstractArray{T,0}, i::Int) = () # TODO: REMOVE FOR #14770
_to_subscript_indices{T}(A::AbstractArray{T,0}, I::Int...) = () # TODO: DEPRECATE FOR #14770
function _to_subscript_indices{T,N}(A::AbstractArray{T,N}, I::Int...) # TODO: DEPRECATE FOR #14770
_to_subscript_indices{T,N}(A::AbstractArray{T,N}) = (@_inline_meta; map(first, indices(A)))
_to_subscript_indices{T}(A::AbstractArray{T,0}, I::Int...) = ()
function _to_subscript_indices{T,N}(A::AbstractArray{T,N}, I::Int...)
@_inline_meta
J, _ = IteratorsMD.split(I, Val{N}) # (maybe) drop any trailing indices
sz = _remaining_size(J, size(A)) # compute trailing size (overlapping the final index)
(front(J)..., _unsafe_ind2sub(sz, last(J))...) # (maybe) extend the last index
end
J, Jrem = IteratorsMD.split(I, Val{N})
_to_subscript_indices(A, J, Jrem)
end
_to_subscript_indices(A, J::Tuple, Jrem::Tuple) = J # already bounds-checked, safe to drop Jrem
# TODO: uncomment when deprecation period for partial linear indexing has ended
# function _to_subscript_indices(A::AbstractArray, J::Tuple, Jrem::Tuple{})
# throw(ArgumentError("partial linear indexing is not allowed. Use `reshape(A, Val{$(length(J))})` to make the dimensionality of the array match the number of indices"))
# end
_to_subscript_indices{T,N}(A::AbstractArray{T,N}, I::Vararg{Int,N}) = I
_remaining_size(::Tuple{Any}, t::Tuple) = t
_remaining_size(h::Tuple, t::Tuple) = (@_inline_meta; _remaining_size(tail(h), tail(t)))
Expand Down Expand Up @@ -926,7 +931,6 @@ end

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

# TODO: DEPRECATE FOR #14770 (just the partial linear indexing part)
function get!{T}(X::AbstractArray{T}, A::AbstractArray, I::RangeVecIntList, default::T)
fill!(X, default)
dst, src = indcopy(size(A), I)
Expand Down
7 changes: 5 additions & 2 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -471,8 +471,9 @@ done(a::Array,i) = (@_inline_meta; i == length(a)+1)
## Indexing: getindex ##

# This is more complicated than it needs to be in order to get Win64 through bootstrap
getindex{T}(A::Array{T,0}) = arrayref(A, 1)
getindex(A::Array, i1::Int) = arrayref(A, i1)
getindex(A::Array, i1::Int, i2::Int, I::Int...) = (@_inline_meta; arrayref(A, i1, i2, I...)) # TODO: REMOVE FOR #14770
getindex{T,N}(A::Array{T,N}, I::Vararg{Int,N}) = (@_inline_meta; arrayref(A, I...))

# Faster contiguous indexing using copy! for UnitRange and Colon
function getindex(A::Array, I::UnitRange{Int})
Expand Down Expand Up @@ -500,8 +501,9 @@ function getindex{S}(A::Array{S}, I::Range{Int})
end

## Indexing: setindex! ##
setindex!{T}(A::Array{T,0}, x) = arrayset(A, convert(T,x)::T, 1)
setindex!{T}(A::Array{T}, x, i1::Int) = arrayset(A, convert(T,x)::T, i1)
setindex!{T}(A::Array{T}, x, i1::Int, i2::Int, I::Int...) = (@_inline_meta; arrayset(A, convert(T,x)::T, i1, i2, I...)) # TODO: REMOVE FOR #14770
setindex!{T,N}(A::Array{T,N}, x, I::Vararg{Int,N}) = (@_inline_meta; arrayset(A, convert(T,x)::T, I...))

# These are redundant with the abstract fallbacks but needed for bootstrap
function setindex!(A::Array, x, I::AbstractVector{Int})
Expand Down Expand Up @@ -548,6 +550,7 @@ function setindex!{T}(A::Array{T}, X::Array{T}, c::Colon)
end

setindex!(A::Array, x::Number, ::Colon) = fill!(A, x)
setindex!{T}(A::Array{T,0}, x::Number) = fill!(A, x) # ambiguity resolution
setindex!{T, N}(A::Array{T, N}, x::Number, ::Vararg{Colon, N}) = fill!(A, x)

# efficiently grow an array
Expand Down
50 changes: 14 additions & 36 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1032,43 +1032,19 @@ isempty(::Task) = error("isempty not defined for Tasks")
end

# Deprecate partial linear indexing
function partial_linear_indexing_warning_lookup(nidxs_remaining)
# We need to figure out how many indices were passed for a sensible deprecation warning
opts = JLOptions()
if opts.depwarn > 0
# Find the caller -- this is very expensive so we don't want to do it twice
bt = backtrace()
found = false
call = StackTraces.UNKNOWN
caller = StackTraces.UNKNOWN
for frame in bt
lkups = StackTraces.lookup(frame)
for caller in lkups
if caller === StackTraces.UNKNOWN
continue
end
found && @goto found
if caller.func in (:getindex, :setindex!, :view)
found = true
call = caller
end
end
end
@label found
fn = "`reshape`"
if call != StackTraces.UNKNOWN && !isnull(call.linfo)
# Try to grab the number of dimensions in the parent array
mi = get(call.linfo)
args = mi.specTypes.parameters
if length(args) >= 2 && args[2] <: AbstractArray
fn = "`reshape(A, Val{$(ndims(args[2]) - nidxs_remaining + 1)})`"
end
end
_depwarn("Partial linear indexing is deprecated. Use $fn to make the dimensionality of the array match the number of indices.", opts, bt, caller)
end
check_oneto(A::AbstractArray) = check_oneto(indices(A))
check_oneto(inds::Tuple{Vararg{OneTo}}) = nothing
check_oneto(inds::Tuple{AbstractUnitRange,Vararg{AbstractUnitRange}}) = error("partial linear indexing is not supported for arrays with non-1 indexing")
function _to_linear_index(A::AbstractArray, J::Tuple, Jrem::Tuple{})
check_oneto(A)
depwarn("partial linear indexing is deprecated. Use `reshape(A, Val{$(length(J))})` to make the dimensionality of the array match the number of indices", :_to_linear_index)
sub2ind(A, J...)
end
function partial_linear_indexing_warning(n)
depwarn("Partial linear indexing is deprecated. Use `reshape(A, Val{$n})` to make the dimensionality of the array match the number of indices.", (:getindex, :setindex!, :view))
function _to_subscript_indices(A::AbstractArray, J::Tuple, Jrem::Tuple{})
check_oneto(A)
depwarn("partial linear indexing is deprecated. Use `reshape(A, Val{$(length(J))})` to make the dimensionality of the array match the number of indices", :_to_subscript_indices)
sz = _remaining_size(J, indices(A)) # compute trailing size (overlapping the final index)
(front(J)..., _unsafe_ind2sub(sz, last(J))...)
end

# Deprecate Array(T, dims...) in favor of proper type constructors
Expand Down Expand Up @@ -1262,6 +1238,8 @@ for f in (:airyai, :airyaiprime, :airybi, :airybiprime, :airyaix, :airyaiprimex,
end

# END 0.6 deprecations
# IMPORTANT: when the 0.6 deprecations are removed, uncomment error messages for
# _to_linear_index and _to_subscript_indices in abstractarray.jl

# BEGIN 1.0 deprecations
# END 1.0 deprecations
2 changes: 2 additions & 0 deletions base/multidimensional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,10 @@ end
# Indexing into Array with mixtures of Integers and CartesianIndices is
# extremely performance-sensitive. While the abstract fallbacks support this,
# codegen has extra support for SIMDification that sub2ind doesn't (yet) support
@propagate_inbounds getindex(A::Array, I::Integer...) = _getindex(LinearFast(), A, to_indices(A, I)...)
@propagate_inbounds getindex(A::Array, i1::Union{Integer, CartesianIndex}, I::Union{Integer, CartesianIndex}...) =
A[to_indices(A, (i1, I...))...]
@propagate_inbounds setindex!(A::Array, v, I::Integer...) = _setindex!(LinearFast(), A, v, to_indices(A, I)...)
@propagate_inbounds setindex!(A::Array, v, i1::Union{Integer, CartesianIndex}, I::Union{Integer, CartesianIndex}...) =
(A[to_indices(A, (i1, I...))...] = v; A)

Expand Down
83 changes: 64 additions & 19 deletions test/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ Base.setindex!{T}(A::TSlow{T,5}, v, i1::Int, i2::Int, i3::Int, i4::Int, i5::Int)
(A.data[(i1,i2,i3,i4,i5)] = v)

const can_inline = Base.JLOptions().can_inline != 0
const depwarns_not_error = Base.JLOptions().depwarn < 2
function test_scalar_indexing{T}(::Type{T}, shape, ::Type{TestAbstractArray})
N = prod(shape)
A = reshape(collect(1:N), shape)
Expand All @@ -263,27 +264,44 @@ function test_scalar_indexing{T}(::Type{T}, shape, ::Type{TestAbstractArray})
end
end
# Test linear indexing and partial linear indexing
@test A == B
i=0
for i1 = 1:length(B)
i += 1
@test A[i1] == B[i1] == i
end
i=0
for i2 = 1:size(B, 2)
for i1 = 1:size(B, 1)
i += 1
@test A[i1,i2] == B[i1,i2] == i
# TODO (#14770): run only for ndims(A) <= 2, elim. redirect_stderr stuff
if ndims(A) <= 2 || depwarns_not_error
filename = tempname()
open(filename, "w") do f
redirect_stderr(f) do
i=0
for i2 = 1:size(B, 2)
for i1 = 1:size(B, 1)
i += 1
@test A[i1,i2] == B[i1,i2] == i
end
end
end
end
rm(filename)
end
@test A == B
i=0
for i3 = 1:size(B, 3)
for i2 = 1:size(B, 2)
for i1 = 1:size(B, 1)
i += 1
@test A[i1,i2,i3] == B[i1,i2,i3] == i
if ndims(A) <= 3 || depwarns_not_error
filename = tempname()
open(filename, "w") do f
redirect_stderr(f) do
i=0
for i3 = 1:size(B, 3)
for i2 = 1:size(B, 2)
for i1 = 1:size(B, 1)
i += 1
@test A[i1,i2,i3] == B[i1,i2,i3] == i
end
end
end
end
end
rm(filename)
end
# Test zero-dimensional accesses
@test A[] == B[] == A[1] == B[1] == 1
Expand Down Expand Up @@ -369,22 +387,49 @@ function test_vector_indexing{T}(::Type{T}, shape, ::Type{TestAbstractArray})
# Test adding dimensions with matrices
idx1 = rand(1:size(A, 1), 3)
idx2 = rand(1:size(A, 2), 4, 5)
@test B[idx1, idx2] == A[idx1, idx2] == reshape(A[idx1, vec(idx2)], 3, 4, 5) == reshape(B[idx1, vec(idx2)], 3, 4, 5)
@test B[1, idx2] == A[1, idx2] == reshape(A[1, vec(idx2)], 4, 5) == reshape(B[1, vec(idx2)], 4, 5)
# #14770
if ndims(A) <= 2 || depwarns_not_error
filename = tempname()
open(filename, "w") do f
redirect_stderr(f) do
@test B[idx1, idx2] == A[idx1, idx2] == reshape(A[idx1, vec(idx2)], 3, 4, 5) == reshape(B[idx1, vec(idx2)], 3, 4, 5)
@test B[1, idx2] == A[1, idx2] == reshape(A[1, vec(idx2)], 4, 5) == reshape(B[1, vec(idx2)], 4, 5)
end
end
rm(filename)
end

# test removing dimensions with 0-d arrays
idx0 = reshape([rand(1:size(A, 1))])
@test B[idx0, idx2] == A[idx0, idx2] == reshape(A[idx0[], vec(idx2)], 4, 5) == reshape(B[idx0[], vec(idx2)], 4, 5)
# @test B[reshape([end]), reshape([end])] == A[reshape([end]), reshape([end])] == reshape([A[end,end]]) == reshape([B[end,end]]) # TODO: Re-enable after partial linear indexing deprecation
# TODO (#14770): run only for ndims(A) <= 2, elim. redirect_stderr stuff
if ndims(A) <= 2 || depwarns_not_error
filename = tempname()
open(filename, "w") do f
redirect_stderr(f) do
@test B[idx0, idx2] == A[idx0, idx2] == reshape(A[idx0[], vec(idx2)], 4, 5) == reshape(B[idx0[], vec(idx2)], 4, 5)
# @test B[reshape([end]), reshape([end])] == A[reshape([end]), reshape([end])] == reshape([A[end,end]]) == reshape([B[end,end]]) # TODO: Re-enable after partial linear indexing deprecation
end
end
rm(filename)
end

# test logical indexing
mask = bitrand(shape)
@test B[mask] == A[mask] == B[find(mask)] == A[find(mask)] == find(mask)
@test B[vec(mask)] == A[vec(mask)] == find(mask)
mask1 = bitrand(size(A, 1))
mask2 = bitrand(size(A, 2))
@test B[mask1, mask2] == A[mask1, mask2] == B[find(mask1), find(mask2)]
@test B[mask1, 1] == A[mask1, 1] == find(mask1)
mask2 = bitrand(prod(Base.tail(size(A))))
# TODO (#14770): run only for ndims(A) <= 2, elim. redirect_stderr stuff
if ndims(A) <= 2 || depwarns_not_error
filename = tempname()
open(filename, "w") do f
redirect_stderr(f) do
@test B[mask1, mask2] == A[mask1, mask2] == B[find(mask1), find(mask2)]
@test B[mask1, 1] == A[mask1, 1] == find(mask1)
end
end
rm(filename)
end
end

function test_primitives{T}(::Type{T}, shape, ::Type{TestAbstractArray})
Expand Down
25 changes: 18 additions & 7 deletions test/subarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,26 @@ function test_linear(A::ANY, B::ANY)
end

# "mixed" means 2 indexes even for N-dimensional arrays
# TODO (#14770): eliminate this test when deprecations removed
const depwarns_not_error = Base.JLOptions().depwarn < 2
test_mixed{T}(::AbstractArray{T,1}, ::Array) = nothing
test_mixed{T}(::AbstractArray{T,2}, ::Array) = nothing
test_mixed(A, B::Array) = _test_mixed(A, reshape(B, size(A)))
function _test_mixed(A::ANY, B::ANY)
m = size(A, 1)
n = size(A, 2)
isgood = true
for j = 1:n, i = 1:m
if A[i,j] != B[i,j]
isgood = false
break
if depwarns_not_error
filename = tempname()
open(filename, "w") do f
redirect_stderr(f) do
for j = 1:n, i = 1:m
if A[i,j] != B[i,j]
isgood = false
break
end
end
end
end
end
if !isgood
Expand Down Expand Up @@ -229,9 +238,11 @@ function runviews(SB::AbstractArray, indexN, indexNN, indexNNN)
ndims(SB) > 3 && i3 isa Colon && continue # TODO: Re-enable once Colon no longer spans partial trailing dimensions
runsubarraytests(SB, i1, i2, i3)
end
for i2 in indexN, i1 in indexN
i2 isa Colon && continue # TODO: Re-enable once Colon no longer spans partial trailing dimensions
runsubarraytests(SB, i1, i2)
if depwarns_not_error
for i2 in indexN, i1 in indexN
i2 isa Colon && continue # TODO: Re-enable once Colon no longer spans partial trailing dimensions
runsubarraytests(SB, i1, i2)
end
end
for i1 in indexNNN
runsubarraytests(SB, i1)
Expand Down

0 comments on commit a99f379

Please sign in to comment.