Resume opcode to hold ResumeType by ShaharNaveh · Pull Request #7465 · RustPython/RustPython · GitHub

Summary by CodeRabbit

Refactor
- Improved type safety in bytecode instruction handling by refining argument type representation for resume operations.

📝 Walkthrough

Walkthrough

Refactored the Instruction::Resume opcode's operand type from Arg<u32> to Arg<oparg::ResumeType>, replacing the auto-generated ResumeType enum with a manually defined variant that includes a catch-all Other(u32) case, and updated all emission sites to pass enum values directly.

Changes

Cohort / File(s)	Summary
ResumeType Enum Redefinition crates/compiler-core/src/bytecode/oparg.rs	Replaced auto-generated ResumeType with manual enum definition featuring named variants (AtFuncStart, AfterYield, AfterYieldFrom, AfterAwait) plus Other(u32) catch-all; added bidirectional From<u32> and From<ResumeType> for u32 conversions, Display trait, and OpArgType implementation.
Instruction Type Update crates/compiler-core/src/bytecode/instruction.rs	Changed Instruction::Resume { context: Arg<u32> } field type to Arg<oparg::ResumeType>, updating the typed interpretation of the RESUME opcode operand.
Emission Site Updates crates/codegen/src/compile.rs	Updated four Resume instruction emission sites to pass ResumeType enum variants directly instead of converting them via u32::from(...), affecting yield-from/await sequences and generator-expression resume paths.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

RustPython/RustPython#6322: Modifies Instruction::Resume operand typing and updates corresponding emission sites.
RustPython/RustPython#7032: Changes ResumeType representation and bidirectional conversions in the bytecode layer.
RustPython/RustPython#5944: Introduced the initial Resume instruction variant and ResumeType enum definition.

Suggested reviewers

youknowone

Poem

🐰 A rabbit hops through bytecode neat,
No more raw u32s to compete—
Resume now carries types so fine,
Each variant dancing in perfect line,
Enums and conversions align! 🎯

🚥 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 accurately summarizes the main change: updating the Resume opcode's context field from u32 to hold ResumeType enum values directly.
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 unit tests (beta)

Create PR with unit tests

📝 Coding Plan

Generate coding plan for human review comments

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.

@youknowone I'm thinking to create a derive macro called: #[newtype_oparg] that can be defined on either struct or enums. this will help streamline the oparg definition while being able to handle slightly less straight forward cases like overriding the fmt::Display for specific enum variants like, like we do here:

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

Lines 265 to 277 in b11b8e6

    
           impl fmt::Display for ConvertValueOparg { 
        
               fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { 
        
                   let out = match self { 
        
                       Self::Str => "1 (str)", 
        
                       Self::Repr => "2 (repr)", 
        
                       Self::Ascii => "3 (ascii)", 
        
                       // We should never reach this. `FVC_NONE` are being handled by `Instruction::FormatSimple` 
        
                       Self::None => "", 
        
                   }; 
        
                   write!(f, "{out}") 
        
               } 
        
           }

and

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

Lines 575 to 585 in b11b8e6

    
           impl fmt::Display for SpecialMethod { 
        
               fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { 
        
                   let method_name = match self { 
        
                       Self::Enter => "__enter__", 
        
                       Self::Exit => "__exit__", 
        
                       Self::AEnter => "__aenter__", 
        
                       Self::AExit => "__aexit__", 
        
                   }; 
        
                   write!(f, "{method_name}") 
        
               } 
        
           }

(we have more places)

Also with generate the From<u32> and From<OPARG_ENUM> for u32 for enums with a fallback variant, like here with "Other(u32)". Instruction::GetAwaitable oparg can also use this pattern

lmk what you think:)

🧹 Nitpick comments (1)

crates/compiler-core/src/bytecode/oparg.rs (1)

313-317: Consider showing variant names in Display for better diagnostics.

The current Display impl shows only the numeric value. For disassembly output, showing variant names could improve readability.

