Split stack_effect into pushed & popped by ShaharNaveh · Pull Request #6893 · RustPython/RustPython

coderabbitai

📝 Walkthrough

Walkthrough

Refactors stack-effect handling by introducing a new StackEffect type (pushed/popped + effect), extends InstructionMetadata with stack_effect_info (returning StackEffect) while keeping a default stack_effect delegating to it, adds From<OpArg> for u32, re-exports StackEffect, and updates call sites in codegen and stdlib to use the new API (plus a minor local import removal).

Changes

Cohort / File(s)	Summary
Core trait & implementations `crates/compiler-core/src/bytecode/instruction.rs`, `crates/compiler-core/src/bytecode.rs`	Introduce `pub struct StackEffect { pushed, popped }`; add `stack_effect_info(&self, oparg: u32) -> StackEffect` to `InstructionMetadata` with a default `stack_effect` that returns `.effect()`; implement for `Instruction`/`PseudoInstruction`/`AnyInstruction`; re-export `StackEffect`.
OpArg conversions `crates/compiler-core/src/bytecode/oparg.rs`	Add `impl From<OpArg> for u32` to enable converting `OpArg` -> `u32`.
Call-site updates `crates/codegen/src/ir.rs`, `crates/stdlib/src/_opcode.rs`	Update call sites to pass numeric oparg (use `.into()` or pass `u32`) and to consume the new stack-effect API; remove a local import of `InstructionMetadata` in `codegen/src/ir.rs`; minor change in `_opcode.rs` to pass oparg directly.

Sequence Diagram(s)

sequenceDiagram
  participant Codegen as Codegen (crates/codegen)
  participant Stdlib as Stdlib (_opcode)
  participant Compiler as Compiler-core (AnyInstruction / Instruction)
  participant OpArg as OpArg (converter)
  Note over Codegen,Stdlib: Obtain numeric oparg and request stack effect
  Codegen->>OpArg: convert/forward oparg -> u32 (use `.into()` where needed)
  Stdlib->>OpArg: use oparg as u32 / forward as-is
  Codegen->>Compiler: call stack_effect_info(u32) or stack_effect(u32)
  Stdlib->>Compiler: call stack_effect(u32)
  Compiler-->>Codegen: return StackEffect (pushed,popped) / .effect()
  Compiler-->>Stdlib: return StackEffect (pushed,popped) / .effect()

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Remove SUBSCRIPT, JUMP_IF_{TRUE,FALSE}_OR_POP #6810: Modifies instruction stack-effect API and relates to the InstructionMetadata / stack-effect changes.
instruction improvements #6829: Updates IR and call sites to the new StackEffect-based interface.
Align psuedo ops to CPython 3.14.2 #6846: Overlaps on stack-effect computation and oparg handling across compiler-core and stdlib.

Suggested reviewers

youknowone

Poem

🐇 I hop through bytes and stacky streams,
Counting pops and pushed-up dreams.
From OpArg bits to effect’s song,
I tally tidy, neat, and strong—
A rabbit’s cheer for code that gleams! 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Split stack_effect into pushed & popped' directly and accurately describes the main architectural change: refactoring stack_effect from a single i32 return into a structured StackEffect type with separate pushed and popped fields.
Docstring Coverage	✅ Passed	Docstring coverage is 94.44% 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 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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}