Sort `Instruction` enum & related match arms by ShaharNaveh · Pull Request #6322 · RustPython/RustPython
Walkthrough
This PR performs a comprehensive restructuring of the bytecode instruction set across the compiler, JIT, and VM, reorganizing opcode variants alphabetically and updating instruction handlers to support an expanded set of operations including async/context-manager support, advanced data-structure construction, pattern matching, and improved control flow and exception handling.
Changes
| Cohort / File(s) | Summary |
|---|---|
Bytecode Instruction Enum Restructuring crates/compiler-core/src/bytecode.rs |
Major reorganization of the public Instruction enum with alphabetically sorted variants. Removed legacy opcodes and introduced new/renamed suite including async (BeforeAsyncWith, SetupAsyncWith, EndAsyncFor), container construction (BuildList, BuildTuple, BuildMap, BuildSet variants), pattern matching (MatchClass, MatchKeys, MatchMapping), and expanded control flow (JumpIfFalseOrPop, JumpIfTrueOrPop, PopBlock, PopException). Updated LAST_INSTRUCTION constant from ExtendedArg to YieldValue, affecting marshaling bounds and range checks. |
JIT Instruction Handler Expansion crates/jit/src/instructions.rs |
Reworked FunctionCompiler::add_instruction with support for expanded instruction set including ExtendedArg, Jump variants, LoadFast/Global/Const, Pop operations, block management (SetupLoop, PopBlock), comparisons (CompareOperation with numeric and mixed-type paths), and double-double arithmetic (dd_\* functions). Added handling for UnpackSequence, BuildTuple, CallFunction variants, and enhanced error paths (NotSupported, BadBytecode). |
VM Frame Execution Handler Rewrite crates/vm/src/frame.rs |
Extensive expansion of ExecutingFrame::execute_instruction with comprehensive handlers for: async operations (BeforeAsyncWith, GetAIter, GetANext, EndAsyncFor), container construction (BuildList, BuildTuple, BuildMap, BuildSet with tuple/iterable variants), pattern matching (MatchClass, MatchKeys, MatchMapping), method/attribute resolution (LoadMethod, LoadAttr, LoadBuildClass), finalization logic (WithCleanupStart/Finish, EndFinally, PopException), and call patterns (CallFunctionEx, CallMethodEx/Keyword/Positional). Includes detailed extraction logic and match_args protocol handling. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
- Enum variant completeness: Verify all instruction variants are correctly alphabetized and none are missing; cross-reference against CPython opcode_ids.h structure
- Oparg type consistency: Ensure oparg handling and type conversions are uniform across bytecode.rs, instructions.rs (JIT), and frame.rs (VM)
- Async/with/finally semantics: Validate BeforeAsyncWith, SetupAsyncWith, EndAsyncFor, WithCleanupStart/Finish, and exception preservation logic match Python semantics
- Pattern matching protocol: Confirm match_args extraction and MATCH_SELF handling are correctly implemented
- Control flow branching: Review PopJumpIfFalse/PopJumpIfTrue, JumpIfFalseOrPop, and block management for correctness
- Edge cases: Pay attention to async protocol mismatches, attribute access failures, and exception unwinding paths
Possibly related PRs
- Remove
Rotate*&Duplicate*instructions #6303: Related refactoring replacing Rotate/Duplicate instructions with CopyItem/Swap in the same opcode enum restructuring effort - Split
TestOperatorinstruction #6306: Directly related split of TestOperation into IsOp, ContainsOp, and JumpIfNotExcMatch with Invert handling - Unify BINARY_OP bytecodes #6317: Related unification of BinaryOperation/BinaryOperationInplace into BinaryOp within the same Instruction enum rewrite
Suggested reviewers
- youknowone
Poem
🐰 Opcodes shuffled, sorted A to Z,
Async hops and patterns dance with glee,
Containers built, exceptions caught just right,
The bytecode garden blooms in Python's light! ✨
Pre-merge checks and finishing touches
❌ Failed checks (3 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | ⚠️ Warning | The title describes sorting the Instruction enum and match arms, but the changes are far broader, including significant opcode restructuring, new instruction variants, and extensive implementation work across multiple files. | Update the title to reflect the actual scope: something like 'Restructure Instruction enum variants and expand opcode support across compiler, VM, and JIT' would better capture the comprehensive changes made. |
| Linked Issues check | ⚠️ Warning | The PR only partially addresses #6312 objectives. While enum variants appear reorganized, the changes include extensive new implementations (BeforeAsyncWith, async/with handling, pattern matching, advanced call patterns) far beyond sorting and converting named fields to tuple variants. | Clarify whether the PR's expanded instruction set implementation is in scope for #6312 or should be split into separate issues. The linked issue focuses on sorting and field refactoring, not substantial new feature implementation. |
| Out of Scope Changes check | ⚠️ Warning | Significant out-of-scope changes detected. Beyond sorting and field conversion (#6312 scope), the PR introduces comprehensive new instruction handling for async operations (BeforeAsyncWith, GetAIter, GetANext, EndAsyncFor), pattern matching (MatchClass, MatchKeys, MatchMapping), advanced call patterns, and extensive new implementations in frame.rs and instructions.rs. | Review and separate feature implementations from refactoring. Move new async, pattern-matching, and call-handling implementations to distinct issues/PRs unless they are explicitly part of #6312's scope with documented justification. |
✅ 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 83.33% which is sufficient. The required threshold is 80.00%. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 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.