bpart: Also partition the export flag (#57405)

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

(cherry picked from commit b27a24a)

Author: Keno

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

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)

base/show.jl

@@ -3368,6 +3368,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

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)

src/codegen.cpp

@@ -3139,7 +3139,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;
     }
@@ -3165,7 +3165,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)
@@ -3192,7 +3192,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)
@@ -3443,14 +3443,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);
@@ -3461,7 +3461,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) {
@@ -3472,7 +3472,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);
@@ -3495,7 +3495,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!";
@@ -4181,7 +4181,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);

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,30 +719,30 @@ 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) JL_NOTSAFEPOINT
+{
+    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
     _Atomic(jl_value_t*) value;
     _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 {

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);

src/method.c

@@ -1061,10 +1061,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);