jw3126 / Setfield.jl

Update deeply nested immutable structs.
Other
167 stars 17 forks source link

`setindex` fails on arrays with boxed `undef` #157

Open torfjelde opened 3 years ago

torfjelde commented 3 years ago
julia> x = Vector{Real}(undef, 2)
2-element Vector{Real}:
 #undef
 #undef

julia> @set! x[1] = 1.0
ERROR: UndefRefError: access to undefined reference
Stacktrace:
 [1] getindex
   @ ./array.jl:801 [inlined]
 [2] copy!(dst::Vector{Real}, src::Vector{Real})
   @ Base ./abstractarray.jl:827
 [3] setindex
   @ ~/.julia/packages/Setfield/XM37G/src/setindex.jl:9 [inlined]
 [4] set(obj::Vector{Real}, l::Setfield.IndexLens{Tuple{Int64}}, val::Float64)
   @ Setfield ~/.julia/packages/Setfield/XM37G/src/lens.jl:181
 [5] top-level scope
   @ ~/.julia/packages/Setfield/XM37G/src/sugar.jl:182

It correctly works when eltype isbitstype since then you don't end up with undef but instead an "unitialized" value, for example

julia> x = Vector{Float32}(undef, 2)
2-element Vector{Float32}:
 1.0f-45
 1.7f-44

julia> @set! x[1] = 1.0
2-element Vector{Float64}:
 1.0
 1.6815581571897805e-44

Maybe the way to go is:

Base.@propagate_inbounds function setindex(xs::AbstractArray, v, I...)
    T = promote_type(eltype(xs), typeof(v))
    ys = similar(xs, T)
    if isbitstype(T)
        copy!(ys, xs)
    elseif eltype(xs) !== Union{}
    for index in CartesianIndices(xs)
        if isassigned(xs, Tuple(index)...)
        ys[index] = xs[index]
        end
    end
    end
    ys[I...] = v
    return ys
end

? Seems sub-optimal though.

This is not an urgent issue to resolve for our sake, but figured I'd at least raise it here for now. In the meantime (and this is what we'll do anyways as it aligns better with the behavior we want) one can use BangBang.@set!! which uses BangBang.prefermutation instead of identity in setmacro.

jw3126 commented 3 years ago

Side note, ideally this would be resolved in Base https://github.com/JuliaLang/julia/pull/33495