Bytecode enum named oparg by ShaharNaveh · Pull Request #7294 · RustPython/RustPython
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/vm/src/frame.rs
📝 Walkthrough
Walkthrough
This PR renames and restructures many bytecode Instruction and PseudoInstruction operand fields (e.g., idx → consti/namei/i/var_num, target → delta, size → count, nargs → argc) and converts many tuple-like variants to struct-like variants across compiler-core, codegen, JIT, VM, stdlib, and tests.
Changes
| Cohort / File(s) | Summary |
|---|---|
Core Instruction Definitions crates/compiler-core/src/bytecode/instruction.rs |
Major public API changes: many Instruction/PseudoInstruction variants restructured and renamed (e.g., idx→consti/namei/i, target→delta, size→count, nargs→argc); display, metadata, and instrumented/base mappings updated. |
Codegen IR & Optimizations crates/codegen/src/ir.rs |
Updated IR transformations, peephole patterns, and pseudo-op lowering to use renamed fields (e.g., LoadConst.consti, LoadFast.var_num, jumps use delta). |
JIT Implementation & Tests crates/jit/src/instructions.rs, crates/jit/tests/common.rs |
Adjusted JIT instruction handling, operand extraction, and test helpers to new field names (argc, consti, namei, var_num, count, var_nums). |
VM Execution & Builtins crates/vm/src/frame.rs, crates/vm/src/builtins/frame.rs |
Interpreter and builtin frame match arms updated to struct-style variants and renamed operand fields throughout execution and deopt paths. |
Stdlib Opcode Utilities crates/stdlib/src/_opcode.rs |
Pattern matches converted from tuple-like to struct-like variants for name/free/local cases to align with restructured Instructions. |
Spell dictionary .cspell.dict/cpython.txt |
Added lexical token consti. |
Sequence Diagram(s)
(Skipped — changes are renames/restructuring without new multi-component control flow requiring visualization.)
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- Bytecode pseudo opcodes #6715: Modifies Instruction/PseudoInstruction enums and updates call sites — strongly overlaps with operand renames.
- Instruction 3.14 #6805: Overlapping renames/restructuring of Instruction/PseudoInstruction operands (
consti,var_num,delta,count,namei). - slice ops and a little bit of peephole #6860: Changes to peephole fusion and LoadFast/StoreFast combined ops that touch the same fused-instruction paths.
Suggested reviewers
- youknowone
Poem
"I nibble bytes and hop through code,
var_num and namei on the road.
consti counts my rosy treats,
delta marks the jumping beats.
argc gathers all the loads." 🐇✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | ❓ Inconclusive | The title 'Bytecode enum named oparg' is vague and does not clearly convey the main purpose of the changeset, which involves renaming multiple instruction field names (idx→namei/consti, size→count, nargs→argc, etc.) to align with Python dis documentation. | Consider a more descriptive title such as 'Align bytecode instruction field names with Python dis documentation' or 'Rename instruction operand fields for consistency with Python dis.' |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
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 @coderabbitai help to get the list of available commands and usage tips.