replace incorrect Method.deleted_world with more useful Method.dispatch_status enum

vtjnash · vtjnash · commit 76872e5f8921 · 2025-05-05T21:31:11.000Z
The original purpose of this was to manage quickly detecting if a method was replaced, but that stopped being correct after #53415. It is also a fairly heavy-weight description of a single bit of information, and apparently (despite not having "min" or "max" anywhere in the name), its existence misleads people into thinking that primary/deleted are somehow corresponding to min/max. That is always false: this bit only exists for invalidations, and primary_world is primarily valid to use for supporting generated functions (or cases like `show` where correctness isn't especially important, though we may use it at times as a short-cut to computing the result of a dispatch result). This implements METHOD_ISMINMAX_INVOKE_LATEST fully, and provides just a stub for adding METHOD_ISMINMAX_CALL_LATEST later. Fix #58215
diff --git a/Compiler/src/typeinfer.jl b/Compiler/src/typeinfer.jl
@@ -1540,7 +1540,7 @@ function compile!(codeinfos::Vector{Any}, workqueue::CompilationQueue;
             # if this method is generally visible to the current compilation world,
             # and this is either the primary world, or not applicable in the primary world
             # then we want to compile and emit this
-            if item.def.primary_world <= world <= item.def.deleted_world
+            if item.def.primary_world <= world
                 ci = typeinf_ext(interp, item, SOURCE_MODE_GET_SOURCE)
                 ci isa CodeInstance && push!(workqueue, ci)
             end
diff --git a/base/errorshow.jl b/base/errorshow.jl
@@ -592,8 +592,6 @@ function show_method_candidates(io::IO, ex::MethodError, kwargs=[])
             end
             if ex.world < reinterpret(UInt, method.primary_world)
                 print(iob, " (method too new to be called from this world context.)")
-            elseif ex.world > reinterpret(UInt, method.deleted_world)
-                print(iob, " (method deleted before this world age.)")
             end
             println(iob)
 
diff --git a/base/staticdata.jl b/base/staticdata.jl
@@ -340,24 +340,22 @@ function verify_call(@nospecialize(sig), expecteds::Core.SimpleVector, i::Int, n
     return minworld[], maxworld[], result
 end
 
+const METHOD_ISMINMAX_INVOKE_LATEST = 0x1
+const METHOD_ISMINMAX_CALL_LATEST = 0x2
+
 function verify_invokesig(@nospecialize(invokesig), expected::Method, world::UInt)
     @assert invokesig isa Type
     local minworld::UInt, maxworld::UInt
     matched = nothing
-    if invokesig === expected.sig
-        # the invoke match is `expected` for `expected->sig`, unless `expected` is invalid
-        # TODO: this is broken since PR #53415
+    if invokesig === expected.sig && !iszero(expected.dispatch_status & METHOD_ISMINMAX_INVOKE_LATEST)
+        # the invoke match is `expected` for `expected->sig`, unless `expected` is replaced
         minworld = expected.primary_world
-        maxworld = expected.deleted_world
         @assert minworld ≤ world
-        if maxworld < world
-            maxworld = 0
-        end
-    else
-        minworld = 1
         maxworld = typemax(UInt)
+    else
         mt = get_methodtable(expected)
         if mt === nothing
+            minworld = 1
             maxworld = 0
         else
             matched, valid_worlds = Compiler._findsup(invokesig, mt, world)
diff --git a/src/gf.c b/src/gf.c
@@ -296,7 +296,7 @@ void jl_mk_builtin_func(jl_datatype_t *dt, jl_sym_t *sname, jl_fptr_args_t fptr)
     m->isva = 1;
     m->nargs = 2;
     jl_atomic_store_relaxed(&m->primary_world, 1);
-    jl_atomic_store_relaxed(&m->deleted_world, ~(size_t)0);
+    jl_atomic_store_relaxed(&m->dispatch_status, METHOD_ISMINMAX_INVOKE_LATEST | METHOD_ISMINMAX_CALL_LATEST);
     m->sig = (jl_value_t*)jl_anytuple_type;
     m->slot_syms = jl_an_empty_string;
     m->nospecialize = 0;
@@ -307,7 +307,7 @@ void jl_mk_builtin_func(jl_datatype_t *dt, jl_sym_t *sname, jl_fptr_args_t fptr)
     JL_GC_PUSH2(&m, &newentry);
 
     newentry = jl_typemap_alloc(jl_anytuple_type, NULL, jl_emptysvec,
-            (jl_value_t*)m, jl_atomic_load_relaxed(&m->primary_world), jl_atomic_load_relaxed(&m->deleted_world));
+            (jl_value_t*)m, 1, ~(size_t)0);
     jl_typemap_insert(&mt->defs, (jl_value_t*)mt, newentry, jl_cachearg_offset(mt));
 
     jl_method_instance_t *mi = jl_get_specialized(m, (jl_value_t*)jl_anytuple_type, jl_emptysvec);
@@ -2242,17 +2242,22 @@ JL_DLLEXPORT void jl_method_table_disable(jl_methtable_t *mt, jl_method_t *metho
     JL_LOCK(&world_counter_lock);
     if (!jl_atomic_load_relaxed(&allow_new_worlds))
         jl_error("Method changes have been disabled via a call to disable_new_worlds.");
-    JL_LOCK(&mt->writelock);
-    // Narrow the world age on the method to make it uncallable
-    size_t world = jl_atomic_load_relaxed(&jl_world_counter);
-    assert(method == methodentry->func.method);
-    assert(jl_atomic_load_relaxed(&method->deleted_world) == ~(size_t)0);
-    jl_atomic_store_relaxed(&method->deleted_world, world);
-    jl_atomic_store_relaxed(&methodentry->max_world, world);
-    jl_method_table_invalidate(mt, method, world);
-    jl_atomic_store_release(&jl_world_counter, world + 1);
-    JL_UNLOCK(&mt->writelock);
+    int enabled = jl_atomic_load_relaxed(&methodentry->max_world) == ~(size_t)0;
+    if (enabled) {
+        JL_LOCK(&mt->writelock);
+        // Narrow the world age on the method to make it uncallable
+        size_t world = jl_atomic_load_relaxed(&jl_world_counter);
+        assert(method == methodentry->func.method);
+        jl_atomic_store_relaxed(&method->dispatch_status, 0);
+        assert(jl_atomic_load_relaxed(&methodentry->max_world) == ~(size_t)0);
+        jl_atomic_store_relaxed(&methodentry->max_world, world);
+        jl_method_table_invalidate(mt, method, world);
+        jl_atomic_store_release(&jl_world_counter, world + 1);
+        JL_UNLOCK(&mt->writelock);
+    }
     JL_UNLOCK(&world_counter_lock);
+    if (!enabled)
+        jl_errorf("Method of %s already disabled", jl_symbol_name(method->name));
 }
 
 static int jl_type_intersection2(jl_value_t *t1, jl_value_t *t2, jl_value_t **isect JL_REQUIRE_ROOTED_SLOT, jl_value_t **isect2 JL_REQUIRE_ROOTED_SLOT)
@@ -2292,9 +2297,8 @@ jl_typemap_entry_t *jl_method_table_add(jl_methtable_t *mt, jl_method_t *method,
     JL_LOCK(&mt->writelock);
     // add our new entry
     assert(jl_atomic_load_relaxed(&method->primary_world) == ~(size_t)0); // min-world
-    assert(jl_atomic_load_relaxed(&method->deleted_world) == 1); // max-world
-    newentry = jl_typemap_alloc((jl_tupletype_t*)method->sig, simpletype, jl_emptysvec, (jl_value_t*)method,
-            jl_atomic_load_relaxed(&method->primary_world), jl_atomic_load_relaxed(&method->deleted_world));
+    assert(jl_atomic_load_relaxed(&method->dispatch_status) == 0);
+    newentry = jl_typemap_alloc((jl_tupletype_t*)method->sig, simpletype, jl_emptysvec, (jl_value_t*)method, ~(size_t)0, 1);
     jl_typemap_insert(&mt->defs, (jl_value_t*)mt, newentry, jl_cachearg_offset(mt));
     update_max_args(mt, method->sig);
     JL_UNLOCK(&mt->writelock);
@@ -2315,7 +2319,7 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry)
     JL_LOCK(&mt->writelock);
     size_t world = jl_atomic_load_relaxed(&method->primary_world);
     assert(world == jl_atomic_load_relaxed(&jl_world_counter) + 1); // min-world
-    assert(jl_atomic_load_relaxed(&method->deleted_world) == ~(size_t)0); // max-world
+    assert(jl_atomic_load_relaxed(&method->dispatch_status) == 0);
     assert(jl_atomic_load_relaxed(&newentry->min_world) == ~(size_t)0);
     assert(jl_atomic_load_relaxed(&newentry->max_world) == 1);
     jl_atomic_store_relaxed(&newentry->min_world, world);
@@ -2336,6 +2340,7 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry)
         method_overwrite(newentry, replaced->func.method);
         // this is an optimized version of below, given we know the type-intersection is exact
         jl_method_table_invalidate(mt, replaced->func.method, max_world);
