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

476-482: The singleton fast path still materializes a temporary PyStr for borrowed inputs.

For call sites like new_str(" "), the s.into() happens before the Latin-1 check, so the cache hit only skips the final object allocation. If you want this path to be allocation-free for single-character borrowed inputs, it probably needs a borrowed-input fast path or a dedicated new_char/latin1_char entry point at the API boundary.

🤖 Prompt for AI Agents

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

In `@crates/vm/src/vm/context.rs` around lines 476 - 482, new_str currently calls
s.into() before checking latin1_singleton_index, which materializes a temporary
PyStr for borrowed inputs and defeats the allocation-free singleton fast path;
change the implementation to avoid calling s.into() until after the Latin‑1
single-char check by either adding an overload/new API that accepts a borrowed
&str (or &impl AsRef<str>) and runs Self::latin1_singleton_index(&s) first, or
add a dedicated API like new_char/latin1_char used at the API boundary for
single-character inputs and have new_str route to it only after converting
non-borrowed inputs; ensure latin1_singleton_index is invoked on the borrowed
string and that you only call s.into() when the singleton fast path fails so no
temporary PyStr is allocated for borrowed single-char inputs.

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

4089-4098: Extend this specialized char fast path beyond ASCII if the new cache is Latin-1-wide.

BinaryOpSubscrStrInt only hits ascii_char_cache, so one-character subscripts in '\x80'..='\xff' still bounce to the generic path even after quickening. If the new singleton cache is meant to cover all Latin-1 characters, wiring it in here would avoid that steady-state miss.

🤖 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 4089 - 4098, The specialized fast path
in BinaryOpSubscrStrInt currently only uses vm.ctx.ascii_char_cache for one-char
string subscripts, causing Latin-1 (0x80..0xFF) characters to miss the cache;
update the path after obtaining ch (and ascii_idx) to branch: if ch.is_ascii()
continue using vm.ctx.ascii_char_cache[ascii_idx], else index into the new
Latin-1 singleton cache (e.g., vm.ctx.latin1_char_cache[ascii_idx] or
vm.ctx.single_char_cache[ascii_idx], whichever your context defines) and push
that clone.into(); keep the same guards
(specialization_nonnegative_compact_index, getitem_by_index) and ensure
ascii_idx is computed from ch.to_u32() and bounds-checked for 0..256 before
indexing the Latin-1 cache.

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

7794-7798: ⚠️ Potential issue | 🟠 Major

Cached __getitem__ still needs a type-version guard.

Specializing the opcode here is still unsafe because the execution path at Lines 4043-4049 reads get_cached_getitem_for_specialization() without first confirming owner.class().tp_version_tag.load(Acquire) == cached_version. Rebinding __getitem__ on a heap type can keep this fast path calling stale code.

🤖 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 7794 - 7798, The cached __getitem__
specialization is used without verifying the class type-version, so add a
type-version guard: when installing/using the specialization via
cls.cache_getitem_for_specialization and when reading via
get_cached_getitem_for_specialization, compare
owner.class().tp_version_tag.load(Acquire) against the cached_version before
taking the fast path; if they differ, fall back to the slow path and refresh the
cache (or invalidate the cached handler) so rebinding __getitem__ on a heap type
cannot call stale code.

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

275-281: ⚠️ Potential issue | 🟠 Major

Version the cached __init__ entry, not just the owning type.

tp_version_tag only changes when the type mutates. PyFunction::func_version can change independently via __code__, __defaults__, or __kwdefaults__, but get_cached_init_for_specialization() still reuses the cached function on a type-version match alone. That leaves the CALL_ALLOC_AND_ENTER_INIT fast path free to keep driving prepare_exact_args_frame() with an __init__ that no longer satisfies the cached exact-args contract.

