crates/vm/src/stdlib/gc.rs (1)

191-202: Short-circuit get_referrers() when no targets are provided.

At Line 201, this still scans all tracked objects when args is empty, but the result must be empty. An early return avoids an unnecessary heap-wide scan.

Suggested patch

 let targets: HashSet<usize> = args
     .args
     .iter()
     .map(|obj| obj.as_ref() as *const crate::PyObject as usize)
     .collect();

+if targets.is_empty() {
+    return vm.ctx.new_list(Vec::<PyObjectRef>::new());
+}
+
 let mut result = Vec::new();

🤖 Prompt for AI Agents

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

In `@crates/vm/src/stdlib/gc.rs` around lines 191 - 202, get_referrers currently
builds targets and then scans all tracked objects even when args.args is empty;
short-circuit by returning an empty Vec immediately when args.args is empty (or
when targets.is_empty()) before calling gc_state::gc_state().get_objects to
avoid the heap-wide scan—update the get_referrers function to check args (or
targets) and return early.

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

423-430: Extract the repeated “finish collection” block into one shared path.

The branches at Line 423, Line 536, and Line 556 repeat the same duration calculation, stats update, and CollectResult construction. Consolidating this will reduce maintenance drift.

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: 536-543, 556-563

🤖 Prompt for AI Agents

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

In `@crates/vm/src/gc_state.rs` around lines 423 - 430, Branches duplicate the
"finish collection" sequence (compute duration via
start_time.elapsed().as_secs_f64(), call
self.generations[generation].update_stats(...), and return a CollectResult) in
gc_state.rs; refactor by first assigning the differing numeric results
(collected, uncollectable, candidates) into local variables in each branch, then
move the common logic that computes duration, calls
self.generations[generation].update_stats, and constructs/returns the
CollectResult into a single shared path at the end of the function (referencing
start_time, generation, self.generations, update_stats, and CollectResult) so
the repeated block is centralized.

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

527-529: ⚠️ Potential issue | 🟠 Major

Unify generation counter reset bounds in these early-return paths.

Line 527 and Line 547 still use for i in 0..generation, while other return paths (Line 416 and Line 693) use the reset_end logic. This makes post-collection counter resets path-dependent and can skew gc.get_count() behavior after successful collections with no unreachable objects.

Proposed fix