+        jl_atomic_store_relaxed(&replaced->func.method->dispatch_status, 0);
     }
     else {
         jl_method_t *const *d;
@@ -2479,13 +2484,15 @@ void jl_method_table_activate(jl_methtable_t *mt, jl_typemap_entry_t *newentry)
                 }
             }
         }
+        // TODO: computed and store updates to METHOD_ISMINMAX_CALL_LATEST here
     }
     if (invalidated && _jl_debug_method_invalidation) {
         jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)method);
         loctag = jl_cstr_to_string("jl_method_table_insert");
         jl_array_ptr_1d_push(_jl_debug_method_invalidation, loctag);
     }
-    jl_atomic_store_relaxed(&newentry->max_world, jl_atomic_load_relaxed(&method->deleted_world));
+    jl_atomic_store_relaxed(&newentry->max_world, ~(size_t)0);
+    jl_atomic_store_relaxed(&method->dispatch_status, METHOD_ISMINMAX_INVOKE_LATEST); // TODO: this should be sequenced after the world counter store
     JL_UNLOCK(&mt->writelock);
     JL_GC_POP();
 }
@@ -2499,7 +2506,6 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method
         jl_error("Method changes have been disabled via a call to disable_new_worlds.");
     size_t world = jl_atomic_load_relaxed(&jl_world_counter) + 1;
     jl_atomic_store_relaxed(&method->primary_world, world);
