crates/vm/src/frame.rs (1)

4418-4455: Consider extracting shared descriptor-args assembly into one helper.

The four CALL_METHOD_DESCRIPTOR_* branches duplicate nearly identical stack-pop and FuncArgs construction logic. A single helper would reduce drift and make guard consistency easier to maintain.

As per coding guidelines, "When branches differ only in a value but share common logic, extract the differing value first, then call the common logic once to avoid duplicate code."

Also applies to: 4469-4506, 4519-4557, 4650-4690

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/frame.rs` around lines 4418 - 4455, Duplicate logic across the
CALL_METHOD_DESCRIPTOR_* branches should be consolidated: extract the shared
sequence that pops the callable/positional/self values from the frame (using
self.pop_multiple(n), self.pop_value_opt(), self.pop_value()), checks/uses
nth_value and localsplus.stack_index, and constructs the FuncArgs (args Vec and
kwargs default) into a single helper function (e.g.,
assemble_descriptor_call_args or similar) that returns the built FuncArgs and
the method func to invoke; then replace each branch (the ones checking
PyMethodDescriptor, PyMethodFlags and class guard) to compute only the differing
guard/value (descr and any flag-dependent selection) and call the new helper to
get the FuncArgs before continuing, updating usages of positional_args,
all_args, self_or_null, and func to the helper's return values; apply the same
refactor at the other listed regions (around lines 4469-4506, 4519-4557,
4650-4690) so the common stack-manipulation and FuncArgs construction is
implemented once and reused.

crates/vm/src/builtins/function.rs (1)

643-663: Reduce predicate drift between specialization helpers.

is_simple_for_call_specialization and can_specialize_call duplicate most eligibility checks. It would be safer to compose one from the other.

💡 Suggested refactor

 pub(crate) fn can_specialize_call(&self, effective_nargs: u32) -> bool {
-    let code: &Py<PyCode> = &self.code;
-    let flags = code.flags;
-    flags.contains(bytecode::CodeFlags::OPTIMIZED)
-        && !flags.intersects(bytecode::CodeFlags::VARARGS | bytecode::CodeFlags::VARKEYWORDS)
-        && code.kwonlyarg_count == 0
-        && code.arg_count == effective_nargs
+    self.is_simple_for_call_specialization() && self.code.arg_count == effective_nargs
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/function.rs` around lines 643 - 663, Both helpers
duplicate the same eligibility checks; simplify by composing them: keep
is_simple_for_call_specialization as the single predicate for flag/kw-only
checks, and change can_specialize_call to call
is_simple_for_call_specialization(&self) and then only compare code.arg_count ==
effective_nargs to decide specialization. Update can_specialize_call to
reference self.code.arg_count after delegating the common checks to
is_simple_for_call_specialization and remove the duplicated flags/kwonly logic.

crates/vm/src/builtins/type.rs (2)

225-263: ⚠️ Potential issue | 🟠 Major

Track and clear specialization_init in PyType manual GC hooks.

HeapTypeExt::specialization_init introduces a strong PyRef<PyFunction> edge, but PyType::traverse/clear still don’t visit or clear it. This can keep type/function cycles alive.

💡 Suggested fix

 unsafe impl crate::object::Traverse for PyType {
     fn traverse(&self, tracer_fn: &mut crate::object::TraverseFn<'_>) {
         self.base.traverse(tracer_fn);
         self.bases.traverse(tracer_fn);
         self.mro.traverse(tracer_fn);
         self.subclasses.traverse(tracer_fn);
         self.attributes
             .read_recursive()
             .iter()
             .map(|(_, v)| v.traverse(tracer_fn))
             .count();
+        if let Some(ext) = self.heaptype_ext.as_ref() {
+            ext.specialization_init.read().traverse(tracer_fn);
+        }
     }

     /// type_clear: break reference cycles in type objects
     fn clear(&mut self, out: &mut Vec<crate::PyObjectRef>) {
+        if let Some(ext) = self.heaptype_ext.as_ref()
+            && let Some(mut guard) = ext.specialization_init.try_write()
+            && let Some(init) = guard.take()
+        {
+            out.push(init.into());
+        }
         if let Some(base) = self.base.take() {
             out.push(base.into());
         }
         // ...
     }
 }

