Deprecate partial linear indexing

Author: mbauman

base/abstractarray.jl

@@ -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})
@@ -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.
@@ -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)))
 

base/deprecated.jl

@@ -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...)

base/multidimensional.jl

@@ -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,
@@ -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

src/array.c

@@ -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];

src/builtins.c

@@ -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));
     }
@@ -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) {
@@ -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

src/cgutils.cpp

@@ -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`.

src/codegen.cpp

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

src/julia_internal.h

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