-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Many packages fail with UndefVarError
now
#57328
Comments
We could consider extending the backdating mechanism to more cases, but I'm not sure the two linked instances are good examples. The former does need to explicitly be updated because it deals with world ages. The latter example is just confused about the scope of those variables. The more backdating we do, the more problematic the semantics become. |
I just took two examples, maybe they were bad, there are 150 more to chose from. |
Yeah, we'll have to go through them and see if there's some common themes that are useful to mitigate. |
Quite a few of these are actually due to
which Is a completely separate JLD2 error. |
This is just unworkable I guess:
|
This package seems to export |
I think that can just use I can transform (julia 1.11.3) julia> module FooModule
function foo()
eval(:(bar = 3))
return bar
end
end
Main.FooModule
julia> FooModule.foo()
3 to (julia nightly) julia> module FooModule
function foo()
eval(:(bar = 3))
return invokelatest(getproperty, FooModule, :bar)
end
end
Main.FooModule
julia> FooModule.foo()
3 |
Ambiguous bindings are no longer disambiguated by resolvedness, so it's possible to get more ambiguities (the UndefVarError should tell you that's it's ambiguous, though making the error message better is on my TODO list). That said, it's supposed to ignore bindings with undefined values in the lookup set. It's possible that's broken. |
Yes, and/or possibly the package author intended that variable to be |
Wasn't that fixed already in the package? Are these packages using an old version of JLD2? |
Yeah, it seems many were stuck on 0.4. We'll see if JuliaIO/JLD2.jl#631 helps at the next PkgEval run. |
Here is a MWE of a failure in GeoParams (put this in a package) and do something like module GeoParams
module A
module B
using GeoParams
export S
struct S end
module C
using GeoParams
h() = S() # commenting this line: ok
x -> nothing # changing this to a non-anon function: ok
end
end
end
using .A.B
export S # moving this export up above module A: ok
end The fact that the anonymous function is needed is quite wild. |
Here is another MWE for (#57328 (comment)) here for https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/7825364_vs_d63aded/BrillouinZoneMeshes.primary.log: module M
module A
export S
end
using .A
module B
abstract type S end
export S
end
using .B
S
end During package precompilation you don't really get a good error message but pasting into the REPL you see
(And no, I don't know why people write their code like this...) |
The Zygote error is basically (need to comment out the julia> using Zygote
# 1.12
julia> code_warntype(Zygote._pullback, Tuple{Zygote.Context{false}, typeof(>), Int64, Int64})
MethodInstance for ZygoteRules._pullback(::Zygote.Context{false}, ::typeof(>), ::Int64, ::Int64)
from _pullback(ctx::ZygoteRules.AContext, f, args...) @ Zygote ~/PkgEvalAnalysis/JuliaPkgs/Zygote.jl/src/compiler/interface2.jl:101
Arguments
methodinstance::Core.Const(ZygoteRules._pullback)
ctx::Zygote.Context{false}
f::Core.Const(>)
args::Tuple{Int64, Int64}
Body::Any
1 ─ %1 = Base.chain_rrule::Any # <----------------------------- Base module
...
# 1.11
julia> code_warntype(Zygote._pullback, Tuple{Zygote.Context{false}, typeof(>), Int64, Int64})
MethodInstance for ZygoteRules._pullback(::Zygote.Context{false}, ::typeof(>), ::Int64, ::Int64)
...
Body::Tuple{Bool, Zygote.ZBack{Returns{Tuple{ChainRulesCore.NoTangent, ChainRulesCore.NoTangent, ChainRulesCore.NoTangent}}}}
1 ─ %1 = Zygote.chain_rrule::Core.Const(Zygote.chain_rrule) # <---------------------------- Zygote module
... where there is a disagreement over what module the which is then used as input into the I don't really know what determines what module this should be "expanded" into (probably something with the |
https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/7825364_vs_d63aded/report.html
With the new binding rules there are many patterns used in the wild that now causes UndefVarError. The amount of breakage is a bit too high in my opinion so it would be good to see if some mechanisms could be created that would soften this breakage. Some patterns:
using
+ trying to access variable: https://github.com/JuliaLang/PrecompileTools.jl/blob/61138b566718652a4033f528ff2dcabcf4cff2cb/test/runtests.jl#L177-L180@eval
global variable then try to access: https://github.com/matsueushi/RoundingEmulator.jl/blob/4ac8f5ee7103fdbbd0d57e160b9218bf9de559be/test/setrounding_raw.jl#L23-L37~ 150 packages fail with some
Zygote
issue:The set of packages that fail with `UndefVarError`
Packages that fail with Zygote undefvar error:
The text was updated successfully, but these errors were encountered: