From 94ee4f5a3096d004cc3d594e5d0a81fc9dcf137a Mon Sep 17 00:00:00 2001
From: Matt Bauman <mbauman@gmail.com>
Date: Thu, 8 Feb 2018 15:49:07 -0600
Subject: [PATCH] BitArrays should not be DenseArrays (#25858)

BitArrays should not be DenseArrays

BitArray has previously been subtype of DenseArray, making it defined as a StridedArray. However - BitArrays do not and can not support the `pointer` function. As such, they can never be passed to a C function in the same manner that all other StridedArrays can.

This patch makes `BitArray <: AbstractArray`. Surprisingly little breaks in our test set, but this is indeed massively breaking.  Notably, most of the other changes required here were actually long-standing bugs that this change simply happened to uncover.
---
 NEWS.md                  |  3 +++
 base/bitarray.jl         |  5 ++++-
 base/multidimensional.jl | 19 ++++++++++---------
 test/bitarray.jl         |  8 ++++++++
 4 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index fd68ecce6d5ed..af52684046d51 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -269,6 +269,9 @@ This section lists changes that do not have deprecation warnings.
     `Tridiagonal{T,V<:AbstractVector{T}}` and `SymTridiagonal{T,V<:AbstractVector{T}}`
     respectively ([#22718], [#22925], [#23035], [#23154]).
 
+  * The immediate supertype of `BitArray` is now simply `AbstractArray`. `BitArray` is no longer
+    considered a subtype of `DenseArray` and `StridedArray` ([#25858]).
+
   * When called with an argument that contains `NaN` elements, `findmin` and `findmax` now return the
     first `NaN` found and its corresponding index. Previously, `NaN` elements were ignored.
     The new behavior matches that of `min`, `max`, `minimum`, and `maximum`.
diff --git a/base/bitarray.jl b/base/bitarray.jl
index a94f2af6a733d..e8b2aaa3b0008 100644
--- a/base/bitarray.jl
+++ b/base/bitarray.jl
@@ -9,7 +9,7 @@
 
 Space-efficient `N`-dimensional boolean array, which stores one bit per boolean value.
 """
-mutable struct BitArray{N} <: DenseArray{Bool, N}
+mutable struct BitArray{N} <: AbstractArray{Bool, N}
     chunks::Vector{UInt64}
     len::Int
     dims::NTuple{N,Int}
@@ -657,6 +657,9 @@ indexoffset(::Colon) = 0
     fill_chunks!(B.chunks, y, f0, l0)
     return B
 end
+@propagate_inbounds function setindex!(B::BitArray, X::AbstractArray, J0::Union{Colon,UnitRange{Int}})
+    _setindex!(IndexStyle(B), B, X, to_indices(B, (J0,))[1])
+end
 
 # logical indexing
 
diff --git a/base/multidimensional.jl b/base/multidimensional.jl
index 36e4c03f1f8aa..2576140929370 100644
--- a/base/multidimensional.jl
+++ b/base/multidimensional.jl
@@ -1462,7 +1462,7 @@ end
 # contiguous multidimensional indexing: if the first dimension is a range,
 # we can get some performance from using copy_chunks!
 
-@inline function setindex!(B::BitArray, X::StridedArray, J0::Union{Colon,UnitRange{Int}})
+@inline function setindex!(B::BitArray, X::Union{StridedArray,BitArray}, J0::Union{Colon,UnitRange{Int}})
     I0 = to_indices(B, (J0,))[1]
     @boundscheck checkbounds(B, I0)
     l0 = length(I0)
@@ -1473,13 +1473,13 @@ end
     return B
 end
 
-@inline function setindex!(B::BitArray, X::StridedArray,
+@inline function setindex!(B::BitArray, X::Union{StridedArray,BitArray},
         I0::Union{Colon,UnitRange{Int}}, I::Union{Int,UnitRange{Int},Colon}...)
     J = to_indices(B, (I0, I...))
     @boundscheck checkbounds(B, J...)
     _unsafe_setindex!(B, X, J...)
 end
-@generated function _unsafe_setindex!(B::BitArray, X::StridedArray,
+@generated function _unsafe_setindex!(B::BitArray, X::Union{StridedArray,BitArray},
         I0::Union{Slice,UnitRange{Int}}, I::Union{Int,UnitRange{Int},Slice}...)
     N = length(I)
     quote
@@ -1520,6 +1520,11 @@ end
     @boundscheck checkbounds(B, J...)
     _unsafe_setindex!(B, x, J...)
 end
+@propagate_inbounds function setindex!(B::BitArray, X::AbstractArray,
+        I0::Union{Colon,UnitRange{Int}}, I::Union{Int,UnitRange{Int},Colon}...)
+    _setindex!(IndexStyle(B), B, X, to_indices(B, (I0, I...))...)
+end
+
 @generated function _unsafe_setindex!(B::BitArray, x,
         I0::Union{Slice,UnitRange{Int}}, I::Union{Int,UnitRange{Int},Slice}...)
     N = length(I)
@@ -1601,17 +1606,13 @@ for (V, PT, BT) in [((:N,), BitArray, BitArray), ((:T,:N), Array, StridedArray)]
             checkdims_perm(P, B, perm)
 
             #calculates all the strides
+            native_strides = size_to_strides(1, size(B)...)
             strides_1 = 0
-            @nexprs $N d->(strides_{d+1} = stride(B, perm[d]))
+            @nexprs $N d->(strides_{d+1} = native_strides[perm[d]])
 
             #Creates offset, because indexing starts at 1
             offset = 1 - sum(@ntuple $N d->strides_{d+1})
 
-            if isa(B, SubArray)
-                offset += first_index(B::SubArray) - 1
-                B = B.parent
-            end
-
             ind = 1
             @nexprs 1 d->(counts_{$N+1} = strides_{$N+1}) # a trick to set counts_($N+1)
             @nloops($N, i, P,
diff --git a/test/bitarray.jl b/test/bitarray.jl
index 0d2c31572641d..680f72eb5a461 100644
--- a/test/bitarray.jl
+++ b/test/bitarray.jl
@@ -1491,3 +1491,11 @@ end
 end
 
 timesofar("I/O")
+
+@testset "not strided" begin
+    @test_throws ErrorException pointer(trues(1))
+    @test_throws ErrorException pointer(trues(1),1)
+    b = falses(3)
+    b[:] = view(trues(10), [1,3,7])
+    @test b == trues(3)
+end