Update opcode, dis from 3.14.2#6869
Conversation
📝 WalkthroughWalkthroughThis PR centralizes Resume emission into a new Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Sorry, something went wrong.
12b9e0e to
cd978d9
Compare
January 25, 2026 11:45
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@crates/codegen/src/compile.rs`:
- Around line 1145-1187: emit_resume_for_scope currently resets
self.current_source_range to TextRange::default() before emitting RESUME, so
non-module scopes lose the actual definition line; update the call site (where
current_source_range is computed) to pass the real definition line into
emit_resume_for_scope (e.g., add a lineno: Option<u32> parameter) and inside
emit_resume_for_scope set lineno_override = Some(lineno) for non-module scopes
(or, alternatively, restore/populate self.current_source_range from that
definition range before emitting), ensuring the RESUME entry uses the correct
lineno; modify the emit_resume_for_scope signature and its callers accordingly
(refer to emit_resume_for_scope, current_source_range, and the RESUME
instruction emission).
In `@crates/codegen/src/ir.rs`:
- Around line 226-229: When handling a module-scope RESUME with lineno_override
== Some(0), you must also clear end_line and any column information so the
resulting LineTableLocation represents "no line" (line 0, no columns) per
CPython 3.13+ semantics. Update the code that constructs or pushes into
linetable_locations (and any related use of instructions/locations) to, when
lineno_override == Some(0), set start_line and end_line to 0 and clear/reset
start_col/end_col (or their equivalent fields in LineTableLocation) so no column
data is emitted for that entry.
In `@crates/stdlib/src/_opcode.rs`:
- Around line 328-343: The get_common_constants function currently swallows all
errors when calling vm.builtins.get_attr("all"/"any") via unwrap_or_else(|_|
vm.ctx.none()), which masks allocation/internal errors and diverges from frame
opcode behavior; change the signature of get_common_constants to return
PyResult<Vec<PyObjectRef>> (instead of Vec<...>) and propagate get_attr errors
with the ? operator so failures bubble up, or if the intent is to only treat
missing builtins as None explicitly, replace unwrap_or_else with .ok() and map
AttributeError to vm.ctx.none while still propagating other errors; update call
sites to handle the new PyResult as needed and reference get_common_constants,
get_attr, vm.builtins, vm.ctx.none, and VirtualMachine when making the change.
🧹 Nitpick comments (1)
crates/codegen/src/ir.rs (1)
171-186: Optimization helpers and finalize_code integration look good.The fold/convert/cleanup helpers keep the optimization pipeline readable. Operational note: after bytecode instruction changes, run a full clean build and remove any
.pycartifacts to avoid stale caches. Based on learnings, please runrm -r target/debug/build/rustpython-*and delete any.pycfiles.Also applies to: 444-521, 587-617, 619-679, 681-688
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/codegen/src/compile.rs`:
- Around line 1152-1188: The call to enter_scope is passing a byte-offset
(expr.range().start().to_u32()) which enter_scope treats as a line number and
corrupts the line table; locate the call site where enter_scope(...) is invoked
with expr.range().start().to_u32() (around the code that handles expression
scopes) and replace that argument with the actual source line number:
self.get_source_line_number().get().to_u32(), so enter_scope and downstream
SourceLocation creation receive a proper line number consistent with other
callers.
♻️ Duplicate comments (1)
crates/codegen/src/ir.rs (1)
335-344: Override end_line and columns when lineno_override is set to 0.When
lineno_override = Some(0)is set for module-scope RESUME, onlylineis overridden to 0 whileend_line,col, andend_colretain the actual source location. Per CPython 3.13+ semantics, line 0 represents "no line" (artificial instructions) and should have no meaningful column information.🔧 Suggested fix
- let lt_loc = LineTableLocation { - line: info - .lineno_override - .unwrap_or_else(|| info.location.line.get() as i32), - end_line: info.end_location.line.get() as i32, - col: info.location.character_offset.to_zero_indexed() as i32, - end_col: info.end_location.character_offset.to_zero_indexed() as i32, - }; + let line_override = info.lineno_override; + let line = line_override.unwrap_or_else(|| info.location.line.get() as i32); + let (end_line, col, end_col) = if line_override == Some(0) { + // Line 0 = no location info per CPython semantics + (0, 0, 0) + } else { + ( + info.end_location.line.get() as i32, + info.location.character_offset.to_zero_indexed() as i32, + info.end_location.character_offset.to_zero_indexed() as i32, + ) + }; + let lt_loc = LineTableLocation { line, end_line, col, end_col };
Sorry, something went wrong.
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [+] lib: cpython/Lib/dis.py dependencies:
dependent tests: (49 tests)
[+] lib: cpython/Lib/opcode.py dependencies:
dependent tests: (34 tests)
[+] test: cpython/Lib/test/test_compile.py dependencies: dependent tests: (no tests depend on compile) [+] test: cpython/Lib/test/test_peepholer.py dependencies: dependent tests: (no tests depend on peepholer) Legend:
|
Sorry, something went wrong.
93b26bf
into
RustPython:main
Jan 26, 2026
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.