#!/bin/bash
set -euo pipefail
file="crates/vm/src/builtins/type.rs"

# Verify specialization_init exists and inspect manual GC hooks.
rg -n "specialization_init" "$file"
rg -n -A25 -B5 "unsafe impl crate::object::Traverse for PyType" "$file"
rg -n -A45 -B5 "fn clear\(&mut self, out: &mut Vec<crate::PyObjectRef>\)" "$file"

Also applies to: 267-273

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/type.rs` around lines 225 - 263, The manual GC hooks
for PyType are missing handling for HeapTypeExt::specialization_init which holds
a strong PyRef<PyFunction>; update PyType::traverse to call tracer_fn on the
specialization_init edge (e.g., read the specialization_init field and invoke
its traverse/tracer function) and update PyType::clear to take/clear
specialization_init (push the held PyObjectRef into out when present) so that
the strong reference is visited and broken during GC; locate the fields and
handlers via the HeapTypeExt::specialization_init symbol and the unsafe impl
crate::object::Traverse for PyType (methods traverse and clear) and add
symmetric visit/clear logic.

---

800-830: ⚠️ Potential issue | 🟠 Major

Re-check tp_version_tag while holding the cache lock.

Both cache accessors validate tp_version_tag before acquiring specialization_init lock. A concurrent modified() can race and allow stale __init__ to be written/read across versions.

💡 Suggested fix

 pub(crate) fn cache_init_for_specialization(
     &self,
     init: PyRef<PyFunction>,
     tp_version: u32,
 ) -> bool {
     let Some(ext) = self.heaptype_ext.as_ref() else {
         return false;
     };
-    if tp_version == 0 || self.tp_version_tag.load(Ordering::Acquire) != tp_version {
+    if tp_version == 0 {
         return false;
     }
-    *ext.specialization_init.write() = Some(init);
+    let mut guard = ext.specialization_init.write();
+    if self.tp_version_tag.load(Ordering::Acquire) != tp_version {
+        return false;
+    }
+    *guard = Some(init);
     true
 }

 pub(crate) fn get_cached_init_for_specialization(
     &self,
     tp_version: u32,
 ) -> Option<PyRef<PyFunction>> {
     let ext = self.heaptype_ext.as_ref()?;
-    if tp_version == 0 || self.tp_version_tag.load(Ordering::Acquire) != tp_version {
+    if tp_version == 0 {
         return None;
     }
-    ext.specialization_init
-        .read()
-        .as_ref()
-        .map(|init| init.to_owned())
+    let guard = ext.specialization_init.read();
+    if self.tp_version_tag.load(Ordering::Acquire) != tp_version {
+        return None;
+    }
+    guard.as_ref().cloned()
 }

#!/bin/bash
set -euo pipefail
file="crates/vm/src/builtins/type.rs"

# Inspect lock acquisition order vs tp_version_tag checks.
rg -n -A30 -B5 "fn cache_init_for_specialization" "$file"
rg -n -A30 -B5 "fn get_cached_init_for_specialization" "$file"
rg -n -A20 -B5 "pub fn modified\(&self\)" "$file"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/type.rs` around lines 800 - 830, The tp_version_tag
check must be re-validated while holding the specialization_init lock to prevent
a race with modified(); in cache_init_for_specialization() acquire
ext.specialization_init.write() first, then re-check tp_version_tag (and
tp_version != 0) before storing Some(init), returning false if the tag changed;
similarly in get_cached_init_for_specialization() acquire
ext.specialization_init.read() first, then re-check tp_version_tag (and
tp_version != 0) while holding the read lock and return None if it changed,
otherwise clone and return the cached init. Ensure you use the same ext from
heaptype_ext.as_ref() and the existing read()/write() guards to avoid changing
locking primitives.

