Make `OpArgType::from_op_arg` to return a `Result<Self, MarshalError>`#6914

ShaharNaveh

This will allow us to use num_enum crate with a custom error, I'll do it in a followup PR

Summary by CodeRabbit

Refactor
- Improved internal error handling in the compiler's bytecode instruction processing to provide more detailed error information when invalid bytecode is encountered.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai

📝 Walkthrough

Walkthrough

A refactoring of error handling in bytecode instruction processing, converting return types from Option<T> to Result<T, MarshalError> across the OpArgType trait and Arg<T>::try_get method to provide more descriptive error information on invalid bytecode.

Changes

Cohort / File(s)	Summary
OpArgType trait refactoring `crates/compiler-core/src/bytecode/oparg.rs`	Changed `from_op_arg` return type from `Option<Self>` to `Result<Self, MarshalError>` in trait definition and all implementations (`ConvertValueOparg`, `u32`, `bool`, `Label`, `MakeFunctionFlags`, `BuildSliceArgCount`, `UnpackExArgs`, and macro-generated enums). Invalid bytecode conditions now return `Err(MarshalError::InvalidBytecode)` instead of `None`.
Arg type getter `crates/compiler-core/src/bytecode/instruction.rs`	Updated `Arg<T>::try_get` method signature to return `Result<T, MarshalError>` instead of `Option<T>`, delegating to the updated `T::from_op_arg` implementation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

RustPython/RustPython#6703: This PR directly modifies the same OpArgType::from_op_arg trait method and Arg<T>::try_get implementation, converting their return types from Option to Result<_, MarshalError>.

Suggested reviewers

youknowone

Poem

🐰 From Options to Results, we now declare,
Error types flowing with much greater care,
MarshalError whispers of bytecode gone wrong,
No more silent Nones in our instruction song!
The bytecode now speaks what went awry, 🎯

🚥 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 pull request title accurately reflects the main change: converting OpArgType::from_op_arg to return Result<Self, MarshalError> instead of Option. This is clearly the primary objective of the changeset across both modified files.
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.}

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@crates/compiler-core/src/bytecode/oparg.rs`:
- Around line 640-645: from_op_arg for UnpackExArgs currently truncates high
bytes via to_le_bytes; instead validate that the upper two bytes of x are zero
before constructing UnpackExArgs to avoid accepting invalid bytecode: check (x
>> 16) == 0 (or inspect bytes[2..]) and return an appropriate MarshalError if
non-zero, otherwise proceed to extract before and after and return
Ok(Self{before, after}); reference UnpackExArgs, OpArgType::from_op_arg, and
MarshalError to locate where to add the check and error return.

Make to_oparg to return a Result<Self, MarshalError>

21e7584

ShaharNaveh changed the title ~~Make to_oparg to return a Result<Self, MarshalError>~~ Jan 31, 2026

coderabbitai Bot reviewed Jan 31, 2026

View reviewed changes

youknowone approved these changes Jan 31, 2026

View reviewed changes

This was referenced Feb 3, 2026

Use num_enum crate for oparg types #6980

Merged

Make inner oparg values private #7050

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `OpArgType::from_op_arg` to return a `Result<Self, MarshalError>`#6914