GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Bytecode oparg optimization by ShaharNaveh · Pull Request #7032 · RustPython/RustPython

Conversation

ShaharNaveh commented Feb 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Discussed at #6980 (comment)
Pretty much reverts #6980.

Summary by CodeRabbit

Chores
- Removed unused internal dependency
- Internal code structure improvements and standardization
Refactor
- Simplified internal type conversion patterns across the compiler

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ShaharNaveh added 5 commits

February 6, 2026 12:11


          Generate optimized oparg enums

deefe91


          No need to match on 255

b7794a5


          Remove num_enum crate from compiler-core

ebabd58


          Update Cargo.lock

a8b581c


          macro fmt

e48fdeb

coderabbitai Bot commented Feb 6, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

📝 Walkthrough

Walkthrough

This PR refactors bytecode enum handling and casting patterns across the compiler. It replaces explicit u32 casts with standard conversion functions (u32::from()), removes the num_enum dependency, adds Debug derives to bytecode types, macro-generates oparg enum definitions and trait implementations, and removes const qualifiers from eval_ord methods. All changes maintain functional equivalence while modernizing the codebase.

Changes

Cohort / File(s)	Summary
Casting & Conversion Modernization `crates/codegen/src/compile.rs`, `crates/vm/src/types/slot.rs`	Updated explicit u32 and u8 casts to use `u32::from()` and `u8::from()` for ResumeType and comparison operators. In compile.rs, also adjusted oparg calculation for BUILD_INTERPOLATION. Removed const qualifier from `eval_ord` methods in slot.rs.
Bytecode Debug Support `crates/compiler-core/src/bytecode.rs`	Added Debug derive to CodeUnit and CodeUnits structs to enable debug formatting alongside existing Copy/Clone and Clone derives respectively.
Enum Macro Refactoring `crates/compiler-core/src/bytecode/oparg.rs`	Introduced oparg_enum! and impl_oparg_enum macros to auto-generate TryFrom, From, and OpArgType trait implementations. Refactored all public enums (ConvertValueOparg, ResumeType, RaiseKind, IntrinsicFunction1/2, Intrinsic, BinaryOperator, ComparisonOperator, Invert, SpecialMethod, CommonConstant, BuildSliceArgCount) to use macro-based definitions, eliminating repetitive manual trait impls.
Dependency Cleanup `crates/compiler-core/Cargo.toml`	Removed workspace dependency on num_enum from [dependencies] section.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Instruction::Resume #5944: Directly related through compile.rs ResumeType cast updates that align with Resume instruction introduction and ResumeType handling.
Use num_enum crate for oparg types #6980: Connected via oparg enum representation changes and conversion code refactoring; both touch ResumeType and num_enum-related conversions.
Update opcode, dis from 3.14.2 #6869: Related through resume emission logic modifications in compile.rs where ResumeType-to-u32 conversions are adjusted.

Suggested reviewers

youknowone

Poem

🐰 Hoppy hops through casting code,
Macros bloom along the road!
Enums dance in templates bright,
From-conversions feel so right! ✨
Debug fields now shine with care,
Refactored with rabbit flair!

🚥 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 'Bytecode oparg optimization' directly and accurately reflects the main objective of the PR: optimizing bytecode operand arguments through macro-based enum generation and the removal of the num_enum dependency.
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 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.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ShaharNaveh added 6 commits

February 6, 2026 12:55


          Rename macro vars

badcd5f


          Match without ,

56e23ef


          Support alternative values

5f3eb51


          Fix alternatives

a371d16


          Improve docs


          Add const assert

14f3030

ShaharNaveh commented

Feb 6, 2026

View reviewed changes

Comment thread

crates/compiler-core/src/bytecode/oparg.rs

		$vis enum $name {
		$(
		$(#[$variant_meta])*
		$variant, // Do assign value to variant.

ShaharNaveh Feb 6, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@youknowone This breaks RustPython. if I put $variant = $value then everything works just fine.
I suspect that there's some unsafe transmuting somewhere, do you have an idea how can I debug this?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

youknowone reacted with eyes emoji

All reactions

👀 1 reaction

ShaharNaveh Feb 6, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

After lots of debugging I've figured that the only problematic enum is ComparisonOperator. I'm not sure about why yet, but it's not related to an unsafe transmutation like I initially thought

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

youknowone reacted with thumbs up emoji

All reactions

👍 1 reaction

youknowone Feb 7, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ComparisonOperator seems to support bit operation. It doesn't have 0 like other enums

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ShaharNaveh added 8 commits

February 6, 2026 18:20


          Don't use as u32

d2c98f0


          Merge remote-tracking branch 'upstream/main' into bytecode-oparg-opti…

e3b9efa

…mization


          Make only ComparisonOperator unoptimized

4fcc241


          Fix test

4a06d52


          cleanup

2a9ef82


          Merge remote-tracking branch 'upstream/main' into bytecode-oparg-opti…

71be956

…mization


          All opargs are optimized

bb5677c


          Remove comment

ShaharNaveh marked this pull request as ready for review

February 7, 2026 07:00

ShaharNaveh requested a review from youknowone

February 7, 2026 07:59

youknowone approved these changes

Feb 7, 2026

View reviewed changes

youknowone enabled auto-merge (squash)

February 7, 2026 10:11

youknowone disabled auto-merge

February 7, 2026 10:14

youknowone merged commit f817ab8 into RustPython:main

Feb 7, 2026

13 checks passed

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot mentioned this pull request

Feb 8, 2026

Make inner oparg values private #7050

Merged

This was referenced Mar 19, 2026

Resume opcode to hold ResumeType #7465

Merged

marshal #7467

Merged

This was referenced Mar 24, 2026

#[newtype_oparg] derive macro #7489

Closed

Oparg resume depth #7515

Merged

youknowone pushed a commit that referenced this pull request

Sep 16, 2026


          Bytecode oparg optimization (#7032)

21a8733

* Generate optimized oparg enums

* No need to match on 255

* Remove `num_enum` crate from `compiler-core`

* Update `Cargo.lock`

* macro fmt

* Rename macro vars

* Match without `,`

* Support alternative values

* Fix alternatives

* Improve docs

* Add const assert

* Don't use `as u32`

* Make only ComparisonOperator unoptimized

* Fix test

* cleanup

* All opargs are optimized

* Remove comment

Reviewers

youknowone

youknowone approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

ShaharNaveh commented Feb 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

ShaharNaveh Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaharNaveh Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

youknowone Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ShaharNaveh commented Feb 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented Feb 6, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

ShaharNaveh Feb 6, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.