crates/vm/src/builtins/function.rs (1)

643-651: Deduplicate specialization predicates to reduce drift risk.

can_specialize_call and is_simple_for_call_specialization share the same shape checks. Reusing the helper keeps future updates consistent.

♻️ Proposed refactor

 pub(crate) fn can_specialize_call(&self, effective_nargs: u32) -> bool {
-    let code: &Py<PyCode> = &self.code;
-    let flags = code.flags;
-    flags.contains(bytecode::CodeFlags::OPTIMIZED)
-        && !flags.intersects(bytecode::CodeFlags::VARARGS | bytecode::CodeFlags::VARKEYWORDS)
-        && code.kwonlyarg_count == 0
-        && code.arg_count == effective_nargs
+    self.is_simple_for_call_specialization() && self.code.arg_count == effective_nargs
 }

Also applies to: 656-663

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/function.rs` around lines 643 - 651, Both
can_specialize_call and is_simple_for_call_specialization duplicate the same
"simple function" shape checks; extract the shared predicate into a single
helper (e.g., has_simple_call_shape or similar) and call it from both functions.
Update is_simple_for_call_specialization to delegate to the helper (using the
existing self.code and its flags/kwonlyarg_count logic) and change
can_specialize_call to use the same helper so future flag/arg-count changes are
made in one place; reference the existing symbols can_specialize_call and
is_simple_for_call_specialization when locating where to replace the duplicated
logic.

crates/vm/src/builtins/object.rs (1)

108-121: Add a defensive assertion for _nitems in generic_alloc.

Since _nitems is currently ignored, a debug assertion helps catch accidental variable-size allocation usage early.

🛡️ Suggested guard rail

 pub(crate) fn generic_alloc(cls: PyTypeRef, _nitems: usize, vm: &VirtualMachine) -> PyResult {
+    debug_assert_eq!(
+        _nitems, 0,
+        "generic_alloc currently supports fixed-size allocations only"
+    );
     // Only create dict if the class has HAS_DICT flag (i.e., __slots__ was not defined
     // or __dict__ is in __slots__)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/object.rs` around lines 108 - 121, Add a defensive
debug assertion in the generic_alloc function to ensure the ignored _nitems
argument is zero so callers don't accidentally request variable-size allocation;
inside generic_alloc (which takes cls: PyTypeRef, _nitems: usize, vm:
&VirtualMachine) add a debug_assert_eq!(_nitems, 0, "...") (or
debug_assert!(_nitems == 0, "...")) with a brief message indicating
variable-size allocation is not supported for PyBaseObject.

crates/vm/src/builtins/type.rs (2)

236-238: ⚠️ Potential issue | 🟠 Major

specialization_getitem also needs manual traverse/clear.

HeapTypeExt now owns another strong PyRef<PyFunction>, but only specialization_init participates in PyType’s manual GC hooks. That can keep class/function cycles alive after type_clear().

Proposed fix

     fn traverse(&self, tracer_fn: &mut crate::object::TraverseFn<'_>) {
         self.base.traverse(tracer_fn);
         self.bases.traverse(tracer_fn);
         self.mro.traverse(tracer_fn);
         self.subclasses.traverse(tracer_fn);
         self.attributes
             .read_recursive()
             .iter()
             .map(|(_, v)| v.traverse(tracer_fn))
             .count();
         if let Some(ext) = self.heaptype_ext.as_ref() {
             ext.specialization_init.read().traverse(tracer_fn);
+            ext.specialization_getitem.read().traverse(tracer_fn);
         }
     }
@@
         if let Some(ext) = self.heaptype_ext.as_ref()
             && let Some(mut guard) = ext.specialization_init.try_write()
             && let Some(init) = guard.take()
         {
             out.push(init.into());
         }
+        if let Some(ext) = self.heaptype_ext.as_ref()
+            && let Some(mut guard) = ext.specialization_getitem.try_write()
+            && let Some(getitem) = guard.take()
+        {
+            out.push(getitem.into());
+            ext.specialization_getitem_version.store(0, Ordering::Release);
+        }
     }

