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.

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

Broken with Julia master #428

Closed
eschnett opened this issue Oct 31, 2022 · 4 comments · Fixed by #438
Closed

Broken with Julia master #428

eschnett opened this issue Oct 31, 2022 · 4 comments · Fixed by #438

Comments

@eschnett
Copy link
Contributor

The current master version of JLD2 fails on the current master version of Julia:

$ julia +dev
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.9.0-DEV.1696 (2022-10-31)
 _/ |\__'_|_|_|\__'_|  |  Commit f70b5e4767* (0 days old master)
|__/                   |

julia> using JLD2
[ Info: Precompiling JLD2 [033835bb-8acc-5ee8-8aae-3f567f8a3819]
ERROR: LoadError: type DataType has no field size
Stacktrace:
 [1] getproperty(x::Type, f::Symbol)
   @ Base ./Base.jl:32
 [2] top-level scope
   @ ~/.julia/dev/JLD2/src/data/number_types.jl:14
 [3] include(mod::Module, _path::String)
   @ Base ./Base.jl:450
 [4] include(x::String)
   @ JLD2 ~/.julia/dev/JLD2/src/JLD2.jl:1
 [5] top-level scope
   @ ~/.julia/dev/JLD2/src/JLD2.jl:577
 [6] include
   @ ./Base.jl:450 [inlined]
 [7] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt64}}, source::Nothing)
   @ Base ./loading.jl:1667
 [8] top-level scope
   @ stdin:1
in expression starting at /Users/eschnett/.julia/dev/JLD2/src/data/number_types.jl:13
in expression starting at /Users/eschnett/.julia/dev/JLD2/src/JLD2.jl:1
in expression starting at stdin:1
ERROR: Failed to precompile JLD2 [033835bb-8acc-5ee8-8aae-3f567f8a3819] to /Users/eschnett/.julia/compiled/v1.9/JLD2/jl_1Zc39q.
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] compilecache(pkg::Base.PkgId, path::String, internal_stderr::IO, internal_stdout::IO, keep_loaded_modules::Bool)
   @ Base ./loading.jl:1820
 [3] compilecache
   @ ./loading.jl:1764 [inlined]
 [4] _require(pkg::Base.PkgId, env::String)
   @ Base ./loading.jl:1431
 [5] _require_prelocked(uuidkey::Base.PkgId, env::String)
   @ Base ./loading.jl:1295
 [6] macro expansion
   @ ./loading.jl:1275 [inlined]
 [7] macro expansion
   @ ./lock.jl:267 [inlined]
 [8] require(into::Module, mod::Symbol)
   @ Base ./loading.jl:1238
@torrance
Copy link

torrance commented Nov 2, 2022

I am reproducing this error too.

@JonasIsensee
Copy link
Collaborator

This should be fixable relatively easily.
Internals of the julia DataType have changed a few times in recent releases.

  1. find PR to base julia to figure out what changed
  2. replace all code patters T.size in JLD2 with a version-dependent accessor function
    For reference: fix #327 DataType has no field ninitialized #331

@JonasIsensee
Copy link
Collaborator

Appears to be because of JuliaLang/julia#47170

but replacing with Core.sizeof doesn't do the trick since:

julia> Core.sizeof(Matrix{Float64})
ERROR: Type Array does not have a definite size.
Stacktrace:
 [1] top-level scope
   @ REPL[13]:1

julia> Matrix{Float64}.size
0

@eschnett
Copy link
Contributor Author

I think we need this function:

# Modelled after Base.datatype_alignment
function datatype_size(dt::DataType)
    Base.@_foldable_meta
    dt.layout == C_NULL && throw(UndefRefError())
    size = unsafe_load(convert(Ptr{Base.DataTypeLayout}, dt.layout)).size
    return Int(size)
end

PR in progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants