crates/codegen/src/ir.rs (2)

1408-1427: Use instruction_lineno() helper for consistent line number handling.

The next_swappable function extracts line numbers directly from info.location.line.get(), ignoring any lineno_override that may be set (e.g., -1 for cold blocks). The existing instruction_lineno helper at line 3171 handles this correctly.

This inconsistency could cause incorrect line comparisons for instructions with overridden line numbers.

♻️ Proposed fix

         fn next_swappable(instrs: &[InstructionInfo], mut i: usize, lineno: i32) -> Option<usize> {
             loop {
                 i += 1;
                 if i >= instrs.len() {
                     return None;
                 }
                 let info = &instrs[i];
-                let info_lineno = info.location.line.get() as i32;
+                let info_lineno = instruction_lineno(info);
                 if lineno >= 0 && info_lineno > 0 && info_lineno != lineno {
                     return None;
                 }

Note: You'll need to either make instruction_lineno visible here or inline the logic:

let info_lineno = info.lineno_override.unwrap_or_else(|| info.location.line.get() as i32);

🤖 Prompt for AI Agents

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

In `@crates/codegen/src/ir.rs` around lines 1408 - 1427, In next_swappable, the
code reads the instruction line directly from info.location.line.get(), which
ignores any lineno_override; change it to use the existing instruction_lineno
helper (or inline its logic) so comparisons account for overrides (e.g., let
info_lineno = instruction_lineno(info) or use
info.lineno_override.unwrap_or_else(|| info.location.line.get() as i32)); update
next_swappable to call instruction_lineno/info.lineno_override to get
info_lineno before comparing with lineno.

---

1978-1980: Consider avoiding unnecessary clone when no changes occur.

The instructions are cloned unconditionally at line 1978, but the clone is only needed when changed becomes true. For blocks where no LOAD_FAST needs conversion to LOAD_FAST_CHECK, this clone is wasted work.

However, since this runs once during compilation and the optimization benefit is marginal, this is fine as-is.

🤖 Prompt for AI Agents

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

In `@crates/codegen/src/ir.rs` around lines 1978 - 1980, The code unconditionally
clones self.blocks[idx].instructions into old_instructions even when no changes
occur; to fix, iterate over references to self.blocks[idx].instructions (or take
a slice) and only allocate/clone into new_instructions when you detect a needed
change (when encountering a LOAD_FAST that must become LOAD_FAST_CHECK), then
replace or assign the modified Vec back into self.blocks[idx].instructions (or
use std::mem::replace/take) so blocks that are unchanged avoid the clone; update
uses of old_instructions/new_instructions accordingly (refer to
old_instructions, new_instructions, changed, and self.blocks[idx].instructions).

crates/codegen/src/compile.rs (1)

8509-8544: Please add an EX_CALL regression for optimized super() calls.

This introduces a new bytecode shape for super().method(*args, **kwargs) / super(C, self).method(*args). A focused disassembly or behavior test here would make future parity regressions much easier to catch.

🤖 Prompt for AI Agents

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

In `@crates/codegen/src/compile.rs` around lines 8509 - 8544, Add a regression
test that exercises the optimized super() call path (when
can_optimize_super_call returns Some) for both ZeroArg and TwoArg SuperCallType
so we catch the new CALL_FUNCTION_EX shape: construct snippets like
super().method(*args, **kwargs) and super(C, self).method(*args), compile them
and assert the generated bytecode/assembly contains the EX_CALL/CALL_FUNCTION_EX
path (or that uses_ex_call branch was taken) and the sequence produced by
emit_load_super_attr/emit_load_zero_super_attr plus the emitted PushNull and
codegen_call_helper invocation; place the test with other codegen/compile tests
and name it to indicate it’s a regression for optimized super EX_CALLs.

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

2067-2081: Consider removing unreachable else branch.

With ExceptionStack::default() now initializing the stack with vec![None], the stack should never be empty. The debug_assert on line 2070-2073 confirms this invariant, making the else branch (lines 2079-2080) unreachable dead code.

♻️ Suggested simplification

     pub(crate) fn set_exception(&self, exc: Option<PyBaseExceptionRef>) {
         // don't be holding the RefCell guard while __del__ is called
         let mut excs = self.exceptions.borrow_mut();
         debug_assert!(
             !excs.stack.is_empty(),
             "set_exception called with empty exception stack"
         );
-        if let Some(top) = excs.stack.last_mut() {
-            let prev = core::mem::replace(top, exc);
-            drop(excs);
-            drop(prev);
-        } else {
-            excs.stack.push(exc);
-            drop(excs);
-        }
+        let top = excs
+            .stack
+            .last_mut()
+            .expect("set_exception called with empty exception stack");
+        let prev = core::mem::replace(top, exc);
+        drop(excs);
+        drop(prev);
         #[cfg(feature = "threading")]
         thread::update_thread_exception(self.topmost_exception());
     }

🤖 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 2067 - 2081, The else branch in
set_exception is unreachable because ExceptionStack::default() seeds stack with
vec![None] and there's already a debug_assert that the stack is non-empty;
remove the unreachable else branch and simplify set_exception to always operate
on excs.stack.last_mut() (i.e., replace the top entry and drop the previous
value) while preserving the borrow_mut, debug_assert, core::mem::replace usage
and drop(prev) semantics so behavior and lifetimes around __del__ remain
unchanged.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

scripts/compare_bytecode.py (3)

66-98: ⚠️ Potential issue | 🟠 Major

Don't let a failed worker drop its entire file set.

When _finish_one() hits a timeout or JSON error, it returns {}. finish_dump() then loses every file assigned to that worker, so those paths never reach all_files; the report can undercount coverage and still exit successfully after RustPython skipped work. Carry the worker's relpaths alongside (proc, files_file) and synthesize per-file error entries on timeout, non-zero exit, or JSON parse failure instead of returning an empty mapping.

Also applies to: 103-148

🤖 Prompt for AI Agents

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

In `@scripts/compare_bytecode.py` around lines 66 - 98, The worker currently
returns an empty mapping on timeout/JSON errors causing finish_dump to drop all
files for that worker; modify _start_one to return the relpath list alongside
(proc, files_file) (e.g., return (proc, files_file, relpaths)) so callers have
the assigned paths, and update _finish_one/finish_dump to, on timeout, non-zero
exit code, or JSON parse failure, synthesize per-file error entries for each
relpath (marking failure/reason) instead of returning {} and ensure those
synthesized entries are merged into all_files so no files are lost from the
final report; update any code unpacking the tuple to expect the extra relpaths
element.

---

342-342: ⚠️ Potential issue | 🟡 Minor

Replace the named lambdas with local defs.

Both assignments still trip Ruff/Flake8 E731. Plain local functions keep the same behavior without failing lint.

Suggested fix

-    log = lambda *a, **kw: print(*a, file=sys.stderr, **kw)
+    def log(*a, **kw):
+        print(*a, file=sys.stderr, **kw)
@@
-        p = lambda *a: print(*a, file=out)
+        def p(*a):
+            print(*a, file=out)

As per coding guidelines, "Use ruff for linting Python code".

Also applies to: 424-424

🤖 Prompt for AI Agents

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

In `@scripts/compare_bytecode.py` at line 342, The code assigns a named lambda to
a variable (e.g., the lambda assigned to "log" and a similar named-lambda
later), which triggers Ruff/Flake8 E731; replace each named lambda with an
equivalent local def (for example, define def log(*a, **kw): print(*a,
file=sys.stderr, **kw)) so behavior remains identical but lint passes, and
update any callers to use the new local function name.

---

59-60: ⚠️ Potential issue | 🟡 Minor

Normalize the filter path before calling fnmatch().

Line 59 produces OS-native separators, so on Windows a documented filter like asyncio/*.py will not match. Compare against a POSIX-normalized copy and keep relpath unchanged for reporting.

Suggested fix

             fpath = os.path.join(root, fname)
             relpath = os.path.relpath(fpath, lib_dir)
-            if pattern and not fnmatch.fnmatch(relpath, pattern):
+            match_path = relpath.replace(os.sep, "/")
+            if pattern and not fnmatch.fnmatch(match_path, pattern):
                 continue
             targets.append((relpath, fpath))

🤖 Prompt for AI Agents

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

In `@scripts/compare_bytecode.py` around lines 59 - 60, The fnmatch check uses
OS-native relpath which on Windows uses backslashes, so normalize a POSIX-style
copy for matching and keep relpath unchanged for reporting: create a
POSIX-normalized variant (e.g., via pathlib.PurePosixPath(relpath).as_posix() or
relpath.replace(os.sep, "/")) and call fnmatch.fnmatch(posix_relpath, pattern)
instead of fnmatch.fnmatch(relpath, pattern) in the block that contains relpath
and pattern.

crates/codegen/src/ir.rs (1)

1543-1543: Use alloc or crate-level collections for consistency.

This function uses std::collections::HashMap while the rest of the file uses alloc::collections (line 1 imports VecDeque from alloc). This inconsistency could cause issues if the crate needs no_std compatibility.

Consider using a local Vec or the crate's IndexMap instead, which would also maintain insertion order (though not strictly needed here).

♻️ Suggested alternative using a simple Vec scan

                 if run_end - run_start >= 2 {
-                    // Pass 1: find the LAST occurrence of each variable
-                    let mut last_occurrence = std::collections::HashMap::new();
-                    for (j, instr) in instrs[run_start..run_end].iter().enumerate() {
-                        last_occurrence.insert(u32::from(instr.arg), j);
-                    }
-                    // Pass 2: non-last stores to the same variable are dead
-                    for (j, instr) in instrs[run_start..run_end].iter_mut().enumerate() {
-                        let idx = u32::from(instr.arg);
-                        if last_occurrence[&idx] != j {
+                    // Pass 1: find which positions are NOT the last occurrence
+                    let run_len = run_end - run_start;
+                    let mut is_dead = vec![false; run_len];
+                    for j in 0..run_len {
+                        let idx = u32::from(instrs[run_start + j].arg);
+                        // Mark earlier occurrences as dead
+                        for k in 0..j {
+                            if u32::from(instrs[run_start + k].arg) == idx {
+                                is_dead[k] = true;
+                            }
+                        }
+                    }
+                    // Pass 2: replace dead stores with POP_TOP
+                    for (j, &dead) in is_dead.iter().enumerate() {
+                        if dead {
+                            let instr = &mut instrs[run_start + j];
                             instr.instr = AnyInstruction::Real(Instruction::PopTop);
                             instr.arg = OpArg::new(0);
                         }
                     }
                 }

🤖 Prompt for AI Agents

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

In `@crates/codegen/src/ir.rs` at line 1543, Replace the std::collections::HashMap
usage for last_occurrence with an alloc-friendly structure: either use a simple
Vec<(KeyType, usize)> named last_occurrence and do a small linear search/update
where you currently insert/lookup, or switch to a crate-level map like
indexmap::IndexMap (or hashbrown::HashMap) if you need faster lookups; update
all uses of last_occurrence (creation and lookup/insert sites) in the
surrounding function so they operate on the chosen structure to preserve
no_std/alloc compatibility and maintain correct semantics.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes