From 2643484f5e5d8d6c01e2ad7561591f1a766981b0 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Mon, 28 Apr 2025 23:35:49 +0000 Subject: [PATCH] fix isconst definition/accessor issues with binding partitions The problem was that isconst was declaring true when it was actually a back-dated (e.g. mutable) value, which confuses the runtime (e.g. show_datatype) into emitting warnings that are not warranted. Needed an updated version of Documenter, which was trying to use `isconst` in a fixed (old) world to use with `invokelatest` calls to `names` and other reflection (e.g. `setglobal!`). --- src/codegen.cpp | 4 ++-- src/gf.c | 2 +- src/julia.h | 6 +++--- src/julia_internal.h | 4 ++++ src/method.c | 2 +- src/module.c | 36 ++++++++++++++++++------------------ src/rtutils.c | 4 ++-- src/toplevel.c | 2 +- test/syntax.jl | 2 +- 9 files changed, 33 insertions(+), 29 deletions(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index e5600115b3a7d..2a937fc626c0b 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -3198,7 +3198,7 @@ static jl_cgval_t emit_globalref(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t * if (!jl_get_binding_leaf_partitions_restriction_kind(bnd, &rkp, ctx.min_world, ctx.max_world)) { return emit_globalref_runtime(ctx, bnd, mod, name); } - if (jl_bkind_is_some_constant(rkp.kind) && rkp.kind != PARTITION_KIND_BACKDATED_CONST) { + if (jl_bkind_is_real_constant(rkp.kind) || rkp.kind == PARTITION_KIND_UNDEF_CONST) { if (rkp.maybe_depwarn) { Value *bp = julia_binding_gv(ctx, bnd); ctx.builder.CreateCall(prepare_call(jldepcheck_func), { bp }); @@ -3788,7 +3788,7 @@ static jl_cgval_t emit_isdefinedglobal(jl_codectx_t &ctx, jl_module_t *modu, jl_ jl_binding_t *bnd = allow_import ? jl_get_binding(modu, name) : jl_get_module_binding(modu, name, 0); struct restriction_kind_pair rkp = { NULL, NULL, PARTITION_KIND_GUARD, 0 }; if (allow_import && jl_get_binding_leaf_partitions_restriction_kind(bnd, &rkp, ctx.min_world, ctx.max_world)) { - if (jl_bkind_is_some_constant(rkp.kind) && rkp.restriction) + if (jl_bkind_is_real_constant(rkp.kind)) return mark_julia_const(ctx, jl_true); if (rkp.kind == PARTITION_KIND_GLOBAL) { Value *bp = julia_binding_gv(ctx, rkp.binding_if_global); diff --git a/src/gf.c b/src/gf.c index 483a18d13682c..436b4c84347dc 100644 --- a/src/gf.c +++ b/src/gf.c @@ -771,7 +771,7 @@ int foreach_mtable_in_module( if ((void*)b == jl_nothing) break; jl_sym_t *name = b->globalref->name; - jl_value_t *v = jl_get_binding_value_if_const(b); + jl_value_t *v = jl_get_latest_binding_value_if_const(b); if (v) { jl_value_t *uw = jl_unwrap_unionall(v); if (jl_is_datatype(uw)) { diff --git a/src/julia.h b/src/julia.h index f696fff21271f..87c07499f019a 100644 --- a/src/julia.h +++ b/src/julia.h @@ -1933,9 +1933,9 @@ JL_DLLEXPORT jl_sym_t *jl_tagged_gensym(const char *str, size_t len); JL_DLLEXPORT jl_sym_t *jl_get_root_symbol(void); JL_DLLEXPORT jl_value_t *jl_get_binding_value(jl_binding_t *b JL_PROPAGATES_ROOT); JL_DLLEXPORT jl_value_t *jl_get_binding_value_in_world(jl_binding_t *b JL_PROPAGATES_ROOT, size_t world); -JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_const(jl_binding_t *b JL_PROPAGATES_ROOT); -JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved_debug_only(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT; -JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_latest_resolved_and_const_debug_only(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT; +JL_DLLEXPORT jl_value_t *jl_get_latest_binding_value_if_const(jl_binding_t *b JL_PROPAGATES_ROOT); +JL_DLLEXPORT jl_value_t *jl_get_latest_binding_value_if_resolved_debug_only(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT; +JL_DLLEXPORT jl_value_t *jl_get_latest_binding_value_if_resolved_and_const_debug_only(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT; JL_DLLEXPORT jl_value_t *jl_declare_const_gf(jl_module_t *mod, jl_sym_t *name); JL_DLLEXPORT jl_method_t *jl_method_def(jl_svec_t *argdata, jl_methtable_t *mt, jl_code_info_t *f, jl_module_t *module); JL_DLLEXPORT jl_code_info_t *jl_code_for_staged(jl_method_instance_t *linfo, size_t world, jl_code_instance_t **cache); diff --git a/src/julia_internal.h b/src/julia_internal.h index 9436e19fb26b4..938e87935578c 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -987,6 +987,10 @@ STATIC_INLINE int jl_bkind_is_defined_constant(enum jl_partition_kind kind) JL_N return kind == PARTITION_KIND_IMPLICIT_CONST || kind == PARTITION_KIND_CONST || kind == PARTITION_KIND_CONST_IMPORT || kind == PARTITION_KIND_BACKDATED_CONST; } +STATIC_INLINE int jl_bkind_is_real_constant(enum jl_partition_kind kind) JL_NOTSAFEPOINT { + return kind == PARTITION_KIND_IMPLICIT_CONST || kind == PARTITION_KIND_CONST || kind == PARTITION_KIND_CONST_IMPORT; +} + JL_DLLEXPORT jl_binding_partition_t *jl_get_binding_partition(jl_binding_t *b JL_PROPAGATES_ROOT, size_t world) JL_GLOBALLY_ROOTED; JL_DLLEXPORT jl_binding_partition_t *jl_get_binding_partition_with_hint(jl_binding_t *b JL_PROPAGATES_ROOT, jl_binding_partition_t *previous_part, size_t world) JL_GLOBALLY_ROOTED; JL_DLLEXPORT jl_binding_partition_t *jl_get_binding_partition_all(jl_binding_t *b JL_PROPAGATES_ROOT, size_t min_world, size_t max_world) JL_GLOBALLY_ROOTED; diff --git a/src/method.c b/src/method.c index 675293089de44..f71f09ceb83bc 100644 --- a/src/method.c +++ b/src/method.c @@ -224,7 +224,7 @@ static jl_value_t *resolve_definition_effects(jl_value_t *expr, jl_module_t *mod jl_sym_t *fe_sym = jl_globalref_name(fe); // look at some known called functions jl_binding_t *b = jl_get_binding(fe_mod, fe_sym); - if (jl_get_binding_value_if_const(b) == BUILTIN(tuple)) { + if (jl_get_latest_binding_value_if_const(b) == BUILTIN(tuple)) { size_t j; for (j = 1; j < nargs; j++) { if (!jl_is_quotenode(jl_exprarg(e, j))) diff --git a/src/module.c b/src/module.c index 0271a645aa4ed..272748edbadb2 100644 --- a/src/module.c +++ b/src/module.c @@ -463,7 +463,7 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_leaf_partitions_value_if_const(jl_bindin struct restriction_kind_pair rkp = { NULL, NULL, PARTITION_KIND_GUARD, 0 }; if (!jl_get_binding_leaf_partitions_restriction_kind(b, &rkp, min_world, max_world)) return NULL; - if (jl_bkind_is_some_constant(rkp.kind) && rkp.kind != PARTITION_KIND_BACKDATED_CONST) { + if (jl_bkind_is_real_constant(rkp.kind)) { *maybe_depwarn = rkp.maybe_depwarn; return rkp.restriction; } @@ -581,7 +581,7 @@ JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val3( for (;;) { enum jl_partition_kind prev_kind = jl_binding_kind(prev_bpart); if (jl_bkind_is_some_constant(prev_kind) || prev_kind == PARTITION_KIND_GLOBAL || - (jl_bkind_is_some_import(prev_kind))) { + jl_bkind_is_some_import(prev_kind)) { need_backdate = 0; break; } @@ -923,22 +923,23 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_value_seqcst(jl_binding_t *b) return jl_atomic_load(&b->value); } -JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_const(jl_binding_t *b) +JL_DLLEXPORT jl_value_t *jl_get_latest_binding_value_if_const(jl_binding_t *b) { - jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age); + // See note below. Note that this is for some deprecated uses, and should not be added to new code. + size_t world = jl_atomic_load_relaxed(&jl_world_counter); + jl_binding_partition_t *bpart = jl_get_binding_partition(b, world); + jl_walk_binding_inplace(&b, &bpart, world); enum jl_partition_kind kind = jl_binding_kind(bpart); if (jl_bkind_is_some_guard(kind)) return NULL; - if (!jl_bkind_is_some_constant(kind)) + if (!jl_bkind_is_real_constant(kind)) return NULL; - check_backdated_binding(b, kind); return bpart->restriction; } -JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_latest_resolved_and_const_debug_only(jl_binding_t *b) +JL_DLLEXPORT jl_value_t *jl_get_latest_binding_value_if_resolved_and_const_debug_only(jl_binding_t *b) { - // Unlike jl_get_binding_value_if_const this doesn't try to allocate new binding partitions if they + // Unlike jl_get_latest_binding_value_if_const this doesn't try to allocate new binding partitions if they // don't already exist, making this JL_NOTSAFEPOINT. However, as a result, this may fail to return // a value - even if one does exist. It should only be used for reflection/debugging when the integrity // of the runtime is not guaranteed. @@ -948,18 +949,17 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_latest_resolved_and_const_debug if (!bpart) return NULL; size_t max_world = jl_atomic_load_relaxed(&bpart->max_world); - if (jl_atomic_load_relaxed(&bpart->min_world) > jl_current_task->world_age || jl_current_task->world_age > max_world) + if (max_world != ~(size_t)0) return NULL; enum jl_partition_kind kind = jl_binding_kind(bpart); if (jl_bkind_is_some_guard(kind)) return NULL; - if (!jl_bkind_is_some_constant(kind)) + if (!jl_bkind_is_real_constant(kind)) return NULL; - check_backdated_binding(b, kind); return bpart->restriction; } -JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved_debug_only(jl_binding_t *b) +JL_DLLEXPORT jl_value_t *jl_get_latest_binding_value_if_resolved_debug_only(jl_binding_t *b) { // See note above. Use for debug/reflection purposes only. if (!b) @@ -968,7 +968,7 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved_debug_only(jl_binding_ if (!bpart) return NULL; size_t max_world = jl_atomic_load_relaxed(&bpart->max_world); - if (jl_atomic_load_relaxed(&bpart->min_world) > jl_current_task->world_age || jl_current_task->world_age > max_world) + if (max_world != ~(size_t)0) return NULL; enum jl_partition_kind kind = jl_binding_kind(bpart); if (jl_bkind_is_some_guard(kind)) @@ -976,7 +976,6 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved_debug_only(jl_binding_ if (jl_bkind_is_some_import(kind)) return NULL; if (jl_bkind_is_some_constant(kind)) { - check_backdated_binding(b, kind); return bpart->restriction; } return jl_atomic_load_relaxed(&b->value); @@ -1011,6 +1010,7 @@ static jl_module_t *jl_binding_dbgmodule(jl_binding_t *b) // along the way. JL_DLLEXPORT jl_value_t *jl_get_existing_strong_gf(jl_binding_t *b, size_t new_world) { + assert(new_world > jl_atomic_load_relaxed(&jl_world_counter)); jl_binding_partition_t *bpart = jl_get_binding_partition(b, new_world); enum jl_partition_kind kind = jl_binding_kind(bpart); if (jl_bkind_is_some_constant(kind) && kind != PARTITION_KIND_IMPLICIT_CONST) @@ -1032,7 +1032,7 @@ JL_DLLEXPORT jl_value_t *jl_get_existing_strong_gf(jl_binding_t *b, size_t new_w check_safe_newbinding(b->globalref->mod, b->globalref->name); return NULL; } - jl_module_t *from = jl_binding_dbgmodule(b);\ + jl_module_t *from = jl_binding_dbgmodule(b); assert(from); // Can only be NULL if implicit, which we excluded above jl_errorf("invalid method definition in %s: exported function %s.%s does not exist", jl_module_debug_name(b->globalref->mod), jl_module_debug_name(from), jl_symbol_name(b->globalref->name)); @@ -1728,7 +1728,7 @@ JL_DLLEXPORT int jl_globalref_is_const(jl_globalref_t *gr) b = jl_get_module_binding(gr->mod, gr->name, 1); jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age); - return jl_bkind_is_some_constant(jl_binding_kind(bpart)); + return jl_bkind_is_real_constant(jl_binding_kind(bpart)); } JL_DLLEXPORT void jl_disable_binding(jl_globalref_t *gr) @@ -1757,7 +1757,7 @@ JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var) jl_binding_t *b = jl_get_binding(m, var); jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age); - return b && jl_bkind_is_some_constant(jl_binding_kind(bpart)); + return b && jl_bkind_is_real_constant(jl_binding_kind(bpart)); } // set the deprecated flag for a binding: diff --git a/src/rtutils.c b/src/rtutils.c index 5966497ec331c..4baf0ee5e6e9c 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -579,7 +579,7 @@ JL_DLLEXPORT jl_value_t *jl_stderr_obj(void) JL_NOTSAFEPOINT if (jl_base_module == NULL) return NULL; jl_binding_t *stderr_obj = jl_get_module_binding(jl_base_module, jl_symbol("stderr"), 0); - return stderr_obj ? jl_get_binding_value_if_resolved_debug_only(stderr_obj) : NULL; + return stderr_obj ? jl_get_latest_binding_value_if_resolved_debug_only(stderr_obj) : NULL; } // toys for debugging --------------------------------------------------------- @@ -674,7 +674,7 @@ static int is_globname_binding(jl_value_t *v, jl_datatype_t *dv) JL_NOTSAFEPOINT jl_sym_t *globname = dv->name->mt != NULL ? dv->name->mt->name : NULL; if (globname && dv->name->module) { jl_binding_t *b = jl_get_module_binding(dv->name->module, globname, 0); - jl_value_t *bv = jl_get_binding_value_if_latest_resolved_and_const_debug_only(b); + jl_value_t *bv = jl_get_latest_binding_value_if_resolved_and_const_debug_only(b); if (bv && ((jl_value_t*)dv == v ? jl_typeof(bv) == v : bv == v)) return 1; } diff --git a/src/toplevel.c b/src/toplevel.c index 826b9195da059..739eb8024ece0 100644 --- a/src/toplevel.c +++ b/src/toplevel.c @@ -416,7 +416,7 @@ static void expr_attributes(jl_value_t *v, jl_array_t *body, int *has_ccall, int jl_module_t *mod = jl_globalref_mod(f); jl_sym_t *name = jl_globalref_name(f); jl_binding_t *b = jl_get_binding(mod, name); - called = jl_get_binding_value_if_const(b); + called = jl_get_latest_binding_value_if_const(b); } else if (jl_is_quotenode(f)) { called = jl_quotenode_value(f); diff --git a/test/syntax.jl b/test/syntax.jl index 27abea4c57a66..2321d8cf9266f 100644 --- a/test/syntax.jl +++ b/test/syntax.jl @@ -1939,7 +1939,7 @@ end # eval'ing :const exprs eval(Expr(:const, :_var_30877)) @test !isdefined(@__MODULE__, :_var_30877) -@test isconst(@__MODULE__, :_var_30877) +@test !isconst(@__MODULE__, :_var_30877) # anonymous kw function in value position at top level f30926 = function (;k=0)