Suggested direction

 pub struct TypeSpecializationCache {
     pub init: PyAtomicRef<Option<PyFunction>>,
+    pub init_version: AtomicU32,
     pub getitem: PyAtomicRef<Option<PyFunction>>,
     pub getitem_version: AtomicU32,
@@
         Self {
             init: PyAtomicRef::from(None::<PyRef<PyFunction>>),
+            init_version: AtomicU32::new(0),
             getitem: PyAtomicRef::from(None::<PyRef<PyFunction>>),
             getitem_version: AtomicU32::new(0),
             write_lock: PyMutex::new(()),
             retired: PyRwLock::new(Vec::new()),
         }
@@
     pub(crate) fn cache_init_for_specialization(
         &self,
         init: PyRef<PyFunction>,
         tp_version: u32,
         vm: &VirtualMachine,
     ) -> bool {
+        let func_version = init.get_version_for_current_state();
+        if func_version == 0 {
+            return false;
+        }
         ...
         ext.specialization_cache.swap_init(Some(init), Some(vm));
+        ext.specialization_cache
+            .init_version
+            .store(func_version, Ordering::Relaxed);
         true
     }

You'll also need the read side and CALL_ALLOC_AND_ENTER_INIT caller to validate init_version before reusing the cached function.

Also applies to: 893-931

🤖 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 275 - 281, The cached __init__
entry must be versioned separately from the owning type: add an init_version
(AtomicU32) to TypeSpecializationCache and when caching a PyFunction in
get_cached_init_for_specialization() store the function's current
PyFunction::func_version into init_version; on read side (and in
CALL_ALLOC_AND_ENTER_INIT / prepare_exact_args_frame) validate that both the
cached type version (getitem_version or type's tp_version_tag) and init_version
match the live PyFunction::func_version before reusing the cached PyFunction;
update cache write paths to set init and init_version together and ensure
callers reject the cached init if the versions differ so that
prepare_exact_args_frame() never runs with a stale __init__.

---

303-315: ⚠️ Potential issue | 🟠 Major

retired still grows for every live-type invalidation.

modified() always reaches invalidate_for_type_modified() without a VirtualMachine, so both swap_init(None, None) and swap_getitem(None, None) fall back to pushing the replaced functions into retired. clear_into() is the only drain path here, so a long-lived heap type that alternates specialization and attribute writes will retain those strong refs indefinitely.

Also applies to: 318-336, 352-364

🤖 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 303 - 315, The code currently
always moves replaced function objects into the global `retired` list when `vm`
is None (in `swap_init` and `swap_getitem`), causing `retired` to grow for
long‑lived heap types; change both `swap_init` and `swap_getitem` so that when
`vm` is None they do NOT push the old value into `retired` but instead release
the strong reference immediately (or convert to a non‑retaining/weak form) to
avoid accumulation — implement this by extracting the old value via the existing
`self.init.swap(new_init)` path but then dropping or turning it into a
non‑retaining placeholder via a new helper (e.g. `drop_old_without_retire`)
rather than calling `retire_old_function`; keep the current `vm`-present path
that uses `self.init.swap_to_temporary_refs(..., vm)` and `retire_old_function`
unchanged so CPython-style temporary refs remain for executing frames.

.cspell.json (1)

63-64: Minor: words not in alphabetical order.

"compactlong" and "compactlongs" start with "c" but are placed after "d" words. For consistency with the rest of the list, consider moving them to the correct alphabetical position (after "coro").

🤖 Prompt for AI Agents

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

In @.cspell.json around lines 63 - 64, The two dictionary entries "compactlong"
and "compactlongs" are out of alphabetical order; move those two entries so they
appear after the "coro" entry (i.e., in the block of "c" words) to maintain
consistent alphabetical ordering in the word list.

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

424-428: Give the shim its own synthetic code identity.

Using __init__ for source_path, obj_name, and qualname will make tracebacks and code introspection look like user-defined __init__ bytecode even when the frame is just internal cleanup. A dedicated internal label such as <init_cleanup> would avoid that confusion.

🤖 Prompt for AI Agents

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

In `@crates/vm/src/vm/context.rs` around lines 424 - 428, The frame/context
currently reuses names.__init__ for source_path, obj_name, and qualname, which
makes internal cleanup frames appear as user __init__ code; change those fields
(source_path, obj_name, qualname) to a dedicated synthetic identifier like
"<init_cleanup>" when constructing this shim frame (where names.__init__ is
currently used) so tracebacks and introspection show an internal label instead
of a user __init__; keep other fields (e.g. first_line_number, max_stackdepth)
as-is or set a fixed sane value if needed.

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

4044-4051: ⚠️ Potential issue | 🟠 Major

Guard cached __getitem__ dispatch with the current type version.

This path still trusts get_cached_getitem_for_specialization() without checking owner.class().tp_version_tag. After rebinding __getitem__ on a heap type, the cached function can still pass the func_version() check and dispatch stale code here. Please mirror the __init__ cache pattern and treat a type-version mismatch as a cache miss.

🤖 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 4044 - 4051, The cached __getitem__
dispatch in Instruction::BinaryOpSubscrGetitem currently trusts
get_cached_getitem_for_specialization() and func.func_version() but misses
checking the owning class's tp_version_tag; update the guard to also compare
owner.class().tp_version_tag against the cached type version (mirror the
__init__ cache pattern) and treat a mismatch as a cache miss so the fast-path is
skipped and normal lookup occurs; locate the check around self.nth_value(1),
specialization_eval_frame_active(vm), get_cached_getitem_for_specialization(),
func.func_version(), and specialization_has_datastack_space_for_func(vm, &func)
and add the tp_version_tag validation there.

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

281-281: ⚠️ Potential issue | 🟠 Major

retired still has no live reclamation path.

invalidate_for_type_modified() always goes through swap_*(_, None), so every live invalidation keeps the displaced specialization function strongly referenced in retired. Since retired is only drained in clear_into(), long-lived heap types can still grow this list indefinitely under specialize/mutate cycles.

Please route live invalidations through a VM-backed temporary-ref path when a VirtualMachine is available, or add an opportunistic drain for retired.

Also applies to: 303-336, 352-364

🤖 Prompt for AI Agents

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

In `@crates/vm/src/builtins/type.rs` at line 281, The retired field (retired:
PyRwLock<Vec<PyObjectRef>>) accumulates strong references because
invalidate_for_type_modified() always calls swap_*(_, None) which pushes
displaced specialization functions into retired and they are only drained by
clear_into(), causing unbounded growth for long-lived heap types; fix by routing
live invalidations through a VM-backed temporary-ref path when a VirtualMachine
is available (use the VM to create transient/weak refs instead of pushing into
retired) or add an opportunistic drain that periodically clears retired (e.g.,
in invalidate_for_type_modified(), swap_* callers, or type mutation paths) so
retired does not retain references indefinitely—update functions
invalidate_for_type_modified(), any swap_* call sites that pass None, and
clear_into() to implement the chosen reclamation strategy.

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

1566-1620: Please pin the untraced contract with a regression test.

with_frame_untraced() now deliberately bypasses dispatch_traced_frame(). A focused test that sys.settrace / sys.setprofile do not observe this path would help prevent accidental hook reintroduction in later frame refactors.

🤖 Prompt for AI Agents

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

In `@crates/vm/src/vm/mod.rs` around lines 1566 - 1620, Add a regression test that
verifies with_frame_untraced()/with_frame_impl(..., traced: false, ...) bypasses
dispatch_traced_frame: register sys.settrace and sys.setprofile handlers that
will fail the test if invoked (or toggle a flag), create a FrameRef and call
with_frame_untraced (or call with_frame_impl with traced=false) to execute a
dummy closure, then assert the trace/profile handlers were not called and normal
frame cleanup occurs; reference with_frame_untraced, with_frame_impl, and
dispatch_traced_frame so the test covers the untraced execution path explicitly.

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

424-428: Give the synthetic cleanup code a distinct internal identity.

Reusing __init__ for source_path, obj_name, and qualname will make any leaked traceback or debug output look like user __init__ code. A dedicated internal name/path would make cleanup-frame failures much easier to diagnose.

🤖 Prompt for AI Agents

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

In `@crates/vm/src/vm/context.rs` around lines 424 - 428, The synthetic cleanup
frame currently reuses names.__init__ for source_path, obj_name, and qualname
which can mask real user __init__ frames; change the synthetic frame to use a
distinct internal identity (e.g. a constant like INTERNAL_CLEANUP_NAME such as
"<vm_cleanup>" or "<internal_cleanup>") and assign that to source_path,
obj_name, and qualname instead of names.__init__; update any creation site that
sets first_line_number/max_stackdepth on the synthetic cleanup frame (the code
assigning source_path, obj_name, qualname in the synthetic frame construction)
so all three fields use the new internal name to make leaked tracebacks clearly
identifiable.

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

4141-4155: Use the latin-1 singleton path here too.

This fast path still only interns ASCII results. Single-character latin-1 hits like "\xE9"[0] will fall straight back to the generic subscript path, so the new singleton caching work does not apply on this opcode yet.

🤖 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 4141 - 4155, The BinaryOpSubscrStrInt
fast path only handles ASCII single-character results via
vm.ctx.ascii_char_cache, so single-character Latin-1 results (e.g. "\xE9"[0])
bypass the new singleton cache; update the Instruction::BinaryOpSubscrStrInt
branch to, after computing ch (and before falling back), check for a Latin‑1
single-character (e.g. a method like ch.is_latin1() or checking ch.to_u32() in
0x80..=0xFF), and on hit pop the two operands and push the corresponding Latin‑1
singleton from the VM context (analogous to vm.ctx.ascii_char_cache, e.g.
vm.ctx.latin1_char_singleton[index].clone().into()); keep the existing ASCII
path and fallback unchanged.

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

7752-7754: Extract the shared BinaryOpExtend fallback.

The same binary_op_extended_specialization() / cached_extend_descr / Instruction::BinaryOpExtend block is repeated across multiple arms. Pulling that into one helper or a single trailing fallback will make future opcode coverage changes much less error-prone.

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: 7768-7770, 7784-7786, 7792-7794, 7901-7904

🤖 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 7752 - 7754, Multiple match arms repeat
the same pattern calling binary_op_extended_specialization(op, a, b, vm),
setting cached_extend_descr, and returning Instruction::BinaryOpExtend; extract
that shared fallback into one helper or a single trailing fallback to remove
duplication. Locate uses of binary_op_extended_specialization,
cached_extend_descr, and Instruction::BinaryOpExtend in the match (e.g., the
arms around the occurrences at 7752, 7768, 7784, 7792, 7901) and refactor so you
first compute the differing value(s) (op/a/b or the condition), then call a
single helper function (or a final else branch) that runs
binary_op_extended_specialization(op, a, b, vm), assigns cached_extend_descr =
Some(descr) when present, and returns Instruction::BinaryOpExtend; keep the
behavior identical but centralize the common logic.

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

3732-3754: ⚠️ Potential issue | 🟠 Major

Guard LoadAttrModule on module-dict state, not just tp_version_tag.

Line 3743 still goes through PyModule::get_attr(), but the specialization installed at Lines 7388-7400 only guards on the module type version. If the module dict later mutates (__getattr__ added, the attribute removed, lookup starts raising), the specialized path can perform module resolution and then immediately fall back to load_attr_slow(), duplicating user-visible side effects. Cache module-dict state as part of the guard, or make the specialized handler do a direct dict lookup and fall back only on a true miss.

Also applies to: 7385-7401

🤖 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 3732 - 3754, The fast-path for
Instruction::LoadAttrModule uses only tp_version_tag to guard
(owner.downcast_ref_if_exact::<PyModule>, type_version from
code.instructions.read_cache_u32) but still calls PyModule::get_attr, risking
duplicate side effects; change the specialization to also cache and compare the
module dict state (e.g. a dict version/tag or identity pointer) and perform a
direct dict lookup for attr_name (avoiding PyModule::get_attr) so you only fall
back to load_attr_slow(vm, oparg) on a real miss or when the dict/version check
fails; apply the same change to the duplicate specialization region referenced
(around the other LoadAttrModule handler).

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

1179-1253: The mixed float/int extension still skips the inplace operators.

BinaryOpExtend now covers Add/Subtract/Multiply/TrueDivide, but InplaceAdd/InplaceSubtract/InplaceMultiply/InplaceTrueDivide still never route through binary_op_extended_specialization. For immutable floats that leaves x += 1, x -= 1, x *= 1, and x /= 1 on the generic path even though they can reuse the same specialization.

Also applies to: 7752-7795, 7901-7904

🤖 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 1179 - 1253, The float/int mixed
specializations omit the inplace operators, so inplace variants never hit
binary_op_extended_specialization; update the BinaryOpExtendSpecializationDescr
tables (e.g., BINARY_OP_EXTEND_DESCRS) to add entries mapping
bytecode::BinaryOperator::InplaceAdd, InplaceSubtract, InplaceMultiply and
InplaceTrueDivide to the same guards and actions used for
Add/Subtract/Multiply/TrueDivide (e.g., map InplaceAdd ->
float_compactlong_guard/float_compactlong_add and InplaceTrueDivide ->
nonzero_float_compactlong_guard/float_compactlong_true_div, and similarly for
compactlong_float_* entries), and apply the same additions to the other
identical descriptor arrays mentioned (the other occurrences around the
indicated ranges) so inplace ops reuse the existing specializations.

---

4160-4174: Use the Latin-1 singleton path in the specialized str[int] fast path too.

This branch still only interns ASCII characters. Repeated subscripting that returns a single-byte non-ASCII Latin-1 character will keep missing the specialized result path and fall back to generic str.__getitem__, even though this PR adds a Latin-1 singleton cache for that case.

🤖 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 4160 - 4174, The fast path in
Instruction::BinaryOpSubscrStrInt only uses the ascii_char_cache; extend it to
also handle Latin-1 single-byte non-ASCII characters by checking the character
returned from PyStr::getitem_by_index for a Latin-1 single-byte (e.g.,
ch.is_latin1() or equivalent) and, when true, pop the two values and push the
cached singleton from the Latin-1 cache (e.g.,
vm.ctx.latin1_char_cache[ch.to_u32() as usize].clone().into()) just like the
existing ascii branch; keep the existing ascii branch unchanged and add the
Latin-1 branch alongside it after getitem_by_index succeeds.

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

345-350: Consider using .for_each() instead of .map().count().

The current pattern works, but .for_each() better expresses the intent of executing side effects without producing values.

Suggested change

-        self.retired
-            .read()
-            .iter()
-            .map(|obj| obj.traverse(tracer_fn))
-            .count();
+        self.retired
+            .read()
+            .iter()
+            .for_each(|obj| obj.traverse(tracer_fn));

🤖 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 345 - 350, The code iterates
over self.retired.read().iter() and calls obj.traverse(tracer_fn) but uses
.map(...).count() to execute side effects; replace that with .for_each(...) to
convey intent more clearly. Locate the block using
self.retired.read().iter().map(|obj| obj.traverse(tracer_fn)).count() and change
it to call .for_each(|obj| obj.traverse(tracer_fn)) so traverse(tracer_fn) runs
for each retired object without generating/consuming a dummy count value.