Skip to content

Commit

Permalink
Completely remove partial linear indexing
Browse files Browse the repository at this point in the history 
This removes the partial linear indexing deprecation and implents the new behavior: Linear indexing now only takes effect when there is exactly one non-cartesian index.
  • Loading branch information
@mbauman
mbauman committed Aug 24, 2017
1 parent 2767355 commit 8414150
Show file tree
Hide file tree
Showing 11 changed files with 33 additions and 199 deletions.
49 changes: 3 additions & 46 deletions base/abstractarray.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,47 +418,12 @@ function checkbounds_indices(::Type{Bool}, IA::Tuple, I::Tuple)
@_inline_meta
checkindex(Bool, IA[1], I[1]) & checkbounds_indices(Bool, tail(IA), tail(I))
end
checkbounds_indices(::Type{Bool}, ::Tuple{}, ::Tuple{}) = true
function checkbounds_indices(::Type{Bool}, ::Tuple{}, I::Tuple{Any})
@_inline_meta
checkindex(Bool, 1:1, I[1])
end
function checkbounds_indices(::Type{Bool}, ::Tuple{}, I::Tuple)
@_inline_meta
checkindex(Bool, 1:1, I[1]) & checkbounds_indices(Bool, (), tail(I))
end
function checkbounds_indices(::Type{Bool}, IA::Tuple{Any}, I::Tuple{Any})
@_inline_meta
checkindex(Bool, IA[1], I[1])
checkindex(Bool, OneTo(1), I[1]) & checkbounds_indices(Bool, (), tail(I))
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{Vararg{OneTo}}, 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{AbstractUnitRange,Vararg{AbstractUnitRange}}, i)
@_inline_meta
checkindex(Bool, IA[1], i)
end
function checkbounds_linear_indices(::Type{Bool}, IA::Tuple{Vararg{OneTo}}, i::Union{Slice,Colon})
partial_linear_indexing_warning_lookup(length(IA))
true
end
function checkbounds_linear_indices(::Type{Bool},
IA::Tuple{AbstractUnitRange,Vararg{AbstractUnitRange}}, i::Union{Slice,Colon})
partial_linear_indexing_warning_lookup(length(IA))
true
end
checkbounds_indices(::Type{Bool}, ::Tuple, ::Tuple{}) = true
checkbounds_indices(::Type{Bool}, ::Tuple, ::Tuple{}) = true
checkbounds_indices(::Type{Bool}, ::Tuple{}, ::Tuple{}) = true

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

Expand Down Expand Up @@ -1005,14 +970,6 @@ function _to_subscript_indices(A::AbstractArray{T,N}, I::Int...) where {T,N} # T
end
_to_subscript_indices(A::AbstractArray, J::Tuple, Jrem::Tuple{}) =
__to_subscript_indices(A, indices(A), J, Jrem)
# We allow partial linear indexing deprecation for OneTo arrays
function __to_subscript_indices(A::AbstractArray, ::Tuple{Vararg{OneTo}}, J::Tuple, Jrem::Tuple{})
@_inline_meta
sz = _remaining_size(J, indices(A)) # compute trailing size (overlapping the final index)
(front(J)..., _unsafe_ind2sub(sz, last(J))...) # (maybe) extend the last index
end
# After the partial linear indexing deprecation is removed, this next method can
# become the new normal. For now, it's limited to non-OneTo arrays.
function __to_subscript_indices(A::AbstractArray,
::Tuple{AbstractUnitRange,Vararg{AbstractUnitRange}}, J::Tuple, Jrem::Tuple{})
@_inline_meta
Expand Down
40 changes: 0 additions & 40 deletions base/deprecated.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1055,46 +1055,6 @@ isempty(::Task) = error("isempty not defined for Tasks")
export @test_approx_eq
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 outer 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
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))
end

