Skip to content

Commit

Permalink
Deprecate partial linear indexing
Browse files Browse the repository at this point in the history
  • Loading branch information
mbauman committed Jan 16, 2017
1 parent 4045aff commit 58c22b7
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 56 deletions.
46 changes: 32 additions & 14 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ See also [`checkindex`](@ref).
"""
function checkbounds(::Type{Bool}, A::AbstractArray, I...)
@_inline_meta
checkbounds_indices(Bool, indices(A), I)
checkbounds_indices(Bool, A, indices(A), I)
end
# As a special extension, allow using logical arrays that match the source array exactly
function checkbounds{_,N}(::Type{Bool}, A::AbstractArray{_,N}, I::AbstractArray{Bool,N})
Expand All @@ -325,15 +325,15 @@ end
checkbounds(A::AbstractArray) = checkbounds(A, 1) # 0-d case

"""
checkbounds_indices(Bool, IA, I)
checkbounds_indices(Bool, A, IA, I)
Return `true` if the "requested" indices in the tuple `I` fall within
the bounds of the "permitted" indices specified by the tuple
`IA`. This function recursively consumes elements of these tuples,
usually in a 1-for-1 fashion,
checkbounds_indices(Bool, (IA1, IA...), (I1, I...)) = checkindex(Bool, IA1, I1) &
checkbounds_indices(Bool, IA, I)
checkbounds_indices(Bool, A, (IA1, IA...), (I1, I...)) = checkindex(Bool, IA1, I1) &
checkbounds_indices(Bool, A, IA, I)
Note that [`checkindex`](@ref) is being used to perform the actual
bounds-check for a single dimension of the array.
Expand All @@ -342,25 +342,43 @@ There are two important exceptions to the 1-1 rule: linear indexing and
CartesianIndex{N}, both of which may "consume" more than one element
of `IA`.
"""
function checkbounds_indices(::Type{Bool}, IA::Tuple, I::Tuple)
function checkbounds_indices(::Type{Bool}, A::AbstractArray, IA::Tuple, I::Tuple)
@_inline_meta
checkindex(Bool, IA[1], I[1]) & checkbounds_indices(Bool, tail(IA), tail(I))
checkindex(Bool, IA[1], I[1]) & checkbounds_indices(Bool, A, tail(IA), tail(I))
end
checkbounds_indices(::Type{Bool}, ::Tuple{}, ::Tuple{}) = true
checkbounds_indices(::Type{Bool}, ::Tuple{}, I::Tuple{Any}) = (@_inline_meta; checkindex(Bool, 1:1, I[1]))
function checkbounds_indices(::Type{Bool}, ::Tuple{}, I::Tuple)
checkbounds_indices(::Type{Bool}, ::AbstractArray, ::Tuple{}, ::Tuple{}) = true
checkbounds_indices(::Type{Bool}, ::AbstractArray, ::Tuple{}, I::Tuple{Any}) = (@_inline_meta; checkindex(Bool, 1:1, I[1]))
function checkbounds_indices(::Type{Bool}, A::AbstractArray, ::Tuple{}, I::Tuple)
@_inline_meta
checkindex(Bool, 1:1, I[1]) & checkbounds_indices(Bool, (), tail(I))
checkindex(Bool, 1:1, I[1]) & checkbounds_indices(Bool, A, (), tail(I))
end
function checkbounds_indices(::Type{Bool}, IA::Tuple{Any}, I::Tuple{Any})
function checkbounds_indices(::Type{Bool}, A::AbstractArray, IA::Tuple{Any}, I::Tuple{Any})
@_inline_meta
checkindex(Bool, IA[1], I[1])
end
function checkbounds_indices(::Type{Bool}, IA::Tuple, I::Tuple{Any})
function checkbounds_indices(::Type{Bool}, A::AbstractArray, IA::Tuple, I::Tuple{Any})
@_inline_meta
checkindex(Bool, OneTo(trailingsize(IA)), I[1]) # linear indexing
checkbounds_linear_indices(Bool, A, IA, I[1])
end
checkbounds_indices(::Type{Bool}, ::Tuple, ::Tuple{}) = true
function checkbounds_linear_indices{T,N}(::Type{Bool}, A::AbstractArray{T,N}, ::NTuple{N}, i)
@_inline_meta
checkindex(Bool, linearindices(A), i) # linear indexing
end
function checkbounds_linear_indices(::Type{Bool}, A::AbstractArray, 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(ndims(A) - length(IA) + 1)
return true # TODO: Return false after the above function is removed in deprecated.jl
end
return false
end
function checkbounds_linear_indices(::Type{Bool}, A::AbstractArray, IA::Tuple, i::Union{Slice,Colon})
partial_linear_indexing_warning(ndims(A) - length(IA) + 1)
true
end
checkbounds_indices(::Type{Bool}, ::AbstractArray, ::Tuple, ::Tuple{}) = true

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

Expand Down
5 changes: 5 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1745,6 +1745,11 @@ end
export @test_approx_eq
# END code from base/test.jl

# Deprecate partial linear indexing
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{T,N}(::Type{T}, d::NTuple{N,Int}) Array{T,N}(d)
@deprecate Array{T}(::Type{T}, d::Int...) Array{T,length(d)}(d...)
Expand Down
32 changes: 16 additions & 16 deletions base/multidimensional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,12 @@ end # IteratorsMD
using .IteratorsMD

## Bounds-checking with CartesianIndex
@inline checkbounds_indices(::Type{Bool}, ::Tuple{}, I::Tuple{CartesianIndex,Vararg{Any}}) =
checkbounds_indices(Bool, (), (I[1].I..., tail(I)...))
@inline checkbounds_indices(::Type{Bool}, IA::Tuple{Any}, I::Tuple{CartesianIndex,Vararg{Any}}) =
checkbounds_indices(Bool, IA, (I[1].I..., tail(I)...))
@inline checkbounds_indices(::Type{Bool}, IA::Tuple, I::Tuple{CartesianIndex,Vararg{Any}}) =
checkbounds_indices(Bool, IA, (I[1].I..., tail(I)...))
@inline checkbounds_indices(::Type{Bool}, A::AbstractArray, ::Tuple{}, I::Tuple{CartesianIndex,Vararg{Any}}) =
checkbounds_indices(Bool, A, (), (I[1].I..., tail(I)...))
@inline checkbounds_indices(::Type{Bool}, A::AbstractArray, IA::Tuple{Any}, I::Tuple{CartesianIndex,Vararg{Any}}) =
checkbounds_indices(Bool, A, IA, (I[1].I..., tail(I)...))
@inline checkbounds_indices(::Type{Bool}, A::AbstractArray, IA::Tuple, I::Tuple{CartesianIndex,Vararg{Any}}) =
checkbounds_indices(Bool, A, IA, (I[1].I..., tail(I)...))

# Indexing into Array with mixtures of Integers and CartesianIndices is
# extremely performance-sensitive. While the abstract fallbacks support this,
Expand All @@ -184,24 +184,24 @@ using .IteratorsMD
# Support indexing with an array of CartesianIndex{N}s
# Here we try to consume N of the indices (if there are that many available)
# The first two simply handle ambiguities
@inline function checkbounds_indices{N}(::Type{Bool}, ::Tuple{}, I::Tuple{AbstractArray{CartesianIndex{N}},Vararg{Any}})
checkindex(Bool, (), I[1]) & checkbounds_indices(Bool, (), tail(I))
@inline function checkbounds_indices{N}(::Type{Bool}, A::AbstractArray, ::Tuple{}, I::Tuple{AbstractArray{CartesianIndex{N}},Vararg{Any}})
checkindex(Bool, A, (), I[1]) & checkbounds_indices(Bool, A, (), tail(I))
end
@inline function checkbounds_indices(::Type{Bool}, IA::Tuple{Any}, I::Tuple{AbstractArray{CartesianIndex{0}},Vararg{Any}})
checkbounds_indices(Bool, IA, tail(I))
@inline function checkbounds_indices(::Type{Bool}, A::AbstractArray, IA::Tuple{Any}, I::Tuple{AbstractArray{CartesianIndex{0}},Vararg{Any}})
checkbounds_indices(Bool, A, IA, tail(I))
end
@inline function checkbounds_indices{N}(::Type{Bool}, IA::Tuple{Any}, I::Tuple{AbstractArray{CartesianIndex{N}},Vararg{Any}})
checkindex(Bool, IA, I[1]) & checkbounds_indices(Bool, (), tail(I))
@inline function checkbounds_indices{N}(::Type{Bool}, A::AbstractArray, IA::Tuple{Any}, I::Tuple{AbstractArray{CartesianIndex{N}},Vararg{Any}})
checkindex(Bool, A, IA, I[1]) & checkbounds_indices(Bool, A, (), tail(I))
end
@inline function checkbounds_indices{N}(::Type{Bool}, IA::Tuple, I::Tuple{AbstractArray{CartesianIndex{N}},Vararg{Any}})
@inline function checkbounds_indices{N}(::Type{Bool}, A::AbstractArray, IA::Tuple, I::Tuple{AbstractArray{CartesianIndex{N}},Vararg{Any}})
IA1, IArest = IteratorsMD.split(IA, Val{N})
checkindex(Bool, IA1, I[1]) & checkbounds_indices(Bool, IArest, tail(I))
checkindex(Bool, A, IA1, I[1]) & checkbounds_indices(Bool, A, IArest, tail(I))
end

function checkindex{N}(::Type{Bool}, inds::Tuple, I::AbstractArray{CartesianIndex{N}})
function checkindex{N}(::Type{Bool}, A::AbstractArray, inds::Tuple, I::AbstractArray{CartesianIndex{N}})
b = true
for i in I
b &= checkbounds_indices(Bool, inds, (i,))
b &= checkbounds_indices(Bool, A, inds, (i,))
end
b
end
Expand Down
2 changes: 1 addition & 1 deletion src/array.c
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ int jl_array_isdefined(jl_value_t **args0, int nargs)
{
assert(jl_is_array(args0[0]));
jl_depwarn("`isdefined(a::Array, i::Int)` is deprecated, "
"use `isassigned(a, i)` instead", jl_symbol("isdefined"));
"use `isassigned(a, i)` instead", (jl_value_t*)jl_symbol("isdefined"));

jl_array_t *a = (jl_array_t*)args0[0];
jl_value_t **args = &args0[1];
Expand Down
26 changes: 23 additions & 3 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,7 @@ JL_CALLABLE(jl_f_invoke)
if (jl_is_tuple(args[1])) {
jl_depwarn("`invoke(f, (types...), ...)` is deprecated, "
"use `invoke(f, Tuple{types...}, ...)` instead",
jl_symbol("invoke"));
(jl_value_t*)jl_symbol("invoke"));
argtypes = (jl_value_t*)jl_apply_tuple_type_v((jl_value_t**)jl_data_ptr(argtypes),
jl_nfields(argtypes));
}
Expand Down Expand Up @@ -1701,7 +1701,7 @@ JL_DLLEXPORT void jl_breakpoint(jl_value_t *v)
// put a breakpoint in your debugger here
}

void jl_depwarn(const char *msg, jl_sym_t *sym)
void jl_depwarn(const char *msg, jl_value_t *sym)
{
static jl_value_t *depwarn_func = NULL;
if (!depwarn_func && jl_base_module) {
Expand All @@ -1715,11 +1715,31 @@ void jl_depwarn(const char *msg, jl_sym_t *sym)
JL_GC_PUSHARGS(depwarn_args, 3);
depwarn_args[0] = depwarn_func;
depwarn_args[1] = jl_cstr_to_string(msg);
depwarn_args[2] = (jl_value_t*)sym;
depwarn_args[2] = sym;
jl_apply(depwarn_args, 3);
JL_GC_POP();
}

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
26 changes: 24 additions & 2 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1352,8 +1352,30 @@ static Value *emit_array_nd_index(const jl_cgval_t &ainfo, jl_value_t *ex, ssize
if (linear_indexing) {
// Compare the linearized index `i` against the linearized size of
// the accessed array, i.e. `if !(i < alen) goto error`.
Value *alen = emit_arraylen(ainfo, ex, ctx);
builder.CreateCondBr(builder.CreateICmpULT(i, alen), endBB, failBB);
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(ainfo, ex, nidxs, nd, ctx);
builder.CreateCondBr(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);
builder.SetInsertPoint(partidx);
Value *alen = emit_arraylen(ainfo, ex, ctx);
builder.CreateCondBr(builder.CreateICmpULT(i, alen), partidxwarn, failBB);

// We passed the linearized bounds check; now throw the depwarn:
ctx->f->getBasicBlockList().push_back(partidxwarn);
builder.SetInsertPoint(partidxwarn);
builder.CreateCall(prepare_call(jldepwarnpi_func), ConstantInt::get(T_size, nidxs));
builder.CreateBr(endBB);
} else {
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: 8 additions & 0 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ static Function *expect_func;
static Function *jldlsym_func;
static Function *jlnewbits_func;
static Function *jltypeassert_func;
static Function *jldepwarnpi_func;
#if JL_LLVM_VERSION < 30600
static Function *jlpow_func;
static Function *jlpowf_func;
Expand Down Expand Up @@ -5767,6 +5768,13 @@ static void init_julia_llvm_env(Module *m)
"jl_typeassert", m);
add_named_global(jltypeassert_func, &jl_typeassert);

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);

queuerootfun = Function::Create(FunctionType::get(T_void, args_1ptr, false),
Function::ExternalLinkage,
"jl_gc_queue_root", m);
Expand Down
3 changes: 2 additions & 1 deletion src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,8 @@ STATIC_INLINE void *jl_get_frame_addr(void)
}

JL_DLLEXPORT jl_array_t *jl_array_cconvert_cstring(jl_array_t *a);
void jl_depwarn(const char *msg, jl_sym_t *sym);
void jl_depwarn(const char *msg, jl_value_t *sym);
void jl_depwarn_partial_indexing(size_t n);

int isabspath(const char *in);

Expand Down
Loading

0 comments on commit 58c22b7

Please sign in to comment.