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

copyto! fix for BitArray/AbstractArray #46161

Merged
merged 3 commits into from
Jul 25, 2022
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
Fix 5-args copyto! for BitArray/AbstractArray
1. map `copyto!(::BitArray, n1, ::BitArray, n2, l)` to `Base.unsafe_copyto!`
2. add missing unaliasing in `copyto!` for `AbstractArray`
  • Loading branch information
N5N3 committed Jul 25, 2022
commit a3334585d3c72c56b590c05c4523b4c9f77348bb
15 changes: 8 additions & 7 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -894,10 +894,9 @@ function copy!(dst::AbstractVector, src::AbstractVector)
if length(dst) != length(src)
resize!(dst, length(src))
end
for i in eachindex(dst, src)
@inbounds dst[i] = src[i]
end
dst
firstindex(dst) == firstindex(src) || throw(ArgumentError(
N5N3 marked this conversation as resolved.
Show resolved Hide resolved
"vectors must have the same offset for copy! (consider using `copyto!`)"))
copyto!(dst, src)
end

function copy!(dst::AbstractArray, src::AbstractArray)
Expand Down Expand Up @@ -1108,8 +1107,9 @@ function copyto!(dest::AbstractArray, dstart::Integer,
destinds, srcinds = LinearIndices(dest), LinearIndices(src)
(checkbounds(Bool, destinds, dstart) && checkbounds(Bool, destinds, dstart+n-1)) || throw(BoundsError(dest, dstart:dstart+n-1))
(checkbounds(Bool, srcinds, sstart) && checkbounds(Bool, srcinds, sstart+n-1)) || throw(BoundsError(src, sstart:sstart+n-1))
@inbounds for i = 0:(n-1)
dest[dstart+i] = src[sstart+i]
src′ = unalias(dest, src)
@inbounds for i = 0:n-1
dest[dstart+i] = src′[sstart+i]
end
return dest
end
Expand All @@ -1131,11 +1131,12 @@ function copyto!(B::AbstractVecOrMat{R}, ir_dest::AbstractRange{Int}, jr_dest::A
end
@boundscheck checkbounds(B, ir_dest, jr_dest)
@boundscheck checkbounds(A, ir_src, jr_src)
A′ = unalias(B, A)
jdest = first(jr_dest)
for jsrc in jr_src
idest = first(ir_dest)
for isrc in ir_src
@inbounds B[idest,jdest] = A[isrc,jsrc]
@inbounds B[idest,jdest] = A[isrc,jsrc]
idest += step(ir_dest)
end
jdest += step(jr_dest)
Expand Down
4 changes: 2 additions & 2 deletions base/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -458,9 +458,9 @@ function unsafe_copyto!(dest::BitArray, doffs::Integer, src::Union{BitArray,Arra
return dest
end

copyto!(dest::BitArray, doffs::Integer, src::Array, soffs::Integer, n::Integer) =
copyto!(dest::BitArray, doffs::Integer, src::Union{BitArray,Array}, soffs::Integer, n::Integer) =
_copyto_int!(dest, Int(doffs), src, Int(soffs), Int(n))
function _copyto_int!(dest::BitArray, doffs::Int, src::Array, soffs::Int, n::Int)
function _copyto_int!(dest::BitArray, doffs::Int, src::Union{BitArray,Array}, soffs::Int, n::Int)
n == 0 && return dest
soffs < 1 && throw(BoundsError(src, soffs))
doffs < 1 && throw(BoundsError(dest, doffs))
Expand Down
9 changes: 5 additions & 4 deletions test/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@ bitcheck(x) = true
bcast_setindex!(b, x, I...) = (b[I...] .= x; b)

function check_bitop_call(ret_type, func, args...; kwargs...)
r1 = func(args...; kwargs...)
r2 = func(map(x->(isa(x, BitArray) ? Array(x) : x), args)...; kwargs...)
ret_type ≢ nothing && !isa(r1, ret_type) && @show ret_type, typeof(r1)
ret_type ≢ nothing && @test isa(r1, ret_type)
r1 = func(args...; kwargs...)
ret_type ≢ nothing && (@test isa(r1, ret_type) || @show ret_type, typeof(r1))
@test tc(r1, r2)
@test isequal(r1, ret_type ≡ nothing ? r2 : r2)
@test isequal(r1, r2)
@test bitcheck(r1)
end
macro check_bit_operation(ex, ret_type)
Expand Down Expand Up @@ -518,12 +517,14 @@ timesofar("constructors")
end
end

self_copyto!(a, n1, n2, l) = copyto!(a, n1, a, n2, l)
for p1 = [rand(1:v1) 1 63 64 65 191 192 193]
for p2 = [rand(1:v1) 1 63 64 65 191 192 193]
for n = 0 : min(v1 - p1 + 1, v1 - p2 + 1)
b1 = bitrand(v1)
b2 = bitrand(v1)
@check_bit_operation copyto!(b1, p1, b2, p2, n) BitVector
@check_bit_operation self_copyto!(b1, p1, p2, n) BitVector
end
end
end
Expand Down
7 changes: 7 additions & 0 deletions test/copy.jl
Original file line number Diff line number Diff line change
Expand Up @@ -245,3 +245,10 @@ end
@testset "deepcopy_internal arrays" begin
@test (@inferred Base.deepcopy_internal(zeros(), IdDict())) == zeros()
end

@testset "`copyto!`'s unaliasing" begin
a = view([1:3;], :)
@test copyto!(a, 2, a, 1, 2) == [1;1:2;]
a = [1:3;]
@test copyto!(a, 2:3, 1:1, a, 1:2, 1:1) == [1;1:2;]
end