Description
openedon Sep 2, 2017
Noticed in the current master bug:
julia> t = Vector{Union{Float64, Void}}(5)
5-element Array{Union{Void, Float64},1}:
nothing
nothing
nothing
nothing
nothing
julia> b = collect(Union{Float64, Void}, 1.0:3.0)
3-element Array{Union{Void, Float64},1}:
1.0
2.0
3.0
julia> copy!(t, 2, b)
5-element Array{Union{Void, Float64},1}:
0.0
1.0
2.0
nothing
nothing
The issue is that we currently go thru the codepath in array.jl
function unsafe_copy!(dest::Array{T}, doffs, src::Array{T}, soffs, n) where T
if isbits(T)
unsafe_copy!(pointer(dest, doffs), pointer(src, soffs), n)
else
ccall(:jl_array_ptr_copy, Void, (Any, Ptr{Void}, Any, Ptr{Void}, Int),
dest, pointer(dest, doffs), src, pointer(src, soffs), n)
end
but isbits(Union{Float64, Void})
is false; so we hit jl_array_ptr_copy
. Now, I did had a hook in jl_array_ptr_copy
to intercept isbits-Union arrays and handle them specially, the problem is it currently only works when you happen to be copying to the 1st element of the dest array. This is because we only pass dest_p
, a pointer and not an index, to jl_array_ptr_copy
, so we don't actually know where to copy selector bytes from src to dest.
There a couple of ways to address this:
- go ahead and make
isbits(Union{Float64, Void}) == true
, this would involve making sure we're careful in a # of places in Base where we're currently checkingisbits(T)
on arrays and doing optimizations (i.e.vcat
,unsafe_copy
,flipdim
). The biggest ? for me in doing this is how this propagates thru the rest of the system, in particular, inference.jl (which has quite a few calls toisbits(T)
.). Overall, I think it would be more correct, but it does seem a bit worrisome not quite knowing the full extent of this kind of change. I can certainly do more research here if we think it's the best way to try out first. - make another method
jl_array_ptr_copy2
, which also takesind
arguments so we can correctly adjust when copying isbits-Union arrays.
Ultimately, I think we should make isbits-Unions isbits, because we're essentially doing that everywhere in src/
, so we might as well be consistent in Base.
cc: @JeffBezanson, @vtjnash