Fix clear_module! world semantics#2693
Conversation
fingolfin
left a comment
fingolfin
left a comment
There was a problem hiding this comment.
Looks good to me, thank you!
| catch err | ||
| @debug "Could not clear variable `$name` by assigning `nothing`" err | ||
| end | ||
| VERSION >= v"1.12" && Base.delete_binding(M, name) |
There was a problem hiding this comment.
If there is a chance for this to be slow, I'd definitely would want to check this in the Oscar manual were a slowdown in this function caused us major pain before.
|
Hmm, fails Julia 1.6 tests (finalizer of an object in the cleared module is not called). (and of course a changelog entry is missing) |
|
Building Oscar.jl docs with Documenter.jl master ...
So it seems Julia 1.12 is getting slightly slower than 1.11, which is a bunch slower than 1.11 (unfortunately that's a general pattern I am seeing and a reason why I do my daily work with Julia 1.10 still). On the bright side, |
|
@vtjnash would you mind adding a changelog entry? |
mortenpi
left a comment
mortenpi
left a comment
There was a problem hiding this comment.
Thank you!
If this is a blocking bug somewhere (Base manual?) then we could get this out in a backport to 1.10 quickly. Otherwise, there are a few more PRs we might want to get into Documenter 1.11.0.
|
Anytime in a week or two is fine and will hold the bugfix upstream. Not urgent |
3 participants
The
isconstis a world-specific query, so it needsinvokelatesttoo. Restructure a bit of the code to do that in a helper function. Also add a call to delete all bindings in v1.12+. Although that may be expensive and currently warns that it may be incorrect, it may be helpful to get testing of that.