Skip to content

Commit fb59b6d

Browse files
authored
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.
1 parent 338f494 commit fb59b6d

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
@@ -42,7 +42,7 @@ function _isself(ft::DataType)
4242
name = ftname.singletonname
4343
ftname.name === name && return false
4444
mod = parentmodule(ft)
45-
return invokelatest(isdefinedglobal, mod, name) && ft === typeof(invokelatest(getglobal, mod, name))
45+
return isdefinedglobal(mod, name) && ft === typeof(getglobal(mod, name))
4646
end
4747

4848
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
@@ -1477,10 +1477,14 @@ JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var, int allow_import) // u
14771477
} else {
14781478
jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age);
14791479
}
1480-
if (jl_bkind_is_some_guard(jl_binding_kind(bpart)))
1480+
enum jl_partition_kind kind = jl_binding_kind(bpart);
1481+
if (jl_bkind_is_some_guard(kind))
14811482
return 0;
1482-
if (jl_bkind_is_defined_constant(jl_binding_kind(bpart))) {
1483-
// N.B.: No backdated check for isdefined
1483+
if (jl_bkind_is_defined_constant(kind)) {
1484+
if (__unlikely(kind == PARTITION_KIND_BACKDATED_CONST)) {
1485+
return !(jl_current_task->ptls->in_pure_callback || jl_options.depwarn == JL_OPTIONS_DEPWARN_ERROR);
1486+
}
1487+
// N.B.: No backdated admonition for isdefined
14841488
return 1;
14851489
}
14861490
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)