Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/codegen/src/ir.rs`:
- Around line 1429-1431: Rename the local variable instrs to a non-spell-checked
name (e.g., instructions) to avoid the spell-check warning: in the loop over
self.blocks where you currently bind let instrs = &mut block.instructions;
change that binding to let instructions = &mut block.instructions; and update
all subsequent uses in this scope (same pattern as in
optimize_build_tuple_unpack) so references to instrs are replaced with
instructions.
- Around line 1514-1516: Rename the local variable named `instrs` to
`instructions` in the loop that iterates `for block in &mut self.blocks` (the
binding currently `let instrs = &mut block.instructions;`), and update all
references within that loop's scope to use `instructions` instead of `instrs` so
the spell checker no longer flags the abbreviation; leave `block.instructions`
unchanged.
- Around line 1345-1352: The local variable name `instrs` (declared as `let
instrs = &mut block.instructions;`) must be renamed to `instructions` to satisfy
the spell checker; update the binding and every use in this scope (e.g.,
`instrs.len()`, `instrs[i]`, `instrs[i + 1]`) to `instructions`, keeping the
same reference to `block.instructions` and preserving the surrounding logic that
checks `Instruction::BuildTuple` and `Instruction::UnpackSequence`.
In `@scripts/compare_bytecode.py`:
- Around line 103-127: The current _finish_one() swallows worker failures by
returning {} on timeout/parse errors and unlinking files_file so finish_dump()
loses those file paths; change _finish_one() to preserve the assigned file
entries on error by reading the files list from files_file before unlinking and
returning a fallback mapping (e.g., each filename -> [] or a special failure
marker) instead of {}, and ensure this path is taken for
subprocess.TimeoutExpired, non-zero proc.returncode, and JSONDecodeError cases
so finish_dump() receives the original file set; use the existing files_file and
proc symbols to locate and implement reading the file list and returning the
appropriate fallback result.
- Around line 59-61: The relative path used for filtering (relpath from
os.path.relpath(fpath, lib_dir)) may contain Windows backslashes and thus fail
fnmatch.fnmatch(pattern) checks; normalize relpath to POSIX separators (e.g. use
pathlib.Path(relpath).as_posix() or replace backslashes with '/') before calling
fnmatch.fnmatch(relpath, pattern) so patterns like "asyncio/*.py" match
cross-platform.
- Line 342: Replace the inline lambda assignments with proper function
definitions: change the assignment "log = lambda *a, **kw: print(*a,
file=sys.stderr, **kw)" to a def-based implementation (e.g., def log(*a, **kw):
print(*a, file=sys.stderr, **kw)) and do the same for the other lambda
assignment mentioned in the comment (the second lambda at line 424) so both
named callables are defined with def instead of lambdas to satisfy E731; keep
the same signature and behavior when converting to the new functions.
In `@scripts/dis_dump.py`:
- Around line 116-117: The _RP_CMP_OPS dict is incorrect and contains an
extraneous key 6; update the mapping for the COMPARE_OP discriminants in the
top-level symbol _RP_CMP_OPS to match RustPython's ComparisonOperator (0→"<",
1→"<=", 2→"==", 3→"!=", 4→">", 5→">="), removing the invalid key 6 so downstream
COMPARE_OP output is labelled correctly.
- Around line 134-141: The branch that treats deref-like opnames (the elif
listing
"LOAD_DEREF","STORE_DEREF","DELETE_DEREF","LOAD_CLOSURE","MAKE_CELL","COPY_FREE_VARS")
incorrectly treats COPY_FREE_VARS as a name/index into localsplus; remove
"COPY_FREE_VARS" from that tuple (or add an explicit case for COPY_FREE_VARS
before this branch) so its operand is always rendered as a numeric count rather
than passed through the deref/name fallback; update any comments/tests that
assume COPY_FREE_VARS is numeric.
---
Outside diff comments:
In `@crates/codegen/src/compile.rs`:
- Around line 3958-3987: The SetupFinally emission and
push_fblock_full(FBlockType::FinallyTry, ...) are being emitted before
switch_to_block(else_block) so the finally setup attaches to the current
(cleanup) block; move the emit!(..., PseudoInstruction::SetupFinally { delta:
finally_block }) and the self.push_fblock_full(FBlockType::FinallyTry,
finally_block, finally_block, FBlockDatum::FinallyBody(finalbody.to_vec())) call
to immediately after self.switch_to_block(else_block) and before
self.compile_statements(orelse) so the finally is armed on else_block; keep the
existing PopBlock emit and self.pop_fblock(FBlockType::FinallyTry) after
compiling orelse to unwind the finally for the else path.
---
Nitpick comments:
In `@crates/codegen/src/compile.rs`:
- Around line 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.
In `@crates/codegen/src/ir.rs`:
- Around line 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.
- Around line 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).
In `@crates/vm/src/vm/mod.rs`:
- Around line 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.