Problem with generic copyto!

[bkamins]

The issue is:

julia> x = view(["A", "B"], 1:2)
2-element view(::Vector{String}, 1:2) with eltype String:
 "A"
 "B"

julia> y = Vector{String}(undef, 2)
2-element Vector{String}:
 #undef
 #undef

julia> copyto!(x, y)
ERROR: UndefRefError: access to undefined reference
Stacktrace:
 [1] getindex
   @ .\array.jl:861 [inlined]
 [2] copyto_unaliased!(deststyle::IndexLinear, dest::SubArray{String, 1, Vector{String}, Tuple{UnitRange{Int64}}, true}, srcstyle::IndexLinear, src::Vector{String})
   @ Base .\abstractarray.jl:1018
 [3] copyto!(dest::SubArray{String, 1, Vector{String}, Tuple{UnitRange{Int64}}, true}, src::Vector{String})
   @ Base .\abstractarray.jl:998
 [4] top-level scope
   @ REPL[19]:1

The reason is

julia/base/abstractarray.jl

Line 1039 in 6026b5f

dest[i + Δi] = src[i]

(and similar operation below this one) assuming that all elements of source array are defined.

If I understand the intention correctly this is a bug in copyto! as e.g.

julia> x = ["A", "b"]
2-element Vector{String}:
 "A"
 "b"

julia> y = Vector{String}(undef, 2)
2-element Vector{String}:
 #undef
 #undef

julia> copyto!(x, y)
2-element Vector{String}:
 #undef
 #undef

julia> x
2-element Vector{String}:
 #undef
 #undef

works.

[mcabbott]

Since iterate(y) fails, I am surprised that copyto!(x, y) works. It could be that this success is the unintended behaviour, as the lower-level unsafe function doesn't object.

[bkamins]

That is why I opened it as an issue not a PR.

However, as you will see in this PR in generic code users could assume that copyto! will work always if types of x and y match (i.e. support the operation).

[KristofferC]

I would say that

julia> copyto!(x, y)
2-element Vector{String}:
 #undef
 #undef

is the bug. There is (almost) never a case where you can undefine values that have once been defined.

[jakobnissen]

Since #undef elements is implemented in Julia using null pointers, it will probably be very fast to check for the presence of these in the source array - especially considering that it will then be in cache for the upcoming copy operation.

Perhaps a check could be added in the copy! implementation? E.g. it could be done in the following way to add negligible overhead:

function copy_checked(dst::Array{T}, src::Array{T}) where T
    GC.@preserve src dst begin
        sptr = Ptr{UInt}(pointer(src))
        dptr = Ptr{UInt}(pointer(dst))
        uninit = false
        for i in eachindex(src)
            p = unsafe_load(sptr, i)
            uninit |= iszero(p)
            unsafe_store!(dptr, p, i)
        end
    end
    uninit && error()
    dst
end

[quinnj]

Related: #36564

[bkamins]

bump - for my purposes I am OK with any behavior as long as it is clear what contract copyto! has with respect to #undef.

[JeffBezanson]

I believe this was added for a use case in the compiler. I would prefer the compiler to use some different approach if possible, and roll this back, since it is unnerving for it to be inconsistent like this.

[jishnub]

The OP now works on Julia v1.11.0-beta2:

julia> x = view(["A", "B"], 1:2)
2-element view(::Vector{String}, 1:2) with eltype String:
 "A"
 "B"

julia> y = Vector{String}(undef, 2)
2-element Vector{String}:
 #undef
 #undef

julia> copyto!(x, y)
2-element view(::Vector{String}, 1:2) with eltype String:
 #undef
 #undef

[adienes]

not sure I would use the word "works", as this seems worse behavior than before to me

of course unsafe_copyto! can do whatever it wants, but using copyto! I'd really expect an UndefRefError here

[jishnub]

This seems to have been a deliberate choice: #51760

[vtjnash]

Fixed in #51760

[vtjnash]

Broken by #54332