Seperate between scope exit & unconditional jump opcodes by ShaharNaveh · Pull Request #6841 · RustPython/RustPython
📝 Walkthrough
Walkthrough
The PR refactors instruction classification semantics by splitting the unconditional_branch() predicate into two separate methods: is_unconditional_jump() for jump-specific instructions and is_scope_exit() for return/raise-related instructions. Callers in dead-code elimination and stack-depth analysis are updated accordingly.
Changes
| Cohort / File(s) | Summary |
|---|---|
Instruction classification API redesign crates/compiler-core/src/bytecode/instruction.rs |
Replaces unconditional_branch() trait method with is_unconditional_jump() and is_scope_exit(). Splits predicate logic: jumps (JumpForward, JumpBackward, JumpBackwardNoInterrupt) map to is_unconditional_jump(); returns and raises map to is_scope_exit(). Updates implementations for Instruction, PseudoInstruction, and AnyInstruction via inst_either macro. |
Codegen DCE and stack-depth analysis crates/codegen/src/ir.rs |
Updates two callsites in CodeInfo::dce and CodeInfo::max_stackdepth to use is_scope_exit() || is_unconditional_jump() instead of unconditional_branch() for detecting block termination points. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- Remove SUBSCRIPT, JUMP_IF_{TRUE,FALSE}_OR_POP #6810: Modifies InstructionMetadata trait and instruction predicate implementations alongside this refactor of control-flow classification methods.
- Remove ReturnConst/Break/Continue ops #6816: Adjusts unconditional_branch handling and instruction classification semantics in parallel with this PR's predicate split.
- Bytecode instrumented placeholder #6741: Modifies InstructionMetadata implementations in the same instruction.rs file with overlapping structural changes.
Suggested reviewers
- youknowone
Poem
🐰 A hop through control flow so bright,
Jumps and exits, now split just right!
One path for leaps, one for the end,
Stack depths and dead code, old friends.
🌿 ✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The pull request title directly and clearly summarizes the main change: separating scope-exit opcodes from unconditional-jump opcodes, which aligns with both file changes and PR objectives. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
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.