-    jl_atomic_store_relaxed(&method->deleted_world, ~(size_t)0);
     jl_method_table_activate(mt, newentry);
     jl_atomic_store_release(&jl_world_counter, world);
     JL_UNLOCK(&world_counter_lock);
@@ -3882,6 +3888,8 @@ static int ml_matches_visitor(jl_typemap_entry_t *ml, struct typemap_intersectio
             closure->match.min_valid = max_world + 1;
         return 1;
     }
+    if (closure->match.max_valid > max_world)
+        closure->match.max_valid = max_world;
     jl_method_t *meth = ml->func.method;
     if (closure->lim >= 0 && jl_is_dispatch_tupletype(meth->sig)) {
         int replaced = 0;
@@ -4554,12 +4562,9 @@ static jl_value_t *ml_matches(jl_methtable_t *mt,
         jl_method_t *m = matc->method;
         // method applicability is the same as typemapentry applicability
         size_t min_world = jl_atomic_load_relaxed(&m->primary_world);
-        size_t max_world = jl_atomic_load_relaxed(&m->deleted_world);
         // intersect the env valid range with method lookup's inclusive valid range
         if (env.match.min_valid < min_world)
             env.match.min_valid = min_world;
-        if (env.match.max_valid > max_world)
-            env.match.max_valid = max_world;
     }
     if (mt && cache_result && ((jl_datatype_t*)unw)->isdispatchtuple) { // cache_result parameter keeps this from being recursive
         if (len == 1 && !has_ambiguity) {
diff --git a/src/jltypes.c b/src/jltypes.c
@@ -3545,8 +3545,8 @@ void jl_init_types(void) JL_GC_DISABLED
                             "module",
                             "file",
                             "line",
+                            "dispatch_status", // atomic
                             "primary_world", // atomic
-                            "deleted_world", // atomic
                             "sig",
                             "specializations", // !const
                             "speckeyset", // !const
@@ -3578,7 +3578,7 @@ void jl_init_types(void) JL_GC_DISABLED
                             jl_module_type,
                             jl_symbol_type,
                             jl_int32_type,
-                            jl_ulong_type,
+                            jl_int32_type,
                             jl_ulong_type,
                             jl_type_type,
                             jl_any_type, // union(jl_simplevector_type, jl_method_instance_type),
diff --git a/src/julia.h b/src/julia.h
@@ -331,8 +331,8 @@ typedef struct _jl_method_t {
     struct _jl_module_t *module;
     jl_sym_t *file;
     int32_t line;
+    _Atomic(int32_t) dispatch_status;
     _Atomic(size_t) primary_world;
-    _Atomic(size_t) deleted_world;
 
     // method's type signature. redundant with TypeMapEntry->specTypes
     jl_value_t *sig;
diff --git a/src/julia_internal.h b/src/julia_internal.h
@@ -677,6 +677,9 @@ typedef union {
 #define SOURCE_MODE_NOT_REQUIRED            0x0
 #define SOURCE_MODE_ABI                     0x1
 
+#define METHOD_ISMINMAX_INVOKE_LATEST       0b0001
+#define METHOD_ISMINMAX_CALL_LATEST         0b0010
+
 JL_DLLEXPORT jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner);
 JL_DLLEXPORT void jl_engine_fulfill(jl_code_instance_t *ci, jl_code_info_t *src);
 void jl_engine_sweep(jl_ptls_t *gc_all_tls_states) JL_NOTSAFEPOINT;
diff --git a/src/method.c b/src/method.c
@@ -727,7 +727,7 @@ JL_DLLEXPORT jl_code_info_t *jl_code_for_staged(jl_method_instance_t *mi, size_t
     JL_TRY {
         ct->ptls->in_pure_callback = 1;
         ct->world_age = jl_atomic_load_relaxed(&def->primary_world);
-        if (ct->world_age > jl_atomic_load_acquire(&jl_world_counter) || jl_atomic_load_relaxed(&def->deleted_world) < ct->world_age)
+        if (ct->world_age > jl_atomic_load_acquire(&jl_world_counter))
             jl_error("The generator method cannot run until it is added to a method table.");
 
         // invoke code generator
@@ -1006,7 +1006,7 @@ JL_DLLEXPORT jl_method_t *jl_new_method_uninit(jl_module_t *module)
     m->isva = 0;
     m->nargs = 0;
     jl_atomic_store_relaxed(&m->primary_world, ~(size_t)0);
-    jl_atomic_store_relaxed(&m->deleted_world, 1);
+    jl_atomic_store_relaxed(&m->dispatch_status, 0);
     m->is_for_opaque_closure = 0;
     m->nospecializeinfer = 0;
     jl_atomic_store_relaxed(&m->did_scan_source, 0);
diff --git a/src/opaque_closure.c b/src/opaque_closure.c
@@ -159,7 +159,6 @@ JL_DLLEXPORT jl_opaque_closure_t *jl_new_opaque_closure_from_code_info(jl_tuplet
     size_t world = jl_current_task->world_age;
     // these are only legal in the current world since they are not in any tables
     jl_atomic_store_release(&meth->primary_world, world);
-    jl_atomic_store_release(&meth->deleted_world, world);
 
     if (isinferred) {
         jl_value_t *argslotty = jl_array_ptr_ref(ci->slottypes, 0);
diff --git a/src/precompile_utils.c b/src/precompile_utils.c
@@ -421,14 +421,14 @@ static void jl_rebuild_methtables(arraylist_t* MIs, htable_t* mtables)
         if (lookup == jl_nothing || (jl_method_t*)lookup != m) {
             //TODO: should this be a function like unsafe_insert_method?
             size_t min_world = jl_atomic_load_relaxed(&m->primary_world);
-            size_t max_world = jl_atomic_load_relaxed(&m->deleted_world);
+            assert(min_world == jl_atomic_load_relaxed(&m->primary_world));
+            size_t dispatch_status = jl_atomic_load_relaxed(&m->dispatch_status);
             jl_atomic_store_relaxed(&m->primary_world, ~(size_t)0);
-            jl_atomic_store_relaxed(&m->deleted_world, 1);
+            jl_atomic_store_relaxed(&m->dispatch_status, 0);
             jl_typemap_entry_t *newentry = jl_method_table_add(mt, m, NULL);
             jl_atomic_store_relaxed(&m->primary_world, min_world);
-            jl_atomic_store_relaxed(&m->deleted_world, max_world);
+            jl_atomic_store_relaxed(&m->dispatch_status, dispatch_status);
             jl_atomic_store_relaxed(&newentry->min_world, min_world);
-            jl_atomic_store_relaxed(&newentry->max_world, max_world);
         }
     }
 
diff --git a/src/staticdata.c b/src/staticdata.c
@@ -1805,16 +1805,10 @@ static void jl_write_values(jl_serializer_state *s) JL_GC_DISABLED
                 jl_method_t *m = (jl_method_t*)v;
                 jl_method_t *newm = (jl_method_t*)&f->buf[reloc_offset];
                 if (s->incremental) {
-                    if (jl_atomic_load_relaxed(&newm->deleted_world) == ~(size_t)0) {
-                        if (jl_atomic_load_relaxed(&newm->primary_world) > 1) {
-                            jl_atomic_store_relaxed(&newm->primary_world, ~(size_t)0); // min-world
-                            jl_atomic_store_relaxed(&newm->deleted_world, 1); // max_world
-                            arraylist_push(&s->fixup_objs, (void*)reloc_offset);
-                        }
-                    }
-                    else {
-                        jl_atomic_store_relaxed(&newm->primary_world, 1);
-                        jl_atomic_store_relaxed(&newm->deleted_world, 0);
+                    if (jl_atomic_load_relaxed(&newm->primary_world) > 1) {
+                        jl_atomic_store_relaxed(&newm->primary_world, ~(size_t)0); // min-world
+                        jl_atomic_store_relaxed(&newm->dispatch_status, 0);
+                        arraylist_push(&s->fixup_objs, (void*)reloc_offset);
                     }
                 }
                 else {
diff --git a/src/staticdata_utils.c b/src/staticdata_utils.c
@@ -706,9 +706,7 @@ static void jl_activate_methods(jl_array_t *external, jl_array_t *internal, size
         else if (jl_is_method(obj)) {
             jl_method_t *m = (jl_method_t*)obj;
             assert(jl_atomic_load_relaxed(&m->primary_world) == ~(size_t)0);
-            assert(jl_atomic_load_relaxed(&m->deleted_world) == WORLD_AGE_REVALIDATION_SENTINEL);
             jl_atomic_store_release(&m->primary_world, world);
-            jl_atomic_store_release(&m->deleted_world, ~(size_t)0);
         }
         else if (jl_is_code_instance(obj)) {
             jl_code_instance_t *ci = (jl_code_instance_t*)obj;
diff --git a/test/core.jl b/test/core.jl
@@ -35,7 +35,7 @@ end
 for (T, c) in (
         (Core.CodeInfo, []),
         (Core.CodeInstance, [:next, :min_world, :max_world, :inferred, :edges, :debuginfo, :ipo_purity_bits, :invoke, :specptr, :specsigflags, :precompile, :time_compile]),
-        (Core.Method, [:primary_world, :deleted_world]),
+        (Core.Method, [:primary_world, :dispatch_status]),
         (Core.MethodInstance, [:cache, :flags]),
         (Core.MethodTable, [:defs, :leafcache, :cache, :max_args]),
         (Core.TypeMapEntry, [:next, :min_world, :max_world]),
diff --git a/test/worlds.jl b/test/worlds.jl
@@ -502,6 +502,8 @@ Base.delete_method(fshadow_m2)
 @test Base.morespecific(fshadow_m3, fshadow_m1)
 @test !Base.morespecific(fshadow_m2, fshadow_m3)
 
+@test_throws "Method of fshadow already disabled" Base.delete_method(fshadow_m2)
+
 # Generated functions without edges must have min_world = 1.
 # N.B.: If changing this, move this test to precompile and make sure
 # that the specialization survives revalidation.
