Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BitArrays should not be DenseArrays #25858

Merged
merged 2 commits into from
Feb 8, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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 committed Feb 2, 2018
commit 9f1e71f74a326ad924187411f431f4f98c0ce066
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,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` ([#25857]).

* 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