Also applies to: 266-283

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/type.rs` around lines 236 - 238, HeapTypeExt also
holds a strong PyRef for specialization_getitem which isn’t included in the
manual GC hooks; update the manual traversal and clearing code in the PyType
hooks to also visit and drop this field. In the traversal path (where
specialization_init is traversed) add a call to traverse the
specialization_getitem PyRef (e.g.,
ext.specialization_getitem.read().traverse(tracer_fn)); in the clear path (where
specialization_init is cleared) also clear/take the specialization_getitem PyRef
(e.g., ext.specialization_getitem.write().take() or set to None) so both
specialization_init and specialization_getitem are handled; update both the
block around the existing traverse (the one calling
ext.specialization_init.read().traverse(...)) and the corresponding clear block
mirrored in the other function region (the section covering lines ~266-283).

---

855-890: ⚠️ Potential issue | 🟠 Major

Guard the __getitem__ cache with the type version under the lock.

cache_getitem_for_specialization() still validates tp_version_tag before taking the write lock, and get_cached_getitem_for_specialization() no longer receives the specialization-time type version at all. A concurrent modified() can republish or read a stale cached __getitem__ after the type is re-versioned.

Proposed fix

     pub(crate) fn cache_getitem_for_specialization(
         &self,
         getitem: PyRef<PyFunction>,
         tp_version: u32,
     ) -> bool {
         let Some(ext) = self.heaptype_ext.as_ref() else {
             return false;
         };
-        if tp_version == 0 || self.tp_version_tag.load(Ordering::Acquire) != tp_version {
+        if tp_version == 0 {
             return false;
         }
         let func_version = getitem.get_version_for_current_state();
         if func_version == 0 {
             return false;
         }
-        *ext.specialization_getitem.write() = Some(getitem);
-        ext.specialization_getitem_version
-            .store(func_version, Ordering::Release);
+        let mut guard = ext.specialization_getitem.write();
+        if self.tp_version_tag.load(Ordering::Acquire) != tp_version {
+            return false;
+        }
+        *guard = Some(getitem);
+        ext.specialization_getitem_version.store(func_version, Ordering::Release);
         true
     }
@@
-    pub(crate) fn get_cached_getitem_for_specialization(&self) -> Option<(PyRef<PyFunction>, u32)> {
+    pub(crate) fn get_cached_getitem_for_specialization(
+        &self,
+        tp_version: u32,
+    ) -> Option<(PyRef<PyFunction>, u32)> {
         let ext = self.heaptype_ext.as_ref()?;
-        if self.tp_version_tag.load(Ordering::Acquire) == 0 {
+        if tp_version == 0 {
             return None;
         }
+        let guard = ext.specialization_getitem.read();
+        if self.tp_version_tag.load(Ordering::Acquire) != tp_version {
+            return None;
+        }
         let cached_version = ext.specialization_getitem_version.load(Ordering::Acquire);
         if cached_version == 0 {
             return None;
         }
-        ext.specialization_getitem
-            .read()
-            .as_ref()
-            .map(|getitem| (getitem.to_owned(), cached_version))
+        guard
+            .as_ref()
+            .map(|getitem| (getitem.to_owned(), cached_version))
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/type.rs` around lines 855 - 890,
cache_getitem_for_specialization and get_cached_getitem_for_specialization must
validate the type version while holding the specialization_getitem lock to avoid
races with modified(); change cache_getitem_for_specialization to acquire
ext.specialization_getitem.write() first, then re-check
self.tp_version_tag.load(...) == tp_version before storing the getitem and
version; change get_cached_getitem_for_specialization to acquire
ext.specialization_getitem.read(), then check self.tp_version_tag (or accept a
tp_version param and compare) while holding that read lock and return None if
the tag/version changed or is zero, only returning the cached (getitem,
cached_version) when both the lock-held check and cached_version are valid (use
specialization_getitem and specialization_getitem_version fields as now).