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 |
youknowone
force-pushed
the
opcode
branch
2 times, most recently
from
12b9e0e to
cd978d9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
| let mut instructions = Vec::new(); | ||
| let mut locations = Vec::new(); | ||
| let mut linetable_locations: Vec<LineTableLocation> = Vec::new(); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and the specific lines mentioned
cd crates/codegen/src && wc -l ir.rsRepository: RustPython/RustPython
Length of output: 75
🏁 Script executed:
# Examine the code at lines 226-229
sed -n '220,235p' ir.rsRepository: RustPython/RustPython
Length of output: 113
🏁 Script executed:
# Check the other locations mentioned: 335-345, 360-368, 850-901
echo "=== Lines 335-345 ==="
sed -n '330,350p' ir.rs
echo "=== Lines 360-368 ==="
sed -n '355,373p' ir.rs
echo "=== Lines 850-901 ==="
sed -n '845,905p' ir.rsRepository: RustPython/RustPython
Length of output: 294
🏁 Script executed:
# Search for lineno_override and related structures to understand the context
rg -B5 -A5 "lineno_override" ir.rsRepository: RustPython/RustPython
Length of output: 110
🏁 Script executed:
# Search for LineTableLocation struct definition
rg "struct LineTableLocation" ir.rs -A10Repository: RustPython/RustPython
Length of output: 110
🌐 Web query:
CPython 3.13 3.14 line-0 RESUME instruction linetable encoding
💡 Result:
In CPython 3.13 and 3.14, the source-location info for each bytecode instruction (including the initial RESUME) is stored in code.co_linetable as a stream of location entries. Each entry starts with a header byte 1 cccc lll (MSB set), where cccc is the location kind (0–15) and lll is (length_in_code_units - 1); “code units” are 2-byte units (i.e., one wordcode instruction is length 1). The rest of the entry is kind-specific, using 6-bit-chunk varint/svarint encodings. [1][2]
Why RESUME can show line 0
The start line stored in the first entry is encoded as a delta from co_firstlineno (and subsequent entries delta from the previous start line). So if a code object has co_firstlineno == 0, then RESUME can legitimately decode to line 0 via a delta of 0. [1][2]
(Separately, PEP 626 notes that frames for modules start with f_lineno = 0, and that some bytecodes may be “artificial” and have no meaningful line. Tools should prefer co_lines() / co_positions() rather than assuming the first executable line is co_firstlineno + 1. [3])
The actual encoding you’ll typically see for “line 0 RESUME”
Most commonly, RESUME’s entry uses Kind 13 (“No column info”): it stores start/end line deltas but sets columns to None. [1][2]
For a single-instruction RESUME whose decoded line is 0 (i.e., first entry, start-line delta = 0, end-line delta = 0):
- header byte: kind=13, length=1 →
1 1101 000=0xE8 - start-line delta (svarint 0 → varint 0) →
0x00 - end-line delta (Δ from start line, 0) →
0x00
So the entry bytes are:
E8 00 00 (hex) [1][2]
If instead CPython marks that instruction range as having no location at all (Kind 15 “No location”), then for length=1 the entry is just:
F8 (hex) [1][2]
Note on “RESUME has the def-line number”
For functions, co_firstlineno is the line with the def statement, so the first entry’s delta-0 convention makes RESUME map to the function definition line, which is a behavior change that has affected some tools. [2][4]
Sources: CPython 3.13 Objects/locations.md (format reference) [1]; CPython 3.14 InternalDocs/code_objects.md (same format, moved docs) [2]; PEP 626 (line-number model, module line 0, “artificial” bytecodes) [3]; python.org discussion about RESUME line numbering changes [4].
🌐 Web query:
CPython linetable format line 0 no-line instructions bytecode
💡 Result:
In modern CPython (3.10+), line/position info is no longer “every instruction has a real source line”. The bytecode→source mapping is stored in the code object’s line table (co_linetable) and exposed via code.co_lines(), which can yield ranges whose line is None (“no line number”) for artificial / bookkeeping bytecode. This is explicitly part of the PEP 626 model: some bytecodes “must be marked as artificial, and not have a meaningful line number”, and the iterator’s line can be None. [1]
Internally, when CPython decodes the line table, it represents “no line number” as a negative line (range.line < 0). [1] In the older (3.10-era) internal notes for the delta-encoded line table, there’s also an explicit sentinel for “no line number” (a special line-delta value). [2]
So if you’re seeing line 0 attached to certain instructions, that’s almost always a tooling/formatting artifact where “no line” (None / <0) got coerced to 0 for display or storage. Typical examples of bytecode that may have “no-line” (or otherwise surprising) locations are interpreter/setup opcodes like RESUME (and related prologue opcodes around generators/coroutines), which has been discussed as a behavior change users notice when enumerating covered lines. [3]
Practical guidance:
- Treat “line 0” as “no line info”, not a real source line.
- Prefer
code.co_lines()(line can beNone) per PEP 626, rather than reverse-engineeringco_linetable. [1] - If you need per-instruction positions, use
dis.get_instructions()/dis.dis()(these are built to follow the current semantics). [4]
Sources:
[1] PEP 626 (Precise line numbers; co_lines() yields line or None, and internal decode treats <0 as no-line) https://peps.python.org/pep-0626/
[2] CPython Objects/lnotab_notes.txt (sentinel encoding for “no line number”) https://chromium.googlesource.com/external/github.com/python/cpython/%2B/refs/tags/v3.10.0b3/Objects/lnotab_notes.txt
[3] Discussion of RESUME line-number behavior in 3.11 https://discuss.python.org/t/the-resume-opcode-has-the-same-line-number-as-the-function-definition-and-breaks-previous-behaviour/21972
[4] dis module docs (3.11+) https://docs.python.org/3.11/library/dis.html
🏁 Script executed:
git ls-files | grep ir.rsRepository: RustPython/RustPython
Length of output: 89
🏁 Script executed:
# Once we have the path, read the file at specified lines
git ls-files | grep ir.rs | head -1 | xargs -I {} sh -c 'echo "Lines 220-235:"; sed -n "220,235p" {}; echo -e "\nLines 330-350:"; sed -n "330,350p" {}; echo -e "\nLines 355-373:"; sed -n "355,373p" {}; echo -e "\nLines 845-905:"; sed -n "845,905p" {}'Repository: RustPython/RustPython
Length of output: 4652
🏁 Script executed:
# Search for all LineTableLocation creations in the file
rg "LineTableLocation" crates/codegen/src/ir.rs -B3 -A3Repository: RustPython/RustPython
Length of output: 1069
🏁 Script executed:
# Look for the struct definition
rg "struct LineTableLocation" crates/codegen/src/ -A5Repository: RustPython/RustPython
Length of output: 309
🏁 Script executed:
# Search for lineno_override usage to understand the full context
rg "lineno_override" crates/codegen/src/ir.rs -B10 -A5Repository: RustPython/RustPython
Length of output: 1512
🏁 Script executed:
# Check lines 360-368 more carefully
sed -n '355,375p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 696
🏁 Script executed:
# Check if there are other LineTableLocation creations
rg "LineTableLocation {" crates/codegen/src/ir.rs -B1 -A5Repository: RustPython/RustPython
Length of output: 166
🏁 Script executed:
# Verify the generate_linetable function handles these values
sed -n '850,920p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 2870
🏁 Script executed:
# Search for all LineTableLocation creations without regex special chars
grep -n "let lt_loc = LineTableLocation" crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 121
🏁 Script executed:
# Also search for direct LineTableLocation struct instantiations
grep -n "LineTableLocation {" crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 151
🏁 Script executed:
# Let's see what's at lines 360-368 to understand "Also applies to"
sed -n '355,375p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 696
🏁 Script executed:
# Check if there are multiple code blocks creating LineTableLocation with same pattern
sed -n '300,330p' crates/codegen/src/ir.rs
sed -n '340,370p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 2783
🏁 Script executed:
# Let me verify there's only one LineTableLocation creation by searching more carefully
grep -n "LineTableLocation {" crates/codegen/src/ir.rs -A6Repository: RustPython/RustPython
Length of output: 677
🏁 Script executed:
# Check if the code at 226-229 is actually the right location by looking at context
sed -n '220,245p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 1066
🏁 Script executed:
# Verify the lineno_override handling one more time
sed -n '330,350p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 1302
🏁 Script executed:
# Count total LineTableLocation creations
grep -c "let lt_loc = LineTableLocation" crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 66
🏁 Script executed:
# Verify the line numbers in the "Also applies to" section - check 360-368
sed -n '360,375p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 583
🏁 Script executed:
# Let me check around line 850-901 to understand what that refers to
sed -n '848,855p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 310
🏁 Script executed:
# Verify the issue: when lineno_override = Some(0), what happens in generate_linetable?
# Check if line_delta could be 0 but end_line_delta could be non-zero
sed -n '870,920p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 2265
🏁 Script executed:
# Let me understand the "Also applies to" references more carefully
# Check what's at the line ranges mentioned
echo "=== Checking 335-345 range ==="
sed -n '335,345p' crates/codegen/src/ir.rs
echo "=== Checking 360-368 range ==="
sed -n '360,368p' crates/codegen/src/ir.rs
echo "=== Checking 850-901 context - is this generate_linetable? ==="
sed -n '845,860p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 1621
🏁 Script executed:
# Verify the fix would work correctly by checking generate_linetable logic
# When col=0, end_col=0, what happens?
sed -n '895,950p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 3063
🏁 Script executed:
# Let me check the actual line-0 RESUME context by searching for where lineno_override is set
rg "lineno_override\s*=" crates/codegen/src/ -B2 -A2Repository: RustPython/RustPython
Length of output: 392
🏁 Script executed:
# Verify the concern about end_line_delta when lineno_override=Some(0)
# If lineno_override=Some(0) but end_location.line is non-zero, end_line_delta would be non-zero
# This would not fall into the "line_delta == 0 && end_line_delta == 0" path
# Let's check what happens in the else path
sed -n '910,950p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 2306
🏁 Script executed:
# Let me check the complete flow for line-0 case
# First, verify module scope sets lineno_override
sed -n '1,50p' crates/codegen/src/compile.rs | grep -A5 "Module scope"Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Check the line-0 flow: what happens when prev_line=first_line and line=0
sed -n '930,960p' crates/codegen/src/ir.rsRepository: RustPython/RustPython
Length of output: 1703
🏁 Script executed:
# Let me verify one more time that the fix would be appropriate by checking
# if there are tests or comments about line-0 RESUME semantics
rg "RESUME\|line.*0\|line-0" crates/codegen/src/ir.rs | head -20Repository: RustPython/RustPython
Length of output: 47
Override end_line and columns when lineno_override is set to 0.
When lineno_override = Some(0) is set for module-scope RESUME, the code currently overrides only line to 0 while keeping end_line and columns from the actual source location. Per CPython 3.13+ semantics, line 0 represents "no line" (artificial instructions), and such entries should have no meaningful column information. The current code creates a 0→first_line span with real columns, which violates CPython's linetable encoding (Kind 13 "NoColumns" expects no column data for line-0 entries).
🔧 Possible adjustment for line‑0 overrides
- 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) {
+ (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 };🤖 Prompt for AI Agents
In `@crates/codegen/src/ir.rs` around lines 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.
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 };
📦 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:
|
1 participant
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.