Skip to content

Commit

Permalink
Use Undef markers for undefined fields
Browse files Browse the repository at this point in the history
Previously BSON would skip these entirely causing mis-ordered
`setfield!` calls when loading the struct.
  • Loading branch information
topolarity committed Jan 19, 2024
1 parent 3d6da1f commit eea6320
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
15 changes: 13 additions & 2 deletions src/extensions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,16 @@ tags[:array] = d ->

# Structs

structdata(x) = isprimitivetype(typeof(x)) ? reinterpret_(UInt8, [x]) :
Any[getfield(x,f) for f in fieldnames(typeof(x)) if isdefined(x, f)]
struct Undef end
function structdata(x)
if isprimitivetype(typeof(x))
return reinterpret_(UInt8, [x])
elseif !ismutabletype(typeof(x))
return Any[getfield(x,f) for f in fieldnames(typeof(x)) if isdefined(x, f)]
else # mutable structs can have defined fields following undefined fields
return Any[isdefined(x, f) ? getfield(x,f) : Undef() for f in fieldnames(typeof(x))]
end
end

function lower(x)
BSONDict(:tag => "struct", :type => typeof(x), :data => structdata(x))
Expand All @@ -119,6 +127,7 @@ initstruct(T) = ccall(:jl_new_struct_uninit, Any, (Any,), T)

function newstruct!(x, fs...)
for (i, f) = enumerate(fs)
isa(f, Undef) && continue
f = convert(fieldtype(typeof(x),i), f)
ccall(:jl_set_nth_field, Nothing, (Any, Csize_t, Any), x, i-1, f)
end
Expand All @@ -136,6 +145,7 @@ function newstruct(T, xs...)
x = initstruct(T)

for (i, f) = enumerate(xs)
isa(f, Undef) && continue

Check warning on line 148 in src/extensions.jl

View check run for this annotation

Codecov / codecov/patch

src/extensions.jl#L148

Added line #L148 was not covered by tests
f = convert(fieldtype(typeof(x),i), f)
ccall(:jl_set_nth_field, Nothing, (Any, Csize_t, Any), x, i-1, f)
end
Expand All @@ -154,6 +164,7 @@ function newstruct(T, xs...)
x = initstruct(T)

for (i, f) = enumerate(xs)
isa(f, Undef) && continue

Check warning on line 167 in src/extensions.jl

View check run for this annotation

Codecov / codecov/patch

src/extensions.jl#L167

Added line #L167 was not covered by tests
f = convert(fieldtype(typeof(x),i), f)
ccall(:jl_set_nth_field, Nothing, (Any, Csize_t, Any), x, i-1, f)
end
Expand Down
23 changes: 23 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,25 @@ struct Bar
Bar() = new()
end

mutable struct Baz
x
y
z
Baz() = new()
end

function is_field_equal(a, b, field::Symbol)
!isdefined(a, field) && return !isdefined(b, field)
!isdefined(b, field) && return false
return getfield(a, field) == getfield(b, field)
end

function Base.:(==)(a::Baz, b::Baz)
return is_field_equal(a, b, :x) &&
is_field_equal(a, b, :y) &&
is_field_equal(a, b, :z)
end

module A
using DataFrames, BSON
d = DataFrame(a = 1:10, b = rand(10))
Expand Down Expand Up @@ -72,6 +91,10 @@ end
@test_broken roundtrip_equal(Dict(:x => x))

@test roundtrip_equal(Bar())

o = Baz()
o.y = 1
@test roundtrip_equal(o)
end

@testset "Complex Types" begin
Expand Down

0 comments on commit eea6320

Please sign in to comment.