Skip to content

Commit

Permalink
BitArrays should not be DenseArrays (#25858)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mbauman authored Feb 8, 2018
1 parent 24ec0f6 commit 94ee4f5
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 10 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
5 changes: 4 additions & 1 deletion base/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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

Expand Down
19 changes: 10 additions & 9 deletions base/multidimensional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions test/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 94ee4f5

Please sign in to comment.