🔧 Optional enhancement for Display

 impl core::fmt::Display for ResumeType {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        u32::from(*self).fmt(f)
+        match self {
+            Self::AtFuncStart => write!(f, "0 (AtFuncStart)"),
+            Self::AfterYield => write!(f, "1 (AfterYield)"),
+            Self::AfterYieldFrom => write!(f, "2 (AfterYieldFrom)"),
+            Self::AfterAwait => write!(f, "3 (AfterAwait)"),
+            Self::Other(v) => write!(f, "{v}"),
+        }
     }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/compiler-core/src/bytecode/oparg.rs` around lines 313 - 317, The
Display impl for ResumeType currently prints only its numeric value
(u32::from(*self).fmt(f)); change it to print human-friendly variant names for
diagnostics by matching on ResumeType (in the impl core::fmt::Display for
ResumeType, fn fmt) and write the variant identifier strings (e.g., "None",
"Yield", etc.) to the formatter, optionally falling back to the numeric value
for unknown/other variants to preserve behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@crates/compiler-core/src/bytecode/oparg.rs`:
- Around line 313-317: The Display impl for ResumeType currently prints only its
numeric value (u32::from(*self).fmt(f)); change it to print human-friendly
variant names for diagnostics by matching on ResumeType (in the impl
core::fmt::Display for ResumeType, fn fmt) and write the variant identifier
strings (e.g., "None", "Yield", etc.) to the formatter, optionally falling back
to the numeric value for unknown/other variants to preserve behavior.

ℹ️ Review info ⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: a841bb7a-52f4-4af9-85c5-31e89e5a26d1

📥 Commits

Reviewing files that changed from the base of the PR and between b11b8e6 and adae369.

📒 Files selected for processing (3)

crates/codegen/src/compile.rs
crates/compiler-core/src/bytecode/instruction.rs
crates/compiler-core/src/bytecode/oparg.rs

@ShaharNaveh sounds good to me. Could you show me an example how it will look like once the macro is introduced?

@ShaharNaveh sounds good to me. Could you show me an example how it will look like once the macro is introduced?

I was thinking something like:

#[newtype_oparg]
struct VarNum(u32);

#[newtype_oparg]
enum SpecialMethod {
  /// `__enter__` for sync context manager
  #[oparg(display = "__enter__")]
  Enter = 0,
  /// `__exit__` for sync context manager
  #[oparg(display = "__exit__")]
  Exit = 1,
  ...
}

#[newtype_oparg]
enum ResumeType {
  AfterYield = 1
  AfterYieldFrom = 2,
  #[oparg(catch_all)]
  Other(u32)
}

@youknowone loks good?

ShaharNaveh added 2 commits March 19, 2026 14:21

Resume to hold ResumeType

7d890ba

Fix codegen

adae369

coderabbitai Bot reviewed Mar 19, 2026

View reviewed changes

youknowone approved these changes Mar 19, 2026

View reviewed changes

youknowone enabled auto-merge (squash) March 19, 2026 13:36

youknowone merged commit 53fa525 into RustPython:main Mar 19, 2026
15 checks passed

youknowone pushed a commit to youknowone/RustPython that referenced this pull request Mar 22, 2026

Resume opcode to hold ResumeType (RustPython#7465 )

137a988

coderabbitai Bot mentioned this pull request Mar 23, 2026

#[newtype_oparg] derive macro #7489

Closed

Copilot AI pushed a commit that referenced this pull request Mar 25, 2026

Resume opcode to hold ResumeType (#7465 )

cfbf6d6

coderabbitai Bot mentioned this pull request Mar 27, 2026

Oparg resume depth #7515

Merged

Resume opcode to hold ResumeType by ShaharNaveh · Pull Request #7465 · RustPython/RustPython · GitHub

Uh oh!

Resume opcode to hold ResumeType - #7465

Resume opcode to hold ResumeType#7465
youknowone merged 2 commits into
RustPython:mainfrom
ShaharNaveh:resume-type-oparg

ShaharNaveh commented Mar 19, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Mar 19, 2026 •
edited
Loading

Uh oh!

Uh oh!

ShaharNaveh commented Mar 19, 2026 •
edited
Loading

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

youknowone commented Mar 19, 2026

Uh oh!

ShaharNaveh commented Mar 19, 2026 •
edited
Loading

Uh oh!

Uh oh!

Uh oh!

ShaharNaveh commented Mar 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Resume opcode to hold ResumeType by ShaharNaveh · Pull Request #7465 · RustPython/RustPython · GitHub

Uh oh!

Resume opcode to hold ResumeType - #7465

Resume opcode to hold ResumeType#7465youknowone merged 2 commits intoRustPython:mainRustPython/RustPython:mainfrom ShaharNaveh:resume-type-opargShaharNaveh/RustPython:resume-type-opargCopy head branch name to clipboard

Conversation

ShaharNaveh commented Mar 19, 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 Mar 19, 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 commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

youknowone commented Mar 19, 2026

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

ShaharNaveh commented Mar 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Resume opcode to hold ResumeType#7465
youknowone merged 2 commits into
RustPython:mainfrom
ShaharNaveh:resume-type-oparg

ShaharNaveh commented Mar 19, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Mar 19, 2026 •
edited
Loading

ShaharNaveh commented Mar 19, 2026 •
edited
Loading

ShaharNaveh commented Mar 19, 2026 •
edited
Loading