# Deprecate Array(T, dims...) in favor of proper type constructors
@deprecate Array(::Type{T}, d::NTuple{N,Int}) where {T,N} Array{T}(d)
@deprecate Array(::Type{T}, d::Int...) where {T} Array{T}(d...)
Expand Down
1 change: 1 addition & 0 deletions base/indices.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ given tuple of indices and the dimensional indices of `A` in tandem. As such,
not all index types are guaranteed to propagate to `Base.to_index`.
"""
to_indices(A, I::Tuple) = (@_inline_meta; to_indices(A, indices(A), I))
to_indices(A, I::Tuple{Any}) = (@_inline_meta; to_indices(A, (linearindices(A),), I))
to_indices(A, inds, ::Tuple{}) = ()
to_indices(A, inds, I::Tuple{Any, Vararg{Any}}) =
(@_inline_meta; (to_index(A, I[1]), to_indices(A, _maybetail(inds), tail(I))...))
Expand Down
16 changes: 0 additions & 16 deletions base/multidimensional.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,15 +442,6 @@ done(L::LogicalIndex, s) = s[3] > length(L)
end
@inline done(L::LogicalIndex{Int,<:BitArray}, s) = s[2] > length(L)

# Checking bounds with LogicalIndex{Int} is tricky since we allow linear indexing over trailing dimensions
@inline checkbounds_indices(::Type{Bool},IA::Tuple{},I::Tuple{LogicalIndex{Int,AbstractArray{Bool,N}}}) where {N} =
checkindex(Bool, IA, I[1])
@inline checkbounds_indices(::Type{Bool},IA::Tuple{Any},I::Tuple{LogicalIndex{Int,AbstractArray{Bool,N}}}) where {N} =
checkindex(Bool, IA[1], I[1])
@inline function checkbounds_indices(::Type{Bool}, IA::Tuple, I::Tuple{LogicalIndex{Int,AbstractArray{Bool,N}}}) where N
IA1, IArest = IteratorsMD.split(IA, Val(N))
checkindex(Bool, IA1, I[1])
end
@inline checkbounds(::Type{Bool}, A::AbstractArray, I::LogicalIndex{<:Any,<:AbstractArray{Bool,1}}) =
linearindices(A) == linearindices(I.mask)
@inline checkbounds(::Type{Bool}, A::AbstractArray, I::LogicalIndex) = indices(A) == indices(I.mask)
Expand Down Expand Up @@ -490,15 +481,8 @@ _maybe_linear_logical_index(::IndexLinear, A, i) = LogicalIndex{Int}(i)
(uncolon(inds, I), to_indices(A, _maybetail(inds), tail(I))...)

const CI0 = Union{CartesianIndex{0}, AbstractArray{CartesianIndex{0}}}
uncolon(inds::Tuple{}, I::Tuple{Colon}) = Slice(OneTo(1))
uncolon(inds::Tuple{}, I::Tuple{Colon, Vararg{Any}}) = Slice(OneTo(1))
uncolon(inds::Tuple{}, I::Tuple{Colon, Vararg{CI0}}) = Slice(OneTo(1))
uncolon(inds::Tuple{Any}, I::Tuple{Colon}) = Slice(inds[1])
uncolon(inds::Tuple{Any}, I::Tuple{Colon, Vararg{Any}}) = Slice(inds[1])
uncolon(inds::Tuple{Any}, I::Tuple{Colon, Vararg{CI0}}) = Slice(inds[1])
uncolon(inds::Tuple, I::Tuple{Colon, Vararg{Any}}) = Slice(inds[1])
uncolon(inds::Tuple, I::Tuple{Colon}) = Slice(OneTo(trailingsize(inds)))
uncolon(inds::Tuple, I::Tuple{Colon, Vararg{CI0}}) = Slice(OneTo(trailingsize(inds)))

### From abstractarray.jl: Internal multidimensional indexing definitions ###
getindex(x::Number, i::CartesianIndex{0}) = x
Expand Down
6 changes: 2 additions & 4 deletions base/subarray.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ end
replace_ref_end!(ex)
Recursively replace occurrences of the symbol :end in a "ref" expression (i.e. A[...]) `ex`
with the appropriate function calls (`endof`, `size` or `trailingsize`). Replacement uses
with the appropriate function calls (`endof` or `size`). Replacement uses
the closest enclosing ref, so
A[B[end]]
Expand Down Expand Up @@ -402,7 +402,7 @@ function replace_ref_end_!(ex, withex)
else
n = 1
J = endof(ex.args)
for j = 2:J-1
for j = 2:J
exj, used = replace_ref_end_!(ex.args[j],:($size($S,$n)))
used_S |= used
ex.args[j] = exj
Expand All @@ -418,8 +418,6 @@ function replace_ref_end_!(ex, withex)
n += 1
end
end
ex.args[J], used = replace_ref_end_!(ex.args[J],:($trailingsize($S,$n)))
used_S |= used
end
if used_S && S !== ex.args[1]
S0 = ex.args[1]
Expand Down
31 changes: 4 additions & 27 deletions src/cgutils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1672,33 +1672,10 @@ static Value *emit_array_nd_index(jl_codectx_t &ctx,
// We have already emitted a bounds check for each index except for
// the last one which we therefore have to do here.
bool linear_indexing = nd == -1 || nidxs < (size_t)nd;
if (linear_indexing) {
// Compare the linearized index `i` against the linearized size of
// the accessed array, i.e. `if !(i < alen) goto error`.
if (nidxs > 1) {
// TODO: REMOVE DEPWARN AND RETURN FALSE AFTER 0.6.
// We need to check if this is inside the non-linearized size
BasicBlock *partidx = BasicBlock::Create(jl_LLVMContext, "partlinidx");
BasicBlock *partidxwarn = BasicBlock::Create(jl_LLVMContext, "partlinidxwarn");
Value *d = emit_arraysize_for_unsafe_dim(ctx, ainfo, ex, nidxs, nd);
ctx.builder.CreateCondBr(ctx.builder.CreateICmpULT(ii, d), endBB, partidx);

// We failed the normal bounds check; check to see if we're
// inside the linearized size (partial linear indexing):
ctx.f->getBasicBlockList().push_back(partidx);
ctx.builder.SetInsertPoint(partidx);
Value *alen = emit_arraylen(ctx, ainfo, ex);
ctx.builder.CreateCondBr(ctx.builder.CreateICmpULT(i, alen), partidxwarn, failBB);

// We passed the linearized bounds check; now throw the depwarn:
ctx.f->getBasicBlockList().push_back(partidxwarn);
ctx.builder.SetInsertPoint(partidxwarn);
ctx.builder.CreateCall(prepare_call(jldepwarnpi_func), ConstantInt::get(T_size, nidxs));
ctx.builder.CreateBr(endBB);
} else {
Value *alen = emit_arraylen(ctx, ainfo, ex);
ctx.builder.CreateCondBr(ctx.builder.CreateICmpULT(i, alen), endBB, failBB);
}
if (linear_indexing && nidxs == 1) {
// Check against the entire linear span of the array
Value *alen = emit_arraylen(ainfo, ex, ctx);
builder.CreateCondBr(builder.CreateICmpULT(i, alen), endBB, failBB);
} else {
// Compare the last index of the access against the last dimension of
// the accessed array, i.e. `if !(last_index < last_dimension) goto error`.
Expand Down
8 changes: 0 additions & 8 deletions src/codegen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,6 @@ static Function *expect_func;
static Function *jldlsym_func;
static Function *jlnewbits_func;
static Function *jltypeassert_func;
static Function *jldepwarnpi_func;
//static Function *jlgetnthfield_func;
static Function *jlgetnthfieldchecked_func;
//static Function *jlsetnthfield_func;
Expand Down Expand Up @@ -6284,13 +6283,6 @@ static void init_julia_llvm_env(Module *m)

jlapply2va_func = jlcall_func_to_llvm("jl_apply_2va", &jl_apply_2va, m);

std::vector<Type*> argsdepwarnpi(0);
argsdepwarnpi.push_back(T_size);
jldepwarnpi_func = Function::Create(FunctionType::get(T_void, argsdepwarnpi, false),
Function::ExternalLinkage,
"jl_depwarn_partial_indexing", m);
add_named_global(jldepwarnpi_func, &jl_depwarn_partial_indexing);

std::vector<Type *> args_1ptr(0);
args_1ptr.push_back(T_prjlvalue);
queuerootfun = Function::Create(FunctionType::get(T_void, args_1ptr, false),
Expand Down
12 changes: 4 additions & 8 deletions src/julia-syntax.scm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,16 @@
;; the array `a` in the `n`th index.
;; `tuples` are a list of the splatted arguments that precede index `n`
;; `last` = is this last index?
;; returns a call to endof(a), trailingsize(a,n), or size(a,n)
;; returns a call to endof(a) or size(a,n)
(define (end-val a n tuples last)
(if (null? tuples)
(if last
(if (= n 1)
`(call (top endof) ,a)
`(call (top trailingsize) ,a ,n))
(if (and last (= n 1))
`(call (top endof) ,a)
`(call (top size) ,a ,n))
(let ((dimno `(call (top +) ,(- n (length tuples))
,.(map (lambda (t) `(call (top length) ,t))
tuples))))
(if last
`(call (top trailingsize) ,a ,dimno)
`(call (top size) ,a ,dimno)))))
`(call (top size) ,a ,dimno))))

;; replace `end` for the closest ref expression, so doesn't go inside nested refs
(define (replace-end ex a n tuples last)
Expand Down
20 changes: 0 additions & 20 deletions src/rtutils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1016,26 +1016,6 @@ void jl_depwarn(const char *msg, jl_value_t *sym)
JL_GC_POP();
}

JL_DLLEXPORT void jl_depwarn_partial_indexing(size_t n)
{
static jl_value_t *depwarn_func = NULL;
if (!depwarn_func && jl_base_module) {
depwarn_func = jl_get_global(jl_base_module, jl_symbol("partial_linear_indexing_warning"));
}
if (!depwarn_func) {
jl_safe_printf("WARNING: Partial linear indexing is deprecated. Use "
"`reshape(A, Val(%zd))` to make the dimensionality of the array match "
"the number of indices\n", n);
return;
}
jl_value_t **depwarn_args;
JL_GC_PUSHARGS(depwarn_args, 2);
depwarn_args[0] = depwarn_func;
depwarn_args[1] = jl_box_long(n);
jl_apply(depwarn_args, 2);
JL_GC_POP();
}

#ifdef __cplusplus
}
#endif
35 changes: 15 additions & 20 deletions test/abstractarray.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ A = rand(5,4,3)
@test checkbounds(Bool, A, 2, 2, 2, 1) == true # extra indices
@test checkbounds(Bool, A, 2, 2, 2, 2) == false
@test checkbounds(Bool, A, 1, 1) == true # partial linear indexing (PLI)
# @test checkbounds(Bool, A, 1, 12) == false # PLI TODO: Re-enable after partial linear indexing deprecation
# @test checkbounds(Bool, A, 5, 12) == false # PLI TODO: Re-enable after partial linear indexing deprecation
@test checkbounds(Bool, A, 1, 13) == false # PLI
# @test checkbounds(Bool, A, 6, 12) == false # PLI TODO: Re-enable after partial linear indexing deprecation
@test checkbounds(Bool, A, 1, 12) == false
@test checkbounds(Bool, A, 5, 12) == false
@test checkbounds(Bool, A, 1, 13) == false
@test checkbounds(Bool, A, 6, 12) == false
end

@testset "single CartesianIndex" begin
Expand All @@ -32,15 +32,15 @@ end
@test checkbounds(Bool, A, CartesianIndex((5, 5, 3))) == false
@test checkbounds(Bool, A, CartesianIndex((5, 4, 4))) == false
@test checkbounds(Bool, A, CartesianIndex((1,))) == true
# @test checkbounds(Bool, A, CartesianIndex((60,))) == false # TODO: Re-enable after partial linear indexing deprecation
@test checkbounds(Bool, A, CartesianIndex((60,))) == false
@test checkbounds(Bool, A, CartesianIndex((61,))) == false
@test checkbounds(Bool, A, CartesianIndex((2, 2, 2, 1,))) == true
@test checkbounds(Bool, A, CartesianIndex((2, 2, 2, 2,))) == false
@test checkbounds(Bool, A, CartesianIndex((1, 1,))) == true
# @test checkbounds(Bool, A, CartesianIndex((1, 12,))) == false # TODO: Re-enable after partial linear indexing deprecation
# @test checkbounds(Bool, A, CartesianIndex((5, 12,))) == false # TODO: Re-enable after partial linear indexing deprecation
@test checkbounds(Bool, A, CartesianIndex((1, 12,))) == false
@test checkbounds(Bool, A, CartesianIndex((5, 12,))) == false
@test checkbounds(Bool, A, CartesianIndex((1, 13,))) == false
# @test checkbounds(Bool, A, CartesianIndex((6, 12,))) == false # TODO: Re-enable after partial linear indexing deprecation
@test checkbounds(Bool, A, CartesianIndex((6, 12,))) == false
end

@testset "mix of CartesianIndex and Int" begin
Expand All @@ -67,9 +67,9 @@ end
@test checkbounds(Bool, A, 2, 2, 2, 1:1) == true # extra indices
@test checkbounds(Bool, A, 2, 2, 2, 1:2) == false
@test checkbounds(Bool, A, 1:5, 1:4) == true
# @test checkbounds(Bool, A, 1:5, 1:12) == false # TODO: Re-enable after partial linear indexing deprecation
@test checkbounds(Bool, A, 1:5, 1:12) == false
@test checkbounds(Bool, A, 1:5, 1:13) == false
# @test checkbounds(Bool, A, 1:6, 1:12) == false # TODO: Re-enable after partial linear indexing deprecation
@test checkbounds(Bool, A, 1:6, 1:12) == false
end

@testset "logical" begin
Expand All @@ -81,9 +81,9 @@ end
@test checkbounds(Bool, A, trues(61)) == false
@test checkbounds(Bool, A, 2, 2, 2, trues(1)) == true # extra indices
@test checkbounds(Bool, A, 2, 2, 2, trues(2)) == false
# @test checkbounds(Bool, A, trues(5), trues(12)) == false # TODO: Re-enable after partial linear indexing deprecation
@test checkbounds(Bool, A, trues(5), trues(12)) == false
@test checkbounds(Bool, A, trues(5), trues(13)) == false
# @test checkbounds(Bool, A, trues(6), trues(12)) == false # TODO: Re-enable after partial linear indexing deprecation
@test checkbounds(Bool, A, trues(6), trues(12)) == false
@test checkbounds(Bool, A, trues(5, 4, 3)) == true
@test checkbounds(Bool, A, trues(5, 4, 2)) == false
@test checkbounds(Bool, A, trues(5, 12)) == false
Expand Down Expand Up @@ -145,11 +145,6 @@ end
@test sub2ind((0:3,3:5), i-1, j+2) == k
@test ind2sub((0:3,3:5), k) == (i-1, j+2)
end
@testset "Delete when partial linear indexing is deprecated (#14770)" begin
@test sub2ind((4,3), 7) == 7
@test sub2ind((1:4,1:3), 7) == 7
@test sub2ind((0:3,3:5), 7) == 8
end
end

@testset "3-dimensional" begin
Expand Down Expand Up @@ -377,8 +372,8 @@ function test_vector_indexing(::Type{T}, shape, ::Type{TestAbstractArray}) where
@test B[vec(idxs)] == A[vec(idxs)] == vec(idxs)
@test B[:] == A[:] == collect(1:N)
@test B[1:end] == A[1:end] == collect(1:N)
# @test B[:,:] == A[:,:] == reshape(1:N, shape[1], prod(shape[2:end])) # TODO: Re-enable after partial linear indexing deprecation
# @test B[1:end,1:end] == A[1:end,1:end] == reshape(1:N, shape[1], prod(shape[2:end])) # TODO: Re-enable after partial linear indexing deprecation
@test B[:,:] == A[:,:] == B[:,:,1] == A[:,:,1]
B[1:end,1:end] == A[1:end,1:end] == B[1:end,1:end,1] == A[1:end,1:end,1]

@testset "Test with containers that aren't Int[]" begin
@test B[[]] == A[[]] == []
Expand All @@ -395,7 +390,7 @@ function test_vector_indexing(::Type{T}, shape, ::Type{TestAbstractArray}) where
@testset "test removing dimensions with 0-d arrays" begin
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
@test B[reshape([end]), reshape([end])] == A[reshape([end]), reshape([end])] == reshape([A[end,end]]) == reshape([B[end,end]])
end

mask = bitrand(shape)
Expand Down
Loading

0 comments on commit 8414150

Please sign in to comment.