JuliaIO / JLD2.jl

HDF5-compatible file format in pure Julia
Other
547 stars 85 forks source link

Broken with Julia master #428

Closed eschnett closed 1 year ago

eschnett commented 1 year ago

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 commented 1 year ago

I am reproducing this error too.

JonasIsensee commented 1 year ago

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: #331

JonasIsensee commented 1 year ago

Appears to be because of https://github.com/JuliaLang/julia/pull/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 commented 1 year ago

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.