-            for i in 0..generation {
+            let reset_end = if generation >= 2 { 2 } else { generation + 1 };
+            for i in 0..reset_end {
                 self.generations[i].count.store(0, Ordering::SeqCst);
             }

Apply the same change in both blocks (Line 527-529 and Line 547-549).

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: 547-549

🤖 Prompt for AI Agents

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

In `@crates/vm/src/gc_state.rs` around lines 527 - 529, The two early-return
blocks reset per-generation counters using for i in 0..generation which is
inconsistent with other paths that use the shared reset_end logic; replace these
loops to compute the same reset_end value used elsewhere and then run the common
reset loop that sets self.generations[i].count.store(0, Ordering::SeqCst).
Concretely, in the blocks containing for i in 0..generation (the occurrences
around self.generations[i].count.store...), compute reset_end the same way other
branches do (matching the logic used at lines that currently use reset_end) and
then call the shared loop that iterates 0..reset_end to zero the counters so all
return paths use the identical bounds and logic.

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

526-528: ⚠️ Potential issue | 🟠 Major

Fix generation count reset inconsistency in early-return paths.

At Line 526 and Line 546, for i in 0..generation under-resets counts (e.g., generation 0 resets nothing; generation 1 skips gen1). This diverges from the reset_end logic at Line 692 and can leave stale counts.

Suggested patch

         if unreachable.is_empty() {
             drop(gen_locks);
             self.promote_survivors(generation, &survivor_refs);
-            for i in 0..generation {
+            let reset_end = if generation >= 2 { 2 } else { generation + 1 };
+            for i in 0..reset_end {
                 self.generations[i].count.store(0, Ordering::SeqCst);
             }
             let duration = start_time.elapsed().as_secs_f64();
             self.generations[generation].update_stats(0, 0, candidates, duration);
             return CollectResult {
@@
         if unreachable_refs.is_empty() {
             self.promote_survivors(generation, &survivor_refs);
-            for i in 0..generation {
+            let reset_end = if generation >= 2 { 2 } else { generation + 1 };
+            for i in 0..reset_end {
                 self.generations[i].count.store(0, Ordering::SeqCst);
             }
             let duration = start_time.elapsed().as_secs_f64();
             self.generations[generation].update_stats(0, 0, candidates, duration);
             return CollectResult {

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: 546-548

🤖 Prompt for AI Agents

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

In `@crates/vm/src/gc_state.rs` around lines 526 - 528, The loop that zeros
generation counts uses 0..generation which omits the current generation (e.g.,
generation==1 skips generations[1]); change the range to include the current
generation (use 0..=generation or equivalent) so counts for
generations[0..=generation] are cleared; apply the same fix at the other
occurrence (the second 0..generation block) and refactor by computing the
inclusive end index once and calling the shared reset logic on
self.generations[..=end].iter().for_each(|g| g.count.store(0, Ordering::SeqCst))
to avoid duplicated code and match the reset_end behavior.

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

8171-8173: ⚠️ Potential issue | 🟠 Major

Implement specialization_eval_frame_active instead of returning a constant.

Line 8172 hardcodes false, so all !self.specialization_eval_frame_active(vm) guards are effectively always enabled, even when frame-eval/tracing-sensitive paths should fall back.

Suggested fix

-    fn specialization_eval_frame_active(&self, _vm: &VirtualMachine) -> bool {
-        false
+    fn specialization_eval_frame_active(&self, vm: &VirtualMachine) -> bool {
+        vm.use_tracing.get() && !vm.is_none(&self.object.trace.lock())
     }

🤖 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 8171 - 8173, The method
specialization_eval_frame_active currently returns a hardcoded false, which
disables correct guards (see uses like
!self.specialization_eval_frame_active(vm)); replace the constant with the real
runtime check: consult the frame's state and the VM tracing/eval flags (use the
appropriate field or VM accessor that indicates frame-eval/tracing is active)
and return that boolean so callers correctly detect when
frame-eval/tracing-sensitive paths must fall back; update
specialization_eval_frame_active(&self, _vm: &VirtualMachine) to read and return
that real condition rather than false.

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

526-528: ⚠️ Potential issue | 🟡 Minor

Normalize generation-count reset in these early-return branches.

At Line 526 and Line 546, for i in 0..generation diverges from the rest of the function’s reset behavior and can skip resetting generation 0/1 counts in paths where other branches do reset them.

♻️ Proposed fix

-            for i in 0..generation {
+            let reset_end = if generation >= 2 { 2 } else { generation + 1 };
+            for i in 0..reset_end {
                 self.generations[i].count.store(0, Ordering::SeqCst);
             }
@@
-            for i in 0..generation {
+            let reset_end = if generation >= 2 { 2 } else { generation + 1 };
+            for i in 0..reset_end {
                 self.generations[i].count.store(0, Ordering::SeqCst);
             }

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: 546-548

🤖 Prompt for AI Agents

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

In `@crates/vm/src/gc_state.rs` around lines 526 - 528, The early-return branches
use for i in 0..generation which differs from other branches and can omit
resetting generation 0/1; change this to use a single, consistent reset path:
compute the target generation bound once (e.g. let end = generation) and then
reset counts for all generations up through that bound (use 0..=end or a helper
like self.reset_generation_counts(end) that calls
self.generations[i].count.store(0, Ordering::SeqCst) for each i), and replace
the duplicate loops at the spots using generation (the blocks that call
self.generations[i].count.store) with that single common call.

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

8312-8315: ⚠️ Potential issue | 🟠 Major

specialization_eval_frame_active() is still stubbed, so all eval-frame/tracing guards are effectively disabled.

Line 8314 always returns false, which means the new guard checks at Line 3156, Line 3195, Line 5355, Line 7709, and Line 8029 never block specialized execution. This can break tracing/profiling semantics.

Suggested fix

-    fn specialization_eval_frame_active(&self, _vm: &VirtualMachine) -> bool {
-        false
+    fn specialization_eval_frame_active(&self, vm: &VirtualMachine) -> bool {
+        vm.use_tracing.get()
     }

🤖 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 8312 - 8315,
specialization_eval_frame_active currently always returns false, disabling all
eval-frame/tracing guards; implement it to consult the VM's tracing/profiling
state and return true only when eval-frame tracing is actually active.
Specifically, inside specialization_eval_frame_active(&self, vm:
&VirtualMachine) query the appropriate VM flags or methods (e.g.,
vm.is_tracing_enabled(), vm.trace_config().eval_frame_enabled, or the
thread-local/frame-level trace flag used elsewhere) and return that boolean so
the guard checks in the call sites (the specialization guards) correctly
enable/disable specialized execution. Ensure you use the existing VM APIs for
tracing state rather than introducing new globals.