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.

Sign up for GitHub

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

Jump to bottom

Fix all but one 1.7 issues #103

Merged
merged 3 commits into from
Oct 9, 2021
Merged

Fix all but one 1.7 issues #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
23c65b3
julia 1.7: mutable field fix
daviehh Jun 1, 2021
445548a
Mostly fixed the 1.7 issues except for annonymous function test
mauro3 Sep 30, 2021
4fd3ae9
Added tests to reconstruct an undef fields
mauro3 Sep 30, 2021
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
6 changes: 3 additions & 3 deletions src/anonymous.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ structdata(meth::Method) =
_uncompress(meth, meth.source)]
else
structdata(meth::Method) =
[meth.module, meth.name, meth.file, meth.line, meth.sig, getfield(meth, syms_fieldname),
[meth.module, meth.name, meth.file, meth.line, meth.sig, getfield(meth, syms_fieldname),
meth.nargs, meth.isva, meth.nospecialize, _uncompress(meth, meth.source)]
end

Expand Down Expand Up @@ -66,8 +66,8 @@ function structdata(t::TypeName)
[t.mt.name, collect(Base.MethodList(t.mt)), t.mt.max_args,
isdefined(t.mt, :kwsorter) ? t.mt.kwsorter : nothing]
[Base.string(VERSION), t.name, t.names, primary.super, primary.parameters,
primary.types, isdefined(primary, :instance), primary.abstract,
primary.mutable, primary.ninitialized, mt]
primary.types, isdefined(primary, :instance), isabstracttype(primary),
ismutabletype(primary), primary.ninitialized, mt]
end

# Type Names
Expand Down
20 changes: 14 additions & 6 deletions src/extensions.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@ lower(m::Module) = ref(modpath(m)...)

# Types

@static if VERSION < v"1.7.0-DEV"
# Borrowed from julia base
function ismutabletype(@nospecialize(t::Type))
t = Base.unwrap_unionall(t)
# TODO: what to do for `Union`?
return isa(t, DataType) && t.mutable
end
end


ismutable(::Type{<:Type}) = false

typepath(x::DataType) = [modpath(x.name.module)..., x.name.name]
Expand Down Expand Up @@ -83,10 +93,8 @@ tags[:array] = d ->

# Structs

isprimitive(T) = fieldcount(T) == 0 && T.size > 0

structdata(x) = isprimitive(typeof(x)) ? reinterpret_(UInt8, [x]) :
Any[getfield(x, f) for f in fieldnames(typeof(x))]
structdata(x) = isprimitivetype(typeof(x)) ? reinterpret_(UInt8, [x]) :
Any[getfield(x,f) for f in fieldnames(typeof(x)) if isdefined(x, f)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The if isdefined(x, f) was necessary to avoid errors. I don't actually know what it does... So, if this leads to a bug, it would be good to have a test-case which shows this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the errors? We'd need all the fields of a struct to reconstruct it iiuc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the fields was undefined thus access to it threw an error:

Primitive Types: Error During Test at /home/mauro/julia/dot-julia-dev/BSON/test/runtests.jl:41                                                                                                  
  Test threw exception                                                                                                                                                                          
  Expression: roundtrip_equal(Tuple)                                                                                                                                                            
  UndefRefError: access to undefined reference                                                                                                                                                  
  Stacktrace:                                                                                                                                                                                   
    [1] structdata                                                                                                                                                                              
      @ ~/julia/dot-julia-dev/BSON/src/extensions.jl:96 [inlined]

This is from the@test roundtrip_equal(Tuple) test. Everything else works.

I just tried @test roundtrip_equal(Tuple{Int,Float64}) and that works as well.

Copy link
Collaborator

@DhairyaLGandhi DhairyaLGandhi Sep 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify, the roundtrip_equal(Tuple) test passes with master + 1.6 but errors with 1.7?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because one of the fields for Vararg{T} is undefined on construction. See my comment.


function lower(x)
BSONDict(:tag => "struct", :type => typeof(x), :data => structdata(x))
Expand All @@ -103,7 +111,7 @@ function newstruct!(x, fs...)
end

function newstruct(T, xs...)
if !T.mutable
if !ismutabletype(T)
flds = Any[convert(fieldtype(T, i), x) for (i,x) in enumerate(xs)]
return ccall(:jl_new_structv, Any, (Any,Ptr{Cvoid},UInt32), T, flds, length(flds))
else
Expand All @@ -129,7 +137,7 @@ end
newprimitive(T, data) = reinterpret_(T, data)[1]

tags[:struct] = d ->
isprimitive(d[:type]) ?
isprimitivetype(d[:type]) ?
newprimitive(d[:type], d[:data]) :
newstruct(d[:type], d[:data]...)

Expand Down
2 changes: 1 addition & 1 deletion src/write.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ lower(x::Primitive) = x

import Base: RefValue

ismutable(T) = !isa(T, DataType) || T.mutable
ismutable(T) = !isa(T, DataType) || ismutabletype(T)
ismutable(::Type{String}) = false

typeof_(x) = typeof(x)
Expand Down
45 changes: 35 additions & 10 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ end

# avoid hitting bug where
# Dict{Symbol,T} -> Dict{Symbol,Any}
function roundtrip_equal(x::Dict{Symbol})
function roundtrip_equal(x::Dict{Symbol})
y = BSON.roundtrip(x)
y isa Dict{Symbol} && y == x
end
Expand All @@ -23,6 +23,11 @@ end

struct S end

struct Bar
a
Bar() = new()
end

module A
using DataFrames, BSON
d = DataFrame(a = 1:10, b = rand(10))
Expand All @@ -39,6 +44,22 @@ end
@test roundtrip_equal("b")
@test roundtrip_equal([1,"b"])
@test roundtrip_equal(Tuple)
@test roundtrip_equal(Tuple{Int, Float64})
@test roundtrip_equal(Vararg{Any})
end

@testset "Undefined References" begin
# from Issue #3
d = Dict(:a => 1, :b => Dict(:c => 3, :d => Dict("e" => 5)))
@test roundtrip_equal(d)

# from Issue #43
x = Array{String, 1}(undef, 5)
x[1] = "a"
x[4] = "d"
@test_broken roundtrip_equal(Dict(:x => x))

@test roundtrip_equal(Bar())
end

@testset "Complex Types" begin
Expand Down Expand Up @@ -82,15 +103,19 @@ end

@testset "Anonymous Functions" begin
f = x -> x+1
f2 = BSON.roundtrip(f)
@test f2(5) == f(5)
@test typeof(f2) !== typeof(f)

chicken_tikka_masala(y) = x -> x+y
f = chicken_tikka_masala(5)
f2 = BSON.roundtrip(f)
@test f2(6) == f(6)
@test typeof(f2) !== typeof(f)
if VERSION < v"1.7-"
f2 = BSON.roundtrip(f)
@test f2(5) == f(5)
@test typeof(f2) !== typeof(f)

chicken_tikka_masala(y) = x -> x+y
f = chicken_tikka_masala(5)
f2 = BSON.roundtrip(f)
@test f2(6) == f(6)
@test typeof(f2) !== typeof(f)
else
@test_throws ErrorException f2 = BSON.roundtrip(f)
end
end

@testset "Int Literals in Type Params #41" begin
Expand Down