◐ Shell
reader mode source ↗
Skip to content

Add has_* methods to opcode enums#7818

Closed
ShaharNaveh wants to merge 2 commits into
RustPython:mainfrom
ShaharNaveh:opcode-flags
Closed

Add has_* methods to opcode enums#7818
ShaharNaveh wants to merge 2 commits into
RustPython:mainfrom
ShaharNaveh:opcode-flags

Conversation

@ShaharNaveh

@ShaharNaveh ShaharNaveh commented May 10, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Extracted from #7797

Summary by CodeRabbit

  • Refactor
    • Improved bytecode opcode/pseudo-opcode capability checks for more consistent and reliable behavior.
    • Streamlined Python-facing opcode helpers to a uniform approach, reducing duplication and improving maintainability.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: c00767f3-6b24-4865-a081-49a71f9fe8e2

📥 Commits

Reviewing files that changed from the base of the PR and between d946a58 and 17efe8e.

📒 Files selected for processing (1)
  • crates/stdlib/src/_opcode.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/stdlib/src/_opcode.rs
📝 Walkthrough

Walkthrough

This PR adds opcode capability query helpers to the bytecode instruction layer. New predicates (has_const, has_free, has_jump, has_local, has_name) and block classification (is_block_push) are introduced on both Opcode and PseudoOpcode. These are exposed through Instruction/PseudoInstruction delegators and unified via a new either_real_pseudo! macro for AnyInstruction/AnyOpcode dispatch. Python stdlib _opcode helpers are refactored to delegate to these new abstractions.

Changes

Bytecode Instruction Opcode Helpers

Layer / File(s) Summary
Opcode Capability Predicates
crates/compiler-core/src/bytecode/instruction.rs		 Opcode and PseudoOpcode types gain const methods has_const, has_free, has_jump, has_local, has_name, and is_block_push. Real opcodes return false for is_block_push; pseudo opcodes implement it for Setup* variants.
Instruction Delegators
crates/compiler-core/src/bytecode/instruction.rs		 Instruction and PseudoInstruction gain is_block_push(&self) methods that delegate to their underlying Opcode and PseudoOpcode.
AnyInstruction/AnyOpcode Macro Dispatch
crates/compiler-core/src/bytecode/instruction.rs		 New either_real_pseudo! macro unifies InstructionMetadata dispatch for AnyInstruction. AnyOpcode gains const query methods and is_block_push, dispatching to real or pseudo variants. AnyInstruction::is_block_push receiver changed from self to &self.
Python Stdlib Refactoring
crates/stdlib/src/_opcode.rs		 Module imports change to use AnyOpcode. Functions has_const, has_name, has_jump, has_free, has_local, has_exc refactored from hardcoded matches! lists to uniform delegation via `try_from_i32(opcode).map_or(false,

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • RustPython/RustPython#7757: Modifies AnyInstruction opcode-related helpers and refactors instruction/opcode dispatch, directly overlapping.
  • RustPython/RustPython#7711: Changes _opcode pyfunctions' opcode-match logic; closely related to stdlib helper refactors here.
  • RustPython/RustPython#7585: Modifies bytecode instruction metadata and pseudo-instruction behavior across the same file(s).

Suggested reviewers

  • coolreader18
  • youknowone

Poem

🐇 I nibble bytes and hum a tune,

has_const, has_free beneath the moon;
either_real_pseudo! ties the knot,
Opcodes sing, pseudo-ops trot;
A little hop for cleaner thought.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add has_* methods to opcode enums' accurately summarizes the main change: introducing new opcode-flag query helpers (has_const, has_free, has_jump, has_local, has_name) to both Opcode and PseudoOpcode.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

coderabbitai[bot]
coderabbitai Bot reviewed May 10, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/compiler-core/src/bytecode/instruction.rs`:
- Around line 1051-1074: The has_jump() method is missing two specialized opcode
variants; update the matches! list inside pub const fn has_jump(self) -> bool
(in instruction.rs) to also include ForIterGen and SendGen so they map
consistently to the base opcodes ForIter and Send (similar to the existing
ForIterList/ForIterRange/ForIterTuple and JumpBackwardJit/JumpBackwardNoJit
entries); simply add ForIterGen and SendGen to the list of matched variants in
has_jump().
- Around line 1042-1049: The has_free() method currently matches DeleteDeref,
LoadFromDictOrDeref, MakeCell, and StoreDeref; update it to match CPython's
HAS_FREE_FLAG by including LoadDeref and removing LoadFromDictOrDeref and
MakeCell. Edit the matches expression inside the pub const fn has_free(&self) to
only include Self::DeleteDeref | Self::LoadDeref | Self::StoreDeref, ensuring
the function name has_free and the opcode variants DeleteDeref, LoadDeref,
StoreDeref are the referenced symbols to change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 2a215c26-cd0a-4398-8445-1384ce64a21e

📥 Commits

Reviewing files that changed from the base of the PR and between 108461f and d946a58.

📒 Files selected for processing (2)
  • crates/compiler-core/src/bytecode/instruction.rs
  • crates/stdlib/src/_opcode.rs

@ShaharNaveh

ShaharNaveh commented May 10, 2026

Copy link
Copy Markdown
Contributor Author

closed by #7797

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ShaharNaveh