Simplify and relax logical indexing bounds checks

Fixes #18271. Allow multidimensional logical arrays as indices into a particular dimension so long as there is only one non-singleton dimension.

(cherry picked from commit 5fa5b7f)
ref #18401

Fixup inline comment

(cherry picked from commit 2bbcd80)

Update devdocs for bounds checking following #18401

I had forgotten that this behavior was documented in the developer section of the manual when authoring #18401.

(cherry picked from commit b8406ff)
ref #18552

base/abstractarray.jl

@@ -243,9 +243,9 @@ linearindexing(::LinearIndexing, ::LinearIndexing) = LinearSlow()
 
 # The overall hierarchy is
 #     `checkbounds(A, I...)` ->
-#         `checkbounds(Bool, A, I...)` -> either of:
-#             - `checkbounds_logical(Bool, A, I)` when `I` is a single logical array
-#             - `checkbounds_indices(Bool, IA, I)` otherwise (uses `checkindex`)
+#         `checkbounds(Bool, A, I...)` ->
+#             `checkbounds_indices(Bool, IA, I)`, which recursively calls
+#                 `checkindex` for each dimension
 #
 # See the "boundscheck" devdocs for more information.
 #
@@ -269,9 +269,10 @@ function checkbounds(::Type{Bool}, A::AbstractArray, I...)
     @_inline_meta
     checkbounds_indices(Bool, indices(A), I)
 end
-function checkbounds(::Type{Bool}, A::AbstractArray, I::AbstractArray{Bool})
+# 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})
     @_inline_meta
-    checkbounds_logical(Bool, A, I)
+    indices(A) == indices(I)
 end
 
 """
@@ -323,35 +324,6 @@ function checkbounds_indices(::Type{Bool}, IA::Tuple, I::Tuple{Any})
     checkindex(Bool, OneTo(trailingsize(IA)), I[1])  # linear indexing
 end
 
-"""
-    checkbounds_logical(Bool, A, I::AbstractArray{Bool})
-
-Return `true` if the logical array `I` is consistent with the indices
-of `A`. `I` and `A` should have the same size and compatible indices.
-"""
-function checkbounds_logical(::Type{Bool}, A::AbstractArray, I::AbstractArray{Bool})
-    indices(A) == indices(I)
-end
-function checkbounds_logical(::Type{Bool}, A::AbstractArray, I::AbstractVector{Bool})
-    length(A) == length(I)
-end
-function checkbounds_logical(::Type{Bool}, A::AbstractVector, I::AbstractArray{Bool})
-    length(A) == length(I)
-end
-function checkbounds_logical(::Type{Bool}, A::AbstractVector, I::AbstractVector{Bool})
-    indices(A) == indices(I)
-end
-
-"""
-    checkbounds_logical(A, I::AbstractArray{Bool})
-
-Throw an error if the logical array `I` is inconsistent with the indices of `A`.
-"""
-function checkbounds_logical(A, I::AbstractVector{Bool})
-    checkbounds_logical(Bool, A, I) || throw_boundserror(A, I)
-    nothing
-end
-
 throw_boundserror(A, I) = (@_noinline_meta; throw(BoundsError(A, I)))
 
 # check along a single dimension
@@ -370,7 +342,19 @@ function checkindex(::Type{Bool}, inds::AbstractUnitRange, r::Range)
     @_propagate_inbounds_meta
     isempty(r) | (checkindex(Bool, inds, first(r)) & checkindex(Bool, inds, last(r)))
 end
-checkindex{N}(::Type{Bool}, indx::AbstractUnitRange, I::AbstractArray{Bool,N}) = N == 1 && indx == indices1(I)
+checkindex(::Type{Bool}, indx::AbstractUnitRange, I::AbstractVector{Bool}) = indx == indices1(I)
+# Logical indexing is an exception to the "output dimensionality is the sum of
+# the dimensionality of the indices" rule; `A[A.<0]` always returns a vector,
+# regardless of the dimensionalities of `A and `A.<0`. This method goes one step
+# further and ignores singleton dimensions for logical mask indices. While a
+# little strange, it enables idioms like `A[:, sum(A, 1) .< 0]`. Ref #18271.
+function checkindex(::Type{Bool}, indx::AbstractUnitRange, I::AbstractArray{Bool})
+    # Ensure that there's no more than one non-singleton dimension and that it
+    # matches the source array's index. Note that there's an ambiguity at length
+    # 1, since we cannot tell which dimension should be the non-singleton one.
+    @_inline_meta
+    length(indx) == prod(map(length, indices(I))) && any(x->x==indx, indices(I))
+end
 function checkindex(::Type{Bool}, inds::AbstractUnitRange, I::AbstractArray)
     @_inline_meta
     b = true

doc/devdocs/boundscheck.rst

@@ -66,9 +66,9 @@ The bounds checking call hierarchy
 The overall hierarchy is:
 
 |   ``checkbounds(A, I...)`` which calls
-|     ``checkbounds(Bool, A, I...)`` which calls either of:
-|       ``checkbounds_logical(Bool, A, I)``  when ``I`` is a single logical array
-|       ``checkbounds_indices(Bool, indices(A), I)`` otherwise
+|     ``checkbounds(Bool, A, I...)`` which calls
+|       ``checkbounds_indices(Bool, indices(A), I)`` which recursively calls
+|         ``checkindex`` for each dimension
 |
 
 Here ``A`` is the array, and ``I`` contains the "requested" indices.
@@ -89,8 +89,8 @@ dimensions handled by calling another important function,
                                                          checkbounds_indices(Bool, IA, I)
 
 so ``checkindex`` checks a single dimension.  All of these functions,
-including the unexported ``checkbounds_indices`` and
-``checkbounds_logical``, have docstrings accessible with ``?`` .
+including the unexported ``checkbounds_indices`` have docstrings
+accessible with ``?`` .
 
 If you have to customize bounds checking for a specific array type,
 you should specialize ``checkbounds(Bool, A, I...)``. However, in most

test/abstractarray.jl

@@ -79,6 +79,13 @@ A = rand(5,4,3)
 @test checkbounds(Bool, A, trues(5), trues(12)) == true
 @test checkbounds(Bool, A, trues(5), trues(13)) == false
 @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
+@test checkbounds(Bool, A, trues(1, 5), trues(1, 4, 1), trues(1, 1, 3)) == true
+@test checkbounds(Bool, A, trues(1, 5), trues(1, 4, 1), trues(1, 1, 2)) == false
+@test checkbounds(Bool, A, trues(1, 5), trues(1, 5, 1), trues(1, 1, 3)) == false
+@test checkbounds(Bool, A, trues(1, 5), :, 2) == true
 
 # array of CartesianIndex
 @test checkbounds(Bool, A, [CartesianIndex((1, 1, 1))]) == true