From 208b7df18bee21c0b1e845f49e8d8bfdec61f6e1 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 13 Feb 2025 23:03:05 +0000 Subject: [PATCH] bpart: Also partition the export flag Whether or not a binding is exported affects the binding resolution of any downstream modules that `using` the module that defines the binding. As such, it needs to fully participate in the invalidation mechanism, so that code which references bindings whose resolution may have changed get properly invalidated. To do this, move the `exportp` flag from Binding into a separate bitflag set that gets or'd into the BindingPartition `->kind` field. Note that we do not move `publicp` in the same way since it does not affect binding resolution. There is currently no mechanism to un-export a binding, although the system is set up to support this in the future (and Revise may want it). That said, at such a time, we may need to revisit the above decision on `publicp`. Lastly, I will note that this adds a fair number of additional invalidations. Most of these are unnecessary, as changing an export only affects the *downstream* users not the binding itself. I am planning to tackle this as part of a larger future PR that avoids invalidation when this is not required. Fixes #57377 --- base/invalidation.jl | 51 +++++++------- base/runtime_internals.jl | 2 + base/show.jl | 3 + src/ast.c | 2 +- src/codegen.cpp | 18 ++--- src/julia.h | 21 +++--- src/julia_internal.h | 8 ++- src/method.c | 4 +- src/module.c | 140 ++++++++++++++++++++++++-------------- src/staticdata.c | 4 +- src/toplevel.c | 8 +-- test/rebinding.jl | 25 ++++++- 12 files changed, 176 insertions(+), 110 deletions(-) diff --git a/base/invalidation.jl b/base/invalidation.jl index c0aed35aa90a0..462e348e09038 100644 --- a/base/invalidation.jl +++ b/base/invalidation.jl @@ -113,39 +113,36 @@ function invalidate_method_for_globalref!(gr::GlobalRef, method::Method, invalid end end -const BINDING_FLAG_EXPORTP = 0x2 - function invalidate_code_for_globalref!(b::Core.Binding, invalidated_bpart::Core.BindingPartition, new_bpart::Union{Core.BindingPartition, Nothing}, new_max_world::UInt) gr = b.globalref - if is_some_guard(binding_kind(invalidated_bpart)) + if !is_some_guard(binding_kind(invalidated_bpart)) # TODO: We may want to invalidate for these anyway, since they have performance implications - return - end - foreach_module_mtable(gr.mod, new_max_world) do mt::Core.MethodTable - for method in MethodList(mt) - invalidate_method_for_globalref!(gr, method, invalidated_bpart, new_max_world) + foreach_module_mtable(gr.mod, new_max_world) do mt::Core.MethodTable + for method in MethodList(mt) + invalidate_method_for_globalref!(gr, method, invalidated_bpart, new_max_world) + end + return true end - return true - end - if isdefined(b, :backedges) - for edge in b.backedges - if isa(edge, CodeInstance) - ccall(:jl_invalidate_code_instance, Cvoid, (Any, UInt), edge, new_max_world) - elseif isa(edge, Core.Binding) - isdefined(edge, :partitions) || continue - latest_bpart = edge.partitions - latest_bpart.max_world == typemax(UInt) || continue - is_some_imported(binding_kind(latest_bpart)) || continue - partition_restriction(latest_bpart) === b || continue - invalidate_code_for_globalref!(edge, latest_bpart, nothing, new_max_world) - else - invalidate_method_for_globalref!(gr, edge::Method, invalidated_bpart, new_max_world) + if isdefined(b, :backedges) + for edge in b.backedges + if isa(edge, CodeInstance) + ccall(:jl_invalidate_code_instance, Cvoid, (Any, UInt), edge, new_max_world) + elseif isa(edge, Core.Binding) + isdefined(edge, :partitions) || continue + latest_bpart = edge.partitions + latest_bpart.max_world == typemax(UInt) || continue + is_some_imported(binding_kind(latest_bpart)) || continue + partition_restriction(latest_bpart) === b || continue + invalidate_code_for_globalref!(edge, latest_bpart, nothing, new_max_world) + else + invalidate_method_for_globalref!(gr, edge::Method, invalidated_bpart, new_max_world) + end end end end - if (b.flags & BINDING_FLAG_EXPORTP) != 0 + if (invalidated_bpart.kind & BINDING_FLAG_EXPORTED != 0) || (new_bpart !== nothing && (new_bpart.kind & BINDING_FLAG_EXPORTED != 0)) # This binding was exported - we need to check all modules that `using` us to see if they - # have an implicit binding to us. + # have a binding that is affected by this change. usings_backedges = ccall(:jl_get_module_usings_backedges, Any, (Any,), gr.mod) if usings_backedges !== nothing for user in usings_backedges::Vector{Any} @@ -154,8 +151,8 @@ function invalidate_code_for_globalref!(b::Core.Binding, invalidated_bpart::Core isdefined(user_binding, :partitions) || continue latest_bpart = user_binding.partitions latest_bpart.max_world == typemax(UInt) || continue - is_some_imported(binding_kind(latest_bpart)) || continue - partition_restriction(latest_bpart) === b || continue + binding_kind(latest_bpart) in (BINDING_KIND_IMPLICIT, BINDING_KIND_FAILED, BINDING_KIND_GUARD) || continue + @atomic :release latest_bpart.max_world = new_max_world invalidate_code_for_globalref!(convert(Core.Binding, user_binding), latest_bpart, nothing, new_max_world) end end diff --git a/base/runtime_internals.jl b/base/runtime_internals.jl index 36961f58c5c3f..3a66dbda97477 100644 --- a/base/runtime_internals.jl +++ b/base/runtime_internals.jl @@ -209,6 +209,8 @@ const BINDING_KIND_GUARD = 0x8 const BINDING_KIND_UNDEF_CONST = 0x9 const BINDING_KIND_BACKDATED_CONST = 0xa +const BINDING_FLAG_EXPORTED = 0x10 + is_defined_const_binding(kind::UInt8) = (kind == BINDING_KIND_CONST || kind == BINDING_KIND_CONST_IMPORT || kind == BINDING_KIND_BACKDATED_CONST) is_some_const_binding(kind::UInt8) = (is_defined_const_binding(kind) || kind == BINDING_KIND_UNDEF_CONST) is_some_imported(kind::UInt8) = (kind == BINDING_KIND_IMPLICIT || kind == BINDING_KIND_EXPLICIT || kind == BINDING_KIND_IMPORTED) diff --git a/base/show.jl b/base/show.jl index 09b232e54ade6..a0bf44166249e 100644 --- a/base/show.jl +++ b/base/show.jl @@ -3374,6 +3374,9 @@ function print_partition(io::IO, partition::Core.BindingPartition) else print(io, max_world) end + if (partition.kind & BINDING_FLAG_EXPORTED) != 0 + print(io, " [exported]") + end print(io, " - ") kind = binding_kind(partition) if kind == BINDING_KIND_BACKDATED_CONST diff --git a/src/ast.c b/src/ast.c index f643f4a2f40fe..103a52c64253e 100644 --- a/src/ast.c +++ b/src/ast.c @@ -178,7 +178,7 @@ static value_t fl_defined_julia_global(fl_context_t *fl_ctx, value_t *args, uint jl_sym_t *var = scmsym_to_julia(fl_ctx, args[0]); jl_binding_t *b = jl_get_module_binding(ctx->module, var, 0); jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - return (bpart != NULL && bpart->kind == BINDING_KIND_GLOBAL) ? fl_ctx->T : fl_ctx->F; + return (bpart != NULL && jl_binding_kind(bpart) == BINDING_KIND_GLOBAL) ? fl_ctx->T : fl_ctx->F; } // Used to generate a unique suffix for a given symbol (e.g. variable or type name) diff --git a/src/codegen.cpp b/src/codegen.cpp index 26f34d63e578f..7519a6a080415 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -3114,7 +3114,7 @@ static jl_value_t *static_eval(jl_codectx_t &ctx, jl_value_t *ex) jl_binding_t *bnd = jl_get_module_binding(ctx.module, sym, 0); jl_binding_partition_t *bpart = jl_get_binding_partition_all(bnd, ctx.min_world, ctx.max_world); jl_walk_binding_inplace_all(&bnd, &bpart, ctx.min_world, ctx.max_world); - if (bpart && jl_bkind_is_some_constant(bpart->kind)) + if (bpart && jl_bkind_is_some_constant(jl_binding_kind(bpart))) return bpart->restriction; return NULL; } @@ -3140,7 +3140,7 @@ static jl_value_t *static_eval(jl_codectx_t &ctx, jl_value_t *ex) jl_binding_partition_t *bpart = jl_get_binding_partition_all(bnd, ctx.min_world, ctx.max_world); jl_walk_binding_inplace_all(&bnd, &bpart, ctx.min_world, ctx.max_world); jl_value_t *v = NULL; - if (bpart && jl_bkind_is_some_constant(bpart->kind)) + if (bpart && jl_bkind_is_some_constant(jl_binding_kind(bpart))) v = bpart->restriction; if (v) { if (bnd->deprecated) @@ -3167,7 +3167,7 @@ static jl_value_t *static_eval(jl_codectx_t &ctx, jl_value_t *ex) jl_binding_partition_t *bpart = jl_get_binding_partition_all(bnd, ctx.min_world, ctx.max_world); jl_walk_binding_inplace_all(&bnd, &bpart, ctx.min_world, ctx.max_world); jl_value_t *v = NULL; - if (bpart && jl_bkind_is_some_constant(bpart->kind)) + if (bpart && jl_bkind_is_some_constant(jl_binding_kind(bpart))) v = bpart->restriction; if (v) { if (bnd->deprecated) @@ -3419,14 +3419,14 @@ static jl_cgval_t emit_globalref(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t * return emit_globalref_runtime(ctx, bnd, mod, name); } // bpart was updated in place - this will change with full partition - if (jl_bkind_is_some_guard(bpart->kind)) { + if (jl_bkind_is_some_guard(jl_binding_kind(bpart))) { // Redo the lookup at runtime return emit_globalref_runtime(ctx, bnd, mod, name); } else { while (true) { if (!bpart) break; - if (!jl_bkind_is_some_import(bpart->kind)) + if (!jl_bkind_is_some_import(jl_binding_kind(bpart))) break; if (bnd->deprecated) { cg_bdw(ctx, name, bnd); @@ -3437,7 +3437,7 @@ static jl_cgval_t emit_globalref(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t * break; } if (bpart) { - enum jl_partition_kind kind = bpart->kind; + enum jl_partition_kind kind = jl_binding_kind(bpart); if (jl_bkind_is_some_constant(kind) && kind != BINDING_KIND_BACKDATED_CONST) { jl_value_t *constval = bpart->restriction; if (!constval) { @@ -3448,7 +3448,7 @@ static jl_cgval_t emit_globalref(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t * } } } - if (!bpart || bpart->kind != BINDING_KIND_GLOBAL) { + if (!bpart || jl_binding_kind(bpart) != BINDING_KIND_GLOBAL) { return emit_globalref_runtime(ctx, bnd, mod, name); } Value *bp = julia_binding_gv(ctx, bnd); @@ -3471,7 +3471,7 @@ static jl_cgval_t emit_globalop(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *s jl_binding_partition_t *bpart = jl_get_binding_partition_all(bnd, ctx.min_world, ctx.max_world); Value *bp = julia_binding_gv(ctx, bnd); if (bpart) { - if (bpart->kind == BINDING_KIND_GLOBAL) { + if (jl_binding_kind(bpart) == BINDING_KIND_GLOBAL) { jl_value_t *ty = bpart->restriction; if (ty != nullptr) { const std::string fname = issetglobal ? "setglobal!" : isreplaceglobal ? "replaceglobal!" : isswapglobal ? "swapglobal!" : ismodifyglobal ? "modifyglobal!" : "setglobalonce!"; @@ -4157,7 +4157,7 @@ static jl_cgval_t emit_isdefinedglobal(jl_codectx_t &ctx, jl_module_t *modu, jl_ Value *isnull = NULL; jl_binding_t *bnd = allow_import ? jl_get_binding(modu, name) : jl_get_module_binding(modu, name, 0); jl_binding_partition_t *bpart = jl_get_binding_partition_all(bnd, ctx.min_world, ctx.max_world); - enum jl_partition_kind kind = bpart ? bpart->kind : BINDING_KIND_GUARD; + enum jl_partition_kind kind = bpart ? jl_binding_kind(bpart) : BINDING_KIND_GUARD; if (kind == BINDING_KIND_GLOBAL || jl_bkind_is_some_constant(kind)) { if (jl_get_binding_value_if_const(bnd)) return mark_julia_const(ctx, jl_true); diff --git a/src/julia.h b/src/julia.h index bf049c909d833..046668527d4ef 100644 --- a/src/julia.h +++ b/src/julia.h @@ -706,6 +706,9 @@ enum jl_partition_kind { BINDING_KIND_IMPLICIT_RECOMPUTE = 0xb }; +// These are flags that get anded into the above +static const uint8_t BINDING_FLAG_EXPORTED = 0x10; + typedef struct __attribute__((aligned(8))) _jl_binding_partition_t { JL_DATA_TYPE /* union { @@ -716,18 +719,19 @@ typedef struct __attribute__((aligned(8))) _jl_binding_partition_t { * // For ->kind in (BINDING_KIND_IMPLICIT, BINDING_KIND_EXPLICIT, BINDING_KIND_IMPORT) * jl_binding_t *imported; * } restriction; - * - * Currently: Low 3 bits hold ->kind on _P64 to avoid needing >8 byte atomics - * - * This field is updated atomically with both kind and restriction */ jl_value_t *restriction; size_t min_world; _Atomic(size_t) max_world; _Atomic(struct _jl_binding_partition_t *) next; - enum jl_partition_kind kind; + size_t kind; } jl_binding_partition_t; +STATIC_INLINE enum jl_partition_kind jl_binding_kind(jl_binding_partition_t *bpart) +{ + return (enum jl_partition_kind)(bpart->kind & 0xf); +} + typedef struct _jl_binding_t { JL_DATA_TYPE jl_globalref_t *globalref; // cached GlobalRef for this binding @@ -735,11 +739,10 @@ typedef struct _jl_binding_t { _Atomic(jl_binding_partition_t*) partitions; jl_array_t *backedges; uint8_t did_print_backdate_admonition:1; - uint8_t exportp:1; // `public foo` sets `publicp`, `export foo` sets both `publicp` and `exportp` - uint8_t publicp:1; // exportp without publicp is not allowed. - uint8_t deprecated:2; // 0=not deprecated, 1=renamed, 2=moved to another package uint8_t did_print_implicit_import_admonition:1; - uint8_t padding:2; + uint8_t publicp:1; // `export` is tracked in partitions, but sets this as well + uint8_t deprecated:2; // 0=not deprecated, 1=renamed, 2=moved to another package + uint8_t padding:3; } jl_binding_t; typedef struct { diff --git a/src/julia_internal.h b/src/julia_internal.h index 0f52f82ac8230..34ab312c64ca7 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -915,6 +915,8 @@ JL_DLLEXPORT jl_binding_t *jl_get_module_binding(jl_module_t *m JL_PROPAGATES_RO JL_DLLEXPORT void jl_binding_deprecation_warning(jl_module_t *m, jl_sym_t *sym, jl_binding_t *b); JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked(jl_binding_t *b JL_PROPAGATES_ROOT, jl_binding_partition_t *old_bpart, jl_value_t *restriction_val, enum jl_partition_kind kind, size_t new_world) JL_GLOBALLY_ROOTED; +JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked2(jl_binding_t *b JL_PROPAGATES_ROOT, + jl_binding_partition_t *old_bpart, jl_value_t *restriction_val, size_t kind, size_t new_world) JL_GLOBALLY_ROOTED; extern jl_array_t *jl_module_init_order JL_GLOBALLY_ROOTED; extern htable_t jl_current_modules JL_GLOBALLY_ROOTED; extern JL_DLLEXPORT jl_module_t *jl_precompile_toplevel_module JL_GLOBALLY_ROOTED; @@ -952,7 +954,7 @@ JL_DLLEXPORT jl_binding_partition_t *jl_get_binding_partition(jl_binding_t *b JL 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; EXTERN_INLINE_DECLARE uint8_t jl_bpart_get_kind(jl_binding_partition_t *bpart) JL_NOTSAFEPOINT { - return (uint8_t)bpart->kind; + return (uint8_t)(bpart->kind & 0xf); } STATIC_INLINE void jl_walk_binding_inplace(jl_binding_t **bnd, jl_binding_partition_t **bpart JL_PROPAGATES_ROOT, size_t world) JL_NOTSAFEPOINT; @@ -962,7 +964,7 @@ STATIC_INLINE void jl_walk_binding_inplace_all(jl_binding_t **bnd, jl_binding_pa STATIC_INLINE void jl_walk_binding_inplace(jl_binding_t **bnd, jl_binding_partition_t **bpart, size_t world) JL_NOTSAFEPOINT { while (1) { - if (!jl_bkind_is_some_import((*bpart)->kind)) + if (!jl_bkind_is_some_import(jl_binding_kind(*bpart))) return; *bnd = (jl_binding_t*)(*bpart)->restriction; *bpart = jl_get_binding_partition(*bnd, world); @@ -974,7 +976,7 @@ STATIC_INLINE void jl_walk_binding_inplace_all(jl_binding_t **bnd, jl_binding_pa while (1) { if (!(*bpart)) return; - if (!jl_bkind_is_some_import((*bpart)->kind)) + if (!jl_bkind_is_some_import(jl_binding_kind(*bpart))) return; *bnd = (jl_binding_t*)(*bpart)->restriction; *bpart = jl_get_binding_partition_all(*bnd, min_world, max_world); diff --git a/src/method.c b/src/method.c index 0fc0e0ca1f87d..c4f3635713edd 100644 --- a/src/method.c +++ b/src/method.c @@ -1057,10 +1057,10 @@ JL_DLLEXPORT jl_value_t *jl_declare_const_gf(jl_module_t *mod, jl_sym_t *name) jl_binding_t *b = jl_get_binding_for_method_def(mod, name, new_world); jl_binding_partition_t *bpart = jl_get_binding_partition(b, new_world); jl_value_t *gf = NULL; - enum jl_partition_kind kind = bpart->kind; + enum jl_partition_kind kind = jl_binding_kind(bpart); if (!jl_bkind_is_some_guard(kind) && kind != BINDING_KIND_DECLARED && kind != BINDING_KIND_IMPLICIT) { jl_walk_binding_inplace(&b, &bpart, new_world); - if (jl_bkind_is_some_constant(bpart->kind)) { + if (jl_bkind_is_some_constant(jl_binding_kind(bpart))) { gf = bpart->restriction; JL_GC_PROMISE_ROOTED(gf); jl_check_gf(gf, b->globalref->name); diff --git a/src/module.c b/src/module.c index 8adc99a5380d5..3dd55f3e5e48b 100644 --- a/src/module.c +++ b/src/module.c @@ -19,7 +19,7 @@ static jl_binding_partition_t *new_binding_partition(void) { jl_binding_partition_t *bpart = (jl_binding_partition_t*)jl_gc_alloc(jl_current_task->ptls, sizeof(jl_binding_partition_t), jl_binding_partition_type); bpart->restriction = NULL; - bpart->kind = BINDING_KIND_GUARD; + bpart->kind = (size_t)BINDING_KIND_GUARD; bpart->min_world = 0; jl_atomic_store_relaxed(&bpart->max_world, (size_t)-1); jl_atomic_store_relaxed(&bpart->next, NULL); @@ -37,8 +37,8 @@ static int eq_bindings(jl_binding_partition_t *owner, jl_binding_t *alias, size_ return 1; jl_walk_binding_inplace(&ownerb, &owner, world); jl_walk_binding_inplace(&alias, &alias_bpart, world); - if (jl_bkind_is_some_constant(owner->kind) && - jl_bkind_is_some_constant(alias_bpart->kind) && + if (jl_bkind_is_some_constant(jl_binding_kind(owner)) && + jl_bkind_is_some_constant(jl_binding_kind(alias_bpart)) && owner->restriction && alias_bpart->restriction == owner->restriction) return 1; @@ -90,7 +90,7 @@ void jl_check_new_binding_implicit( jl_module_t *imp = data.mod; JL_GC_PROMISE_ROOTED(imp); jl_binding_t *tempb = jl_get_module_binding(imp, var, 0); - if (tempb != NULL && tempb->exportp) { + if (tempb != NULL) { if (data.min_world > min_world) min_world = data.min_world; if (data.max_world < min_world) @@ -105,6 +105,9 @@ void jl_check_new_binding_implicit( if (tempbmax_world < max_world) max_world = tempbmax_world; + if ((tempbpart->kind & BINDING_FLAG_EXPORTED) == 0) + continue; + if (impb) { if (tempb->deprecated) continue; @@ -179,6 +182,8 @@ STATIC_INLINE jl_binding_partition_t *jl_get_binding_partition_(jl_binding_t *b jl_atomic_store_relaxed(&new_bpart->max_world, max_world); JL_GC_PROMISE_ROOTED(new_bpart); // TODO: Analyzer doesn't understand MAYBE_UNROOTED properly jl_check_new_binding_implicit(new_bpart, b, st, world); + if (bpart && (bpart->kind & BINDING_FLAG_EXPORTED)) + new_bpart->kind |= BINDING_FLAG_EXPORTED; if (jl_atomic_cmpswap(insert, &bpart, new_bpart)) { jl_gc_wb(parent, new_bpart); return new_bpart; @@ -260,14 +265,15 @@ JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val3( jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *val, enum jl_partition_kind constant_kind, size_t new_world) { - JL_GC_PUSH1(&val); + jl_binding_partition_t *new_prev_bpart = NULL; + JL_GC_PUSH2(&val, &new_prev_bpart); if (!b) { b = jl_get_module_binding(mod, var, 1); } jl_binding_partition_t *new_bpart = NULL; jl_binding_partition_t *bpart = jl_get_binding_partition(b, new_world); while (!new_bpart) { - enum jl_partition_kind kind = bpart->kind; + enum jl_partition_kind kind = jl_binding_kind(bpart); if (jl_bkind_is_some_constant(kind)) { if (!val) { new_bpart = bpart; @@ -287,7 +293,7 @@ JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val3( jl_symbol_name(mod->name), jl_symbol_name(var)); } if (bpart->min_world == new_world) { - bpart->kind = constant_kind; + bpart->kind = constant_kind | (bpart->kind & 0xf0); bpart->restriction = val; if (val) jl_gc_wb(bpart, val); @@ -301,7 +307,7 @@ JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val3( // declared const, global, or imported. jl_binding_partition_t *prev_bpart = bpart; for (;;) { - enum jl_partition_kind prev_kind = prev_bpart->kind; + enum jl_partition_kind prev_kind = jl_binding_kind(prev_bpart); if (jl_bkind_is_some_constant(prev_kind) || prev_kind == BINDING_KIND_GLOBAL || (jl_bkind_is_some_import(prev_kind))) { need_backdate = 0; @@ -312,16 +318,29 @@ JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val3( prev_bpart = jl_get_binding_partition(b, prev_bpart->min_world - 1); } } - // If backdate is required, create one new binding partition to cover - // the entire backdate range. + // If backdate is required, replace each existing partition by a new one. + // We can't use one binding to cover the entire range, because we need to + // keep the flags partitioned. if (need_backdate) { + jl_binding_partition_t *prev_bpart = bpart; jl_binding_partition_t *backdate_bpart = new_binding_partition(); - backdate_bpart->kind = BINDING_KIND_BACKDATED_CONST; - backdate_bpart->restriction = val; - jl_gc_wb_fresh(backdate_bpart, val); - jl_atomic_store_relaxed(&backdate_bpart->max_world, new_world - 1); - jl_atomic_store_release(&new_bpart->next, backdate_bpart); - jl_gc_wb(new_bpart, backdate_bpart); + new_prev_bpart = backdate_bpart; + while (1) { + backdate_bpart->kind = (size_t)BINDING_KIND_BACKDATED_CONST | (prev_bpart->kind & 0xf0); + backdate_bpart->restriction = val; + backdate_bpart->min_world = prev_bpart->min_world; + jl_gc_wb_fresh(backdate_bpart, val); + jl_atomic_store_relaxed(&backdate_bpart->max_world, prev_bpart->max_world); + prev_bpart = jl_atomic_load_relaxed(&prev_bpart->next); + if (!prev_bpart) + break; + jl_binding_partition_t *next_prev_bpart = new_binding_partition(); + jl_atomic_store_relaxed(&backdate_bpart->next, next_prev_bpart); + jl_gc_wb(backdate_bpart, next_prev_bpart); + backdate_bpart = next_prev_bpart; + } + jl_atomic_store_release(&new_bpart->next, new_prev_bpart); + jl_gc_wb(new_bpart, new_prev_bpart); } } JL_GC_POP(); @@ -454,7 +473,6 @@ static jl_binding_t *new_binding(jl_module_t *mod, jl_sym_t *name) jl_atomic_store_relaxed(&b->partitions, NULL); b->globalref = NULL; b->backedges = NULL; - b->exportp = 0; b->publicp = 0; b->deprecated = 0; b->did_print_backdate_admonition = 0; @@ -500,7 +518,7 @@ static jl_module_t *jl_binding_dbgmodule(jl_binding_t *b, jl_module_t *m, jl_sym JL_DLLEXPORT void jl_check_binding_currently_writable(jl_binding_t *b, jl_module_t *m, jl_sym_t *s) { jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - enum jl_partition_kind kind = bpart->kind; + enum jl_partition_kind kind = jl_binding_kind(bpart); if (kind != BINDING_KIND_GLOBAL && kind != BINDING_KIND_DECLARED && !jl_bkind_is_some_constant(kind)) { if (jl_bkind_is_some_guard(kind)) { jl_errorf("Global %s.%s does not exist and cannot be assigned.\n" @@ -560,7 +578,7 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_value(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); - enum jl_partition_kind kind = bpart->kind; + 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)) { @@ -575,7 +593,7 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_value_seqcst(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); - enum jl_partition_kind kind = bpart->kind; + 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)) { @@ -590,7 +608,7 @@ JL_DLLEXPORT jl_value_t *jl_get_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); - enum jl_partition_kind kind = bpart->kind; + 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)) @@ -611,7 +629,7 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved_and_const(jl_binding_t size_t max_world = jl_atomic_load_relaxed(&bpart->max_world); if (bpart->min_world > jl_current_task->world_age || jl_current_task->world_age > max_world) return NULL; - enum jl_partition_kind kind = bpart->kind; + 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)) @@ -632,7 +650,7 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved(jl_binding_t *b) size_t max_world = jl_atomic_load_relaxed(&bpart->max_world); if (bpart->min_world > jl_current_task->world_age || jl_current_task->world_age > max_world) return NULL; - enum jl_partition_kind kind = bpart->kind; + enum jl_partition_kind kind = jl_binding_kind(bpart); if (jl_bkind_is_some_guard(kind)) return NULL; if (jl_bkind_is_some_import(kind)) @@ -658,7 +676,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_ { jl_binding_t *b = jl_get_module_binding(m, var, 1); jl_binding_partition_t *bpart = jl_get_binding_partition(b, new_world); - enum jl_partition_kind kind = bpart->kind; + enum jl_partition_kind kind = jl_binding_kind(bpart); if (kind == BINDING_KIND_GLOBAL || kind == BINDING_KIND_DECLARED || jl_bkind_is_some_constant(kind)) return b; if (jl_bkind_is_some_guard(kind)) { @@ -668,7 +686,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_ jl_binding_t *ownerb = b; jl_walk_binding_inplace(&ownerb, &bpart, new_world); jl_value_t *f = NULL; - if (jl_bkind_is_some_constant(bpart->kind)) + if (jl_bkind_is_some_constant(jl_binding_kind(bpart))) f = bpart->restriction; if (f == NULL) { if (kind == BINDING_KIND_IMPLICIT) { @@ -722,7 +740,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_ static jl_module_t *jl_binding_dbgmodule(jl_binding_t *b, jl_module_t *m, jl_sym_t *var) { jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - if (jl_bkind_is_some_import(bpart->kind)) { + if (jl_bkind_is_some_import(jl_binding_kind(bpart))) { return ((jl_binding_t*)bpart->restriction)->globalref->mod; } return m; @@ -738,9 +756,9 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_type(jl_module_t *m, jl_sym_t *var) if (b == NULL) return jl_nothing; jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age); - if (jl_bkind_is_some_guard(bpart->kind)) + if (jl_bkind_is_some_guard(jl_binding_kind(bpart))) return jl_nothing; - if (jl_bkind_is_some_constant(bpart->kind)) { + if (jl_bkind_is_some_constant(jl_binding_kind(bpart))) { // TODO: We would like to return the type of the constant, but // currently code relies on this returning any to bypass conversion // before an attempted assignment to a constant. @@ -779,7 +797,7 @@ JL_DLLEXPORT int jl_is_imported(jl_module_t *m, jl_sym_t *var) { jl_binding_t *b = jl_get_module_binding(m, var, 0); jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - return b && bpart->kind == BINDING_KIND_IMPORTED; + return b && jl_binding_kind(bpart) == BINDING_KIND_IMPORTED; } extern const char *jl_filename; @@ -864,7 +882,7 @@ static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *asname, jl_binding_partition_t *ownerbpart = bpart; jl_walk_binding_inplace(&ownerb, &ownerbpart, jl_current_task->world_age); - if (jl_bkind_is_some_guard(ownerbpart->kind)) { + if (jl_bkind_is_some_guard(jl_binding_kind(ownerbpart))) { jl_printf(JL_STDERR, "WARNING: Imported binding %s.%s was undeclared at import time during import to %s.\n", jl_symbol_name(from->name), jl_symbol_name(s), @@ -879,7 +897,7 @@ static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *asname, JL_LOCK(&world_counter_lock); size_t new_world = jl_atomic_load_acquire(&jl_world_counter)+1; jl_binding_partition_t *btopart = jl_get_binding_partition(bto, new_world); - enum jl_partition_kind btokind = btopart->kind; + enum jl_partition_kind btokind = jl_binding_kind(btopart); if (btokind == BINDING_KIND_GUARD || btokind == BINDING_KIND_IMPLICIT || btokind == BINDING_KIND_FAILED) { @@ -980,12 +998,13 @@ JL_DLLEXPORT void jl_module_using(jl_module_t *to, jl_module_t *from) jl_binding_t *b = (jl_binding_t*)jl_svecref(table, i); if ((void*)b == jl_nothing) break; - if (b->exportp) { + jl_binding_partition_t *frombpart = jl_get_binding_partition(b, new_world); + if (frombpart->kind & BINDING_FLAG_EXPORTED) { jl_sym_t *var = b->globalref->name; jl_binding_t *tob = jl_get_module_binding(to, var, 0); if (tob) { jl_binding_partition_t *tobpart = jl_get_binding_partition(tob, new_world); - enum jl_partition_kind kind = tobpart->kind; + enum jl_partition_kind kind = jl_binding_kind(tobpart); if (kind == BINDING_KIND_IMPLICIT || jl_bkind_is_some_guard(kind)) { jl_replace_binding_locked(tob, tobpart, NULL, BINDING_KIND_IMPLICIT_RECOMPUTE, new_world); } @@ -1018,17 +1037,25 @@ JL_DLLEXPORT jl_value_t *jl_get_module_binding_or_nothing(jl_module_t *m, jl_sym JL_DLLEXPORT void jl_module_public(jl_module_t *from, jl_sym_t *s, int exported) { jl_binding_t *b = jl_get_module_binding(from, s, 1); + JL_LOCK(&world_counter_lock); + size_t new_world = jl_atomic_load_acquire(&jl_world_counter)+1; + jl_binding_partition_t *bpart = jl_get_binding_partition(b, new_world); + int was_exported = (bpart->kind & BINDING_FLAG_EXPORTED) != 0; if (b->publicp) { // check for conflicting declarations - if (b->exportp && !exported) + if (was_exported && !exported) jl_errorf("cannot declare %s.%s public; it is already declared exported", jl_symbol_name(from->name), jl_symbol_name(s)); - if (!b->exportp && exported) + if (!was_exported && exported) jl_errorf("cannot declare %s.%s exported; it is already declared public", jl_symbol_name(from->name), jl_symbol_name(s)); } b->publicp = 1; - b->exportp |= exported; + if (was_exported != exported) { + jl_replace_binding_locked2(b, bpart, bpart->restriction, bpart->kind | BINDING_FLAG_EXPORTED, new_world); + jl_atomic_store_release(&jl_world_counter, new_world); + } + JL_UNLOCK(&world_counter_lock); } JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var, int allow_import) // unlike most queries here, this is currently seq_cst @@ -1040,14 +1067,14 @@ JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var, int allow_import) // u if (!bpart) return 0; if (!allow_import) { - if (!bpart || jl_bkind_is_some_import(bpart->kind)) + if (!bpart || jl_bkind_is_some_import(jl_binding_kind(bpart))) return 0; } else { jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age); } - if (jl_bkind_is_some_guard(bpart->kind)) + if (jl_bkind_is_some_guard(jl_binding_kind(bpart))) return 0; - if (jl_bkind_is_defined_constant(bpart->kind)) { + if (jl_bkind_is_defined_constant(jl_binding_kind(bpart))) { // N.B.: No backdated check for isdefined return 1; } @@ -1058,13 +1085,14 @@ JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var) { jl_binding_t *b = jl_get_module_binding(m, var, 0); jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - return b && (b->exportp || bpart->kind == BINDING_KIND_GLOBAL); + return b && ((bpart->kind & BINDING_FLAG_EXPORTED) || jl_binding_kind(bpart) == BINDING_KIND_GLOBAL); } JL_DLLEXPORT int jl_module_exports_p(jl_module_t *m, jl_sym_t *var) { jl_binding_t *b = jl_get_module_binding(m, var, 0); - return b && b->exportp; + jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); + return b && (bpart->kind & BINDING_FLAG_EXPORTED); } JL_DLLEXPORT int jl_module_public_p(jl_module_t *m, jl_sym_t *var) @@ -1171,7 +1199,7 @@ JL_DLLEXPORT void jl_set_const(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var jl_binding_partition_t *bpart = jl_get_binding_partition(bp, jl_current_task->world_age); bpart->min_world = 0; jl_atomic_store_release(&bpart->max_world, ~(size_t)0); - bpart->kind = BINDING_KIND_CONST; + bpart->kind = BINDING_KIND_CONST | (bpart->kind & 0xf0); bpart->restriction = val; jl_gc_wb(bpart, val); } @@ -1228,14 +1256,23 @@ JL_DLLEXPORT void jl_maybe_add_binding_backedge(jl_globalref_t *gr, jl_module_t JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked(jl_binding_t *b, jl_binding_partition_t *old_bpart, jl_value_t *restriction_val, enum jl_partition_kind kind, size_t new_world) +{ + // Copy flags from old bpart + return jl_replace_binding_locked2(b, old_bpart, restriction_val, (size_t)kind | (size_t)(old_bpart->kind & 0xf0), + new_world); +} + +JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked2(jl_binding_t *b, + jl_binding_partition_t *old_bpart, jl_value_t *restriction_val, size_t kind, size_t new_world) { assert(jl_atomic_load_relaxed(&b->partitions) == old_bpart); jl_atomic_store_release(&old_bpart->max_world, new_world-1); jl_binding_partition_t *new_bpart = new_binding_partition(); new_bpart->min_world = new_world; - if (kind == BINDING_KIND_IMPLICIT_RECOMPUTE) { + if ((kind & 0x0f) == BINDING_KIND_IMPLICIT_RECOMPUTE) { assert(!restriction_val); jl_check_new_binding_implicit(new_bpart /* callee rooted */, b, NULL, new_world); + new_bpart->kind |= kind & 0xf0; } else { new_bpart->kind = kind; @@ -1286,7 +1323,7 @@ JL_DLLEXPORT int jl_globalref_is_const(jl_globalref_t *gr) jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); if (!bpart) return 0; - return jl_bkind_is_some_constant(bpart->kind); + return jl_bkind_is_some_constant(jl_binding_kind(bpart)); } JL_DLLEXPORT void jl_disable_binding(jl_globalref_t *gr) @@ -1296,7 +1333,7 @@ JL_DLLEXPORT void jl_disable_binding(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); - if (bpart->kind == BINDING_KIND_GUARD) { + if (jl_binding_kind(bpart) == BINDING_KIND_GUARD) { // Already guard return; } @@ -1311,7 +1348,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(bpart->kind); + return b && jl_bkind_is_some_constant(jl_binding_kind(bpart)); } // set the deprecated flag for a binding: @@ -1362,7 +1399,7 @@ jl_value_t *jl_check_binding_assign_value(jl_binding_t *b JL_PROPAGATES_ROOT, jl { JL_GC_PUSH1(&rhs); // callee-rooted jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - enum jl_partition_kind kind = bpart->kind; + enum jl_partition_kind kind = jl_binding_kind(bpart); if (jl_bkind_is_some_constant(kind)) { jl_value_t *old = bpart->restriction; JL_GC_PROMISE_ROOTED(old); @@ -1412,7 +1449,7 @@ JL_DLLEXPORT jl_value_t *jl_checked_replace(jl_binding_t *b, jl_module_t *mod, j JL_DLLEXPORT jl_value_t *jl_checked_modify(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *op, jl_value_t *rhs) { jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - enum jl_partition_kind kind = bpart->kind; + enum jl_partition_kind kind = jl_binding_kind(bpart); assert(!jl_bkind_is_some_guard(kind) && !jl_bkind_is_some_import(kind)); if (jl_bkind_is_some_constant(kind)) jl_errorf("invalid assignment to constant %s.%s", @@ -1464,7 +1501,7 @@ void append_module_names(jl_array_t* a, jl_module_t *m, int all, int imported, i int hidden = jl_symbol_name(asname)[0]=='#'; int main_public = (m == jl_main_module && !(asname == jl_eval_sym || asname == jl_include_sym)); jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - enum jl_partition_kind kind = bpart->kind; + enum jl_partition_kind kind = jl_binding_kind(bpart); if (((b->publicp) || (imported && (kind == BINDING_KIND_CONST_IMPORT || kind == BINDING_KIND_IMPORTED)) || (usings && kind == BINDING_KIND_EXPLICIT) || @@ -1481,7 +1518,8 @@ void append_exported_names(jl_array_t* a, jl_module_t *m, int all) jl_binding_t *b = (jl_binding_t*)jl_svecref(table, i); if ((void*)b == jl_nothing) break; - if (b->exportp && (all || !b->deprecated)) + jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); + if ((bpart->kind & BINDING_FLAG_EXPORTED) && (all || !b->deprecated)) _append_symbol_to_bindings_array(a, b->globalref->name); } } @@ -1551,7 +1589,7 @@ JL_DLLEXPORT void jl_clear_implicit_imports(jl_module_t *m) if ((void*)b == jl_nothing) break; jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - if (bpart->kind == BINDING_KIND_IMPLICIT) { + if (jl_binding_kind(bpart) == BINDING_KIND_IMPLICIT) { jl_atomic_store_relaxed(&b->partitions, NULL); } } diff --git a/src/staticdata.c b/src/staticdata.c index c29448c491eb7..310bbcfc05a01 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -3498,7 +3498,7 @@ static void jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_ if (jl_atomic_load_relaxed(&bpart->max_world) != ~(size_t)0) return; - enum jl_partition_kind kind = bpart->kind; + enum jl_partition_kind kind = jl_binding_kind(bpart); if (!jl_bkind_is_some_import(kind)) return; jl_binding_t *imported_binding = (jl_binding_t*)bpart->restriction; @@ -3537,7 +3537,7 @@ static void jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_ jl_validate_binding_partition(bedge, jl_atomic_load_relaxed(&bedge->partitions), mod_idx); } } - if (b->exportp) { + if (bpart->kind & BINDING_FLAG_EXPORTED) { jl_module_t *mod = b->globalref->mod; jl_sym_t *name = b->globalref->name; JL_LOCK(&mod->lock); diff --git a/src/toplevel.c b/src/toplevel.c index 9bb82a3451848..3b8b0e2d5b428 100644 --- a/src/toplevel.c +++ b/src/toplevel.c @@ -307,7 +307,7 @@ void jl_declare_global(jl_module_t *m, jl_value_t *arg, jl_value_t *set_type, in global_type = (jl_value_t*)jl_any_type; while (1) { bpart = jl_get_binding_partition(b, new_world); - enum jl_partition_kind kind = bpart->kind; + enum jl_partition_kind kind = jl_binding_kind(bpart); if (kind != BINDING_KIND_GLOBAL) { if (jl_bkind_is_some_guard(kind) || kind == BINDING_KIND_DECLARED || kind == BINDING_KIND_IMPLICIT) { if (kind == new_kind) { @@ -317,7 +317,7 @@ void jl_declare_global(jl_module_t *m, jl_value_t *arg, jl_value_t *set_type, in } check_safe_newbinding(gm, gs); if (bpart->min_world == new_world) { - bpart->kind = new_kind; + bpart->kind = new_kind | (bpart->kind & 0xf0); bpart->restriction = global_type; if (global_type) jl_gc_wb(bpart, global_type); @@ -655,11 +655,11 @@ static void import_module(jl_module_t *JL_NONNULL m, jl_module_t *import, jl_sym // TODO: this is a bit race-y with what error message we might print jl_binding_t *b = jl_get_module_binding(m, name, 1); jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - enum jl_partition_kind kind = bpart->kind; + enum jl_partition_kind kind = jl_binding_kind(bpart); if (kind != BINDING_KIND_GUARD && kind != BINDING_KIND_FAILED && kind != BINDING_KIND_DECLARED && kind != BINDING_KIND_IMPLICIT) { // Unlike regular constant declaration, we allow this as long as we eventually end up at a constant. jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age); - if (bpart->kind == BINDING_KIND_CONST || bpart->kind == BINDING_KIND_BACKDATED_CONST || bpart->kind == BINDING_KIND_CONST_IMPORT) { + if (jl_binding_kind(bpart) == BINDING_KIND_CONST || jl_binding_kind(bpart) == BINDING_KIND_BACKDATED_CONST || jl_binding_kind(bpart) == BINDING_KIND_CONST_IMPORT) { // Already declared (e.g. on another thread) or imported. if (bpart->restriction == (jl_value_t*)import) return; diff --git a/test/rebinding.jl b/test/rebinding.jl index 23feb8ded0e56..2f343fd86eb9a 100644 --- a/test/rebinding.jl +++ b/test/rebinding.jl @@ -193,10 +193,31 @@ module RebindingPrecompile Core.eval(Export2, :(const import_me2 = 22)) end invokelatest() do - # Currently broken - # @test_throws UndefVarError ImportTest.f_use_binding2() + @test_throws UndefVarError ImportTest.f_use_binding2() end end finish_precompile_test!() end + +module Regression + using Test + + # Issue #57377 + module GeoParams57377 + module B + using ...GeoParams57377 + export S + struct S end + module C + using ..GeoParams57377 + h() = S() + x -> nothing + end + end + + using .B + export S + end + @test GeoParams57377.B.C.h() == GeoParams57377.B.C.S() +end