Skip to content

Commit 5a27fdc

Browse files
KenoKristofferC
authored andcommitted
bpart: Tweak isdefinedglobal on backdated constant (#58976)
In d2cc061 and prior commits, we made backdated access a conditional error (if depwarns are enabled or in generators). However, we did not touch `isdefinedglobal`. This resulted in the common pattern `isdefinedglobal(m, s) && getglobal(m, s)` to sometimes error. In particular, this could be observed when attempting to print a type from inside a generated function before that type's definition age. Additionally, I think the usage there, which used `invokelatest` on each of the two queries is problematic because it is racy, since the two `invokelatest` calls may be looking at different world ages. This makes two tweaks: 1. Makes `isdefinedglobal` consistent with `getglobal` in that it now returns false if `getglobal` would throw due to the above referenced restriction. 2. Removes the implicit `invokelatest` in _isself in the show code. Instead, it will use the current world. I considered having it use the exception age when used for MethodErrors. However, because this is used for printing it matters more how the object can be accessed *now* rather than how it could have been accessed in the past. (cherry picked from commit fb59b6d)
1 parent 75330d4 commit 5a27fdc

File tree

3 files changed

+15
-5
lines changed

3 files changed

+15
-5
lines changed

base/show.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ function _isself(ft::DataType)
3535
name = ftname.singletonname
3636
ftname.name === name && return false
3737
mod = parentmodule(ft)
38-
return invokelatest(isdefinedglobal, mod, name) && ft === typeof(invokelatest(getglobal, mod, name))
38+
return isdefinedglobal(mod, name) && ft === typeof(getglobal(mod, name))
3939
end
4040

4141
function show(io::IO, ::MIME"text/plain", f::Function)

src/module.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,10 +1474,14 @@ JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var, int allow_import) // u
14741474
} else {
14751475
jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age);
14761476
}
1477-
if (jl_bkind_is_some_guard(jl_binding_kind(bpart)))
1477+
enum jl_partition_kind kind = jl_binding_kind(bpart);
1478+
if (jl_bkind_is_some_guard(kind))
14781479
return 0;
1479-
if (jl_bkind_is_defined_constant(jl_binding_kind(bpart))) {
1480-
// N.B.: No backdated check for isdefined
1480+
if (jl_bkind_is_defined_constant(kind)) {
1481+
if (__unlikely(kind == PARTITION_KIND_BACKDATED_CONST)) {
1482+
return !(jl_current_task->ptls->in_pure_callback || jl_options.depwarn == JL_OPTIONS_DEPWARN_ERROR);
1483+
}
1484+
// N.B.: No backdated admonition for isdefined
14811485
return 1;
14821486
}
14831487
return jl_atomic_load(&b->value) != NULL;

test/worlds.jl

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,13 @@ struct FooBackdated
557557

558558
FooBackdated() = new(FooBackdated[])
559559
end
560-
@test Base.invoke_in_world(before_backdate_age, isdefined, @__MODULE__, :FooBackdated)
560+
# For depwarn == 1, this throws a warning on access, for depwarn == 2, it throws an error.
561+
# `isdefinedglobal` changes with that, but doesn't error.
562+
if Base.JLOptions().depwarn <= 1
563+
@test Base.invoke_in_world(before_backdate_age, isdefinedglobal, @__MODULE__, :FooBackdated)
564+
else
565+
@test !Base.invoke_in_world(before_backdate_age, isdefinedglobal, @__MODULE__, :FooBackdated)
566+
end
561567

562568
# Test that ambiguous binding intersect the using'd binding's world ranges
563569
module AmbigWorldTest

0 commit comments

Comments
 (0)