From e599382ac4b50d4f98e76b86dc0b345c96370e88 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Thu, 19 Mar 2026 15:23:32 +0100 Subject: [PATCH 01/13] Basic struct support --- Cargo.lock | 10 ++ Cargo.toml | 1 + crates/compiler-core/Cargo.toml | 1 + crates/compiler-core/src/bytecode/oparg.rs | 107 +++------------------ crates/macros/Cargo.toml | 20 ++++ crates/macros/src/lib.rs | 24 +++++ crates/macros/src/newtype_oparg.rs | 107 +++++++++++++++++++++ 7 files changed, 178 insertions(+), 92 deletions(-) create mode 100644 crates/macros/Cargo.toml create mode 100644 crates/macros/src/lib.rs create mode 100644 crates/macros/src/newtype_oparg.rs diff --git a/Cargo.lock b/Cargo.lock index b0788ba0d82..6dc9961a22d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3160,6 +3160,7 @@ dependencies = [ "malachite-bigint", "num-complex", "ruff_source_file", + "rustpython-macros", "rustpython-wtf8", ] @@ -3231,6 +3232,15 @@ dependencies = [ "unic-ucd-category", ] +[[package]] +name = "rustpython-macros" +version = "0.5.0" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "rustpython-pylib" version = "0.5.0" diff --git a/Cargo.toml b/Cargo.toml index 8debf3ca321..54b5a8e218c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -155,6 +155,7 @@ rustpython-stdlib = { path = "crates/stdlib", default-features = false, version rustpython-sre_engine = { path = "crates/sre_engine", version = "0.5.0" } rustpython-wtf8 = { path = "crates/wtf8", version = "0.5.0" } rustpython-doc = { path = "crates/doc", version = "0.5.0" } +rustpython-macros = { path = "crates/macros", version = "0.5.0" } # Ruff tag 0.15.6 is based on commit e4c7f357777a2fdd34dbe6a98b1b7d3e7488f675 # at the time of this capture. We use the commit hash to ensure reproducible builds. diff --git a/crates/compiler-core/Cargo.toml b/crates/compiler-core/Cargo.toml index 7be58432cdf..4996efc5f23 100644 --- a/crates/compiler-core/Cargo.toml +++ b/crates/compiler-core/Cargo.toml @@ -12,6 +12,7 @@ license.workspace = true # rustpython-parser-core = { workspace = true, features=["location"] } ruff_source_file = { workspace = true } rustpython-wtf8 = { workspace = true } +rustpython-macros = { workspace = true } bitflags = { workspace = true } bitflagset = { workspace = true } diff --git a/crates/compiler-core/src/bytecode/oparg.rs b/crates/compiler-core/src/bytecode/oparg.rs index 2dd18fba963..8774b238490 100644 --- a/crates/compiler-core/src/bytecode/oparg.rs +++ b/crates/compiler-core/src/bytecode/oparg.rs @@ -1,5 +1,8 @@ use core::fmt; +use crate as rustpython_compiler_core; // Required for newtype_oparg macro +use rustpython_macros::newtype_oparg; + use crate::{ bytecode::{CodeUnit, instruction::Instruction}, marshal::MarshalError, @@ -745,103 +748,23 @@ impl fmt::Display for UnpackExArgs { } } -macro_rules! newtype_oparg { - ( - $(#[$oparg_meta:meta])* - $vis:vis struct $name:ident(u32) - ) => { - $(#[$oparg_meta])* - $vis struct $name(u32); - - impl $name { - /// Creates a new [`$name`] instance. - #[must_use] - pub const fn new(value: u32) -> Self { - Self(value) - } +#[newtype_oparg] +pub struct ConstIdx; - /// Alias to [`$name::new`]. - #[must_use] - pub const fn from_u32(value: u32) -> Self { - Self::new(value) - } +#[newtype_oparg] +pub struct VarNum; - /// Returns the oparg as a `u32` value. - #[must_use] - pub const fn as_u32(self) -> u32 { - self.0 - } +#[newtype_oparg] +pub struct VarNums; - /// Returns the oparg as a `usize` value. - #[must_use] - pub const fn as_usize(self) -> usize { - self.0 as usize - } - } +#[newtype_oparg] +pub struct LoadAttr; - impl From for $name { - fn from(value: u32) -> Self { - Self::from_u32(value) - } - } - - impl From<$name> for u32 { - fn from(value: $name) -> Self { - value.as_u32() - } - } - - impl From<$name> for usize { - fn from(value: $name) -> Self { - value.as_usize() - } - } +#[newtype_oparg] +pub struct LoadSuperAttr; - impl ::core::fmt::Display for $name { - fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { - self.0.fmt(f) - } - } - - impl OpArgType for $name {} - } -} - -newtype_oparg!( - #[derive(Clone, Copy)] - #[repr(transparent)] - pub struct ConstIdx(u32) -); - -newtype_oparg!( - #[derive(Clone, Copy)] - #[repr(transparent)] - pub struct VarNum(u32) -); - -newtype_oparg!( - #[derive(Clone, Copy)] - #[repr(transparent)] - pub struct VarNums(u32) -); - -newtype_oparg!( - #[derive(Clone, Copy)] - #[repr(transparent)] - pub struct LoadAttr(u32) -); - -newtype_oparg!( - #[derive(Clone, Copy)] - #[repr(transparent)] - pub struct LoadSuperAttr(u32) -); - -newtype_oparg!( - #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd)] - #[repr(transparent)] - pub struct Label(u32) -); +#[newtype_oparg] +pub struct Label; impl VarNums { #[must_use] diff --git a/crates/macros/Cargo.toml b/crates/macros/Cargo.toml new file mode 100644 index 00000000000..d0d3764376b --- /dev/null +++ b/crates/macros/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "rustpython-macros" +version.workspace = true +authors.workspace = true +edition.workspace = true +rust-version.workspace = true +repository.workspace = true +license.workspace = true + +[lib] +proc-macro = true +doctest = false + +[dependencies] +proc-macro2 = { workspace = true } +quote = { workspace = true } +syn = { workspace = true } + +[lints] +workspace = true diff --git a/crates/macros/src/lib.rs b/crates/macros/src/lib.rs new file mode 100644 index 00000000000..ee0d36debe9 --- /dev/null +++ b/crates/macros/src/lib.rs @@ -0,0 +1,24 @@ +//! This crate implements internal macros for the `rustpython` library. + +use proc_macro::TokenStream; +use syn::{Item, parse_macro_input}; + +mod newtype_oparg; + +#[proc_macro_attribute] +pub fn newtype_oparg(_metadata: TokenStream, input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as Item); + + let output = match input { + Item::Enum(data) => { + newtype_oparg::handle_enum(data).unwrap_or_else(|e| e.to_compile_error()) + } + Item::Struct(data) => { + newtype_oparg::handle_struct(data).unwrap_or_else(|e| e.to_compile_error()) + } + _ => syn::Error::new_spanned(input, "newtype_oparg only supports structs and enums") + .to_compile_error(), + }; + + output.into() +} diff --git a/crates/macros/src/newtype_oparg.rs b/crates/macros/src/newtype_oparg.rs new file mode 100644 index 00000000000..cf3eaa0213c --- /dev/null +++ b/crates/macros/src/newtype_oparg.rs @@ -0,0 +1,107 @@ +use syn::{Error, ItemEnum, ItemStruct, spanned::Spanned}; + +pub(super) fn handle_struct(item: ItemStruct) -> syn::Result { + if !item.fields.is_empty() { + return Err(Error::new( + item.span(), + "A new type oparg cannot have any fields.", + )); + } + + if !item.generics.params.is_empty() { + return Err(Error::new( + item.span(), + "A new type oparg cannot be generic.", + )); + } + + let ItemStruct { + attrs, + vis, + struct_token, + ident, + generics: _, + fields: _, + semi_token, + } = item; + + let semi_token = semi_token.unwrap_or_default(); + let output = quote::quote! { + #(#attrs)* + #[derive(Clone, Copy, Eq, Ord, PartialEq, PartialOrd)] + #vis #struct_token #ident(u32)#semi_token + + impl #ident { + #[must_use] + #vis const fn new(value: u32) -> Self { + Self::from_u32(value) + } + + #[must_use] + #vis const fn from_u32(value: u32) -> Self { + Self(value) + } + + /// Returns the oparg as a `u32` value. + #[must_use] + #vis const fn as_u32(self) -> u32 { + self.0 + } + + /// Returns the oparg as a `usize` value. + #[must_use] + #vis const fn as_usize(self) -> usize { + self.0 as usize + } + } + + impl From for #ident { + fn from(value: u32) -> Self { + Self::from_u32(value) + } + } + + impl From<#ident> for u32 { + fn from(value: #ident) -> Self { + value.0 + } + } + + impl From<#ident> for usize { + fn from(value: #ident) -> Self { + value.as_usize() + } + } + + impl ::core::fmt::Display for #ident { + fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + self.0.fmt(f) + } + } + + impl rustpython_compiler_core::bytecode::OpArgType for #ident {} + }; + + Ok(output) +} + +pub(super) fn handle_enum(item: ItemEnum) -> syn::Result { + let ItemEnum { + attrs, + vis, + enum_token, + ident, + generics: _, + fields: _, + variants, + } = item; + + let output = quote::quote! { + #(#attrs)* + #[derive(Clone, Copy, Eq, Ord, PartialEq, PartialOrd)] + #vis #enum_token #ident { + } + }; + + Ok(output) +} From b60f97249df025eab9ab06119c61f9a62c585c44 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Mon, 23 Mar 2026 14:16:46 +0100 Subject: [PATCH 02/13] Base enum support --- crates/compiler-core/src/bytecode/oparg.rs | 9 + crates/macros/Cargo.toml | 2 +- crates/macros/src/newtype_oparg.rs | 211 ++++++++++++++++++++- 3 files changed, 216 insertions(+), 6 deletions(-) diff --git a/crates/compiler-core/src/bytecode/oparg.rs b/crates/compiler-core/src/bytecode/oparg.rs index 8774b238490..d9f01b421d7 100644 --- a/crates/compiler-core/src/bytecode/oparg.rs +++ b/crates/compiler-core/src/bytecode/oparg.rs @@ -887,3 +887,12 @@ impl From for LoadSuperAttr { builder.build() } } + +#[newtype_oparg] +pub enum Foo { + A = 0, + #[oparg(display = "X")] + B = 1, + #[oparg(catch_all)] + C(u32), +} diff --git a/crates/macros/Cargo.toml b/crates/macros/Cargo.toml index d0d3764376b..2658b2f3e13 100644 --- a/crates/macros/Cargo.toml +++ b/crates/macros/Cargo.toml @@ -14,7 +14,7 @@ doctest = false [dependencies] proc-macro2 = { workspace = true } quote = { workspace = true } -syn = { workspace = true } +syn = { workspace = true, features = ["full"] } [lints] workspace = true diff --git a/crates/macros/src/newtype_oparg.rs b/crates/macros/src/newtype_oparg.rs index cf3eaa0213c..53ad62f4c44 100644 --- a/crates/macros/src/newtype_oparg.rs +++ b/crates/macros/src/newtype_oparg.rs @@ -1,4 +1,13 @@ -use syn::{Error, ItemEnum, ItemStruct, spanned::Spanned}; +use quote::quote; +use syn::{ + // Attribute, + Error, + // Expr, + Ident, + ItemEnum, + ItemStruct, + spanned::Spanned, +}; pub(super) fn handle_struct(item: ItemStruct) -> syn::Result { if !item.fields.is_empty() { @@ -26,7 +35,7 @@ pub(super) fn handle_struct(item: ItemStruct) -> syn::Result syn::Result, + display: Option, + catch_all: bool, +} + pub(super) fn handle_enum(item: ItemEnum) -> syn::Result { + if !item.generics.params.is_empty() { + return Err(Error::new( + item.span(), + "A new type oparg cannot be generic.", + )); + } + let ItemEnum { attrs, vis, enum_token, ident, generics: _, - fields: _, + brace_token: _, variants, - } = item; + } = item.clone(); + + let mut variants_info = variants + .iter() + .map(|variant| { + let ident = variant.ident.clone(); + let discriminant = variant.discriminant.as_ref().map(|(_, expr)| expr.clone()); + + let mut display = None; + let mut catch_all = false; + for attr in &variant.attrs { + if !attr.path().is_ident("oparg") { + continue; + } + + attr.parse_nested_meta(|meta| { + if meta.path.is_ident("display") { + let value = meta.value()?.parse::()?; + display = Some(value.value()); + Ok(()) + } else if meta.path.is_ident("catch_all") { + catch_all = true; + Ok(()) + } else { + Err(meta.error("unknown oparg attribute")) + } + }) + .unwrap(); + } + + VariantInfo { + ident, + discriminant, + display, + catch_all, + } + }) + .collect::>(); + + let catch_all = variants_info.pop_if(|info| info.catch_all); + + // Ensure a no multiple `#[oparg(catch_all)]` + if catch_all.is_some() && variants_info.iter().any(|vinfo| vinfo.catch_all) { + return Err(Error::new( + item.span(), + "Cannot define more than one `#[oparg(catch_all)]`", + )); + }; + + match catch_all { + Some(vinfo) if vinfo.display.is_some() => { + return Err(Error::new( + vinfo.ident.span(), + r#"Cannot define both `#[oparg(catch_all)`] and `#[oparg(display = "...")]` on the same variant"#, + )); + } + _ => {} + }; + + // Ensure all variants has a discriminant. + for vinfo in &variants_info { + if vinfo.discriminant.is_none() { + return Err(Error::new( + vinfo.ident.span(), + "Is a variant without an assigned value", + )); + } + } + + let variants_def = variants.iter().cloned().map(|mut variant| { + // Don't assign value. Enables more optimizations by the compiler. + variant.discriminant = None; + + // Remove `#[oparg(...)`. + variant.attrs.retain(|attr| !attr.path().is_ident("oparg")); + + variant + }); - let output = quote::quote! { + let from_u32_arms = variants_info.iter().map(|vinfo| { + let ident = &vinfo.ident; + let discriminant = &vinfo.discriminant; + + quote! { + #discriminant => Self::#ident, + } + }); + + // If we have a `catch_all` we can implement `From`. Otherwise impl `TryFrom` + let impl_from_u32 = match catch_all { + Some(ref vinfo) => { + let vinfo_ident = &vinfo.ident; + quote! { + impl From for #ident { + fn from(value: u32) -> Self { + match value { + #(#from_u32_arms)* + _ => Self::#vinfo_ident(value) + } + } + } + } + } + None => quote! { + impl TryFrom for #ident { + type Error = rustpython_compiler_core::marshal::MarshalError; + + fn try_from(value: u32) -> Result { + Ok( + match value { + #(#from_u32_arms)* + _ => return Err(Self::Error::InvalidBytecode), + } + ) + } + } + }, + }; + + let mut into_u32_arms = vec![]; + let mut display_arms = vec![]; + + for vinfo in &variants_info { + let VariantInfo { + ident: vinfo_ident, + discriminant, + display, + .. + } = &vinfo; + + into_u32_arms.push(quote! { + #ident::#vinfo_ident => #discriminant, + }); + + let display_arm = match display { + Some(v) => quote! { + Self::#vinfo_ident => write!(f, "{}", #v), + }, + None => quote! { + Self::#vinfo_ident => write!(f, "{}", #discriminant), + }, + }; + + display_arms.push(display_arm); + } + + if let Some(ref vinfo) = catch_all { + let vinfo_ident = &vinfo.ident; + + into_u32_arms.push(quote! { + #ident::#vinfo_ident(v) => v, + }); + + display_arms.push(quote! { + Self::#vinfo_ident(v) => write!(f, "{}", v), + }); + } + + let output = quote! { #(#attrs)* #[derive(Clone, Copy, Eq, Ord, PartialEq, PartialOrd)] #vis #enum_token #ident { + #(#variants_def),* + } + + #impl_from_u32 + + impl From<#ident> for u32 { + fn from(value: #ident) -> Self { + match value { + #(#into_u32_arms)* + } + } + } + + impl ::core::fmt::Display for #ident { + fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + match self { + #(#display_arms)* + } + } } + + impl rustpython_compiler_core::bytecode::OpArgType for #ident {} }; Ok(output) From 936a0791d9b718dc49f17318596d447b1156720f Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Mon, 23 Mar 2026 21:19:40 +0100 Subject: [PATCH 03/13] Modify code for new macro --- crates/compiler-core/src/bytecode/oparg.rs | 504 +++++++++------------ crates/macros/src/newtype_oparg.rs | 4 +- crates/vm/src/frame.rs | 16 +- 3 files changed, 221 insertions(+), 303 deletions(-) diff --git a/crates/compiler-core/src/bytecode/oparg.rs b/crates/compiler-core/src/bytecode/oparg.rs index d9f01b421d7..aee0301b666 100644 --- a/crates/compiler-core/src/bytecode/oparg.rs +++ b/crates/compiler-core/src/bytecode/oparg.rs @@ -280,109 +280,71 @@ impl fmt::Display for ConvertValueOparg { } /// Resume type for the RESUME instruction -#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] +#[newtype_oparg] pub enum ResumeType { - AtFuncStart, - AfterYield, - AfterYieldFrom, - AfterAwait, + AtFuncStart = 0, + AfterYield = 1, + AfterYieldFrom = 2, + AfterAwait = 3, + #[oparg(catch_all)] Other(u32), } -impl From for ResumeType { - fn from(value: u32) -> Self { - match value { - 0 => Self::AtFuncStart, - 1 => Self::AfterYield, - 2 => Self::AfterYieldFrom, - 3 => Self::AfterAwait, - _ => Self::Other(value), - } - } -} - -impl From for u32 { - fn from(typ: ResumeType) -> Self { - match typ { - ResumeType::AtFuncStart => 0, - ResumeType::AfterYield => 1, - ResumeType::AfterYieldFrom => 2, - ResumeType::AfterAwait => 3, - ResumeType::Other(v) => v, - } - } -} - -impl core::fmt::Display for ResumeType { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - u32::from(*self).fmt(f) - } -} - -impl OpArgType for ResumeType {} - pub type NameIdx = u32; impl OpArgType for u32 {} -oparg_enum!( - /// The kind of Raise that occurred. - #[derive(Copy, Clone, Debug, PartialEq, Eq)] - pub enum RaiseKind { - /// Bare `raise` statement with no arguments. - /// Gets the current exception from VM state (topmost_exception). - /// Maps to RAISE_VARARGS with oparg=0. - BareRaise = 0, - /// `raise exc` - exception is on the stack. - /// Maps to RAISE_VARARGS with oparg=1. - Raise = 1, - /// `raise exc from cause` - exception and cause are on the stack. - /// Maps to RAISE_VARARGS with oparg=2. - RaiseCause = 2, - /// Reraise exception from the stack top. - /// Used in exception handler cleanup blocks (finally, except). - /// Gets exception from stack, not from VM state. - /// Maps to the RERAISE opcode. - ReraiseFromStack = 3, - } -); - -oparg_enum!( - /// Intrinsic function for CALL_INTRINSIC_1 - #[derive(Copy, Clone, Debug, PartialEq, Eq)] - pub enum IntrinsicFunction1 { - // Invalid = 0, - Print = 1, - /// Import * operation - ImportStar = 2, - /// Convert StopIteration to RuntimeError in async context - StopIterationError = 3, - AsyncGenWrap = 4, - UnaryPositive = 5, - /// Convert list to tuple - ListToTuple = 6, - /// Type parameter related - TypeVar = 7, - ParamSpec = 8, - TypeVarTuple = 9, - /// Generic subscript for PEP 695 - SubscriptGeneric = 10, - TypeAlias = 11, - } -); +/// The kind of Raise that occurred. +#[newtype_oparg] +pub enum RaiseKind { + /// Bare `raise` statement with no arguments. + /// Gets the current exception from VM state (topmost_exception). + /// Maps to RAISE_VARARGS with oparg=0. + BareRaise = 0, + /// `raise exc` - exception is on the stack. + /// Maps to RAISE_VARARGS with oparg=1. + Raise = 1, + /// `raise exc from cause` - exception and cause are on the stack. + /// Maps to RAISE_VARARGS with oparg=2. + RaiseCause = 2, + /// Reraise exception from the stack top. + /// Used in exception handler cleanup blocks (finally, except). + /// Gets exception from stack, not from VM state. + /// Maps to the RERAISE opcode. + ReraiseFromStack = 3, +} -oparg_enum!( - /// Intrinsic function for CALL_INTRINSIC_2 - #[derive(Copy, Clone, Debug, PartialEq, Eq)] - pub enum IntrinsicFunction2 { - PrepReraiseStar = 1, - TypeVarWithBound = 2, - TypeVarWithConstraint = 3, - SetFunctionTypeParams = 4, - /// Set default value for type parameter (PEP 695) - SetTypeparamDefault = 5, - } -); +#[newtype_oparg] +pub enum IntrinsicFunction1 { + // Invalid = 0, + Print = 1, + /// Import * operation + ImportStar = 2, + /// Convert StopIteration to RuntimeError in async context + StopIterationError = 3, + AsyncGenWrap = 4, + UnaryPositive = 5, + /// Convert list to tuple + ListToTuple = 6, + /// Type parameter related + TypeVar = 7, + ParamSpec = 8, + TypeVarTuple = 9, + /// Generic subscript for PEP 695 + SubscriptGeneric = 10, + TypeAlias = 11, +} + +/// Intrinsic function for CALL_INTRINSIC_2 +#[newtype_oparg] +pub enum IntrinsicFunction2 { + PrepReraiseStar = 1, + TypeVarWithBound = 2, + TypeVarWithConstraint = 3, + SetFunctionTypeParams = 4, + /// Set default value for type parameter (PEP 695) + SetTypeparamDefault = 5, +} bitflagset::bitflag! { /// `SET_FUNCTION_ATTRIBUTE` flags. @@ -491,77 +453,102 @@ impl From for u32 { impl OpArgType for ComparisonOperator {} -oparg_enum!( - /// The possible Binary operators - /// - /// # Examples - /// - /// ```rust - /// use rustpython_compiler_core::bytecode::{Arg, BinaryOperator, Instruction}; - /// let (op, _) = Arg::new(BinaryOperator::Add); - /// let instruction = Instruction::BinaryOp { op }; - /// ``` - /// - /// See also: - /// - [_PyEval_BinaryOps](https://github.com/python/cpython/blob/8183fa5e3f78ca6ab862de7fb8b14f3d929421e0/Python/ceval.c#L316-L343) - #[derive(Clone, Copy, Debug, Eq, PartialEq)] - pub enum BinaryOperator { - /// `+` - Add = 0, - /// `&` - And = 1, - /// `//` - FloorDivide = 2, - /// `<<` - Lshift = 3, - /// `@` - MatrixMultiply = 4, - /// `*` - Multiply = 5, - /// `%` - Remainder = 6, - /// `|` - Or = 7, - /// `**` - Power = 8, - /// `>>` - Rshift = 9, - /// `-` - Subtract = 10, - /// `/` - TrueDivide = 11, - /// `^` - Xor = 12, - /// `+=` - InplaceAdd = 13, - /// `&=` - InplaceAnd = 14, - /// `//=` - InplaceFloorDivide = 15, - /// `<<=` - InplaceLshift = 16, - /// `@=` - InplaceMatrixMultiply = 17, - /// `*=` - InplaceMultiply = 18, - /// `%=` - InplaceRemainder = 19, - /// `|=` - InplaceOr = 20, - /// `**=` - InplacePower = 21, - /// `>>=` - InplaceRshift = 22, - /// `-=` - InplaceSubtract = 23, - /// `/=` - InplaceTrueDivide = 24, - /// `^=` - InplaceXor = 25, - /// `[]` subscript - Subscr = 26, - } -); +/// The possible Binary operators +/// +/// # Examples +/// +/// ```rust +/// use rustpython_compiler_core::bytecode::{Arg, BinaryOperator, Instruction}; +/// let (op, _) = Arg::new(BinaryOperator::Add); +/// let instruction = Instruction::BinaryOp { op }; +/// ``` +/// +/// See also: +/// - [_PyEval_BinaryOps](https://github.com/python/cpython/blob/8183fa5e3f78ca6ab862de7fb8b14f3d929421e0/Python/ceval.c#L316-L343) +#[newtype_oparg] +pub enum BinaryOperator { + /// `+` + #[oparg(display = "+")] + Add = 0, + /// `&` + #[oparg(display = "&")] + And = 1, + /// `//` + #[oparg(display = "//")] + FloorDivide = 2, + /// `<<` + #[oparg(display = "<<")] + Lshift = 3, + /// `@` + #[oparg(display = "@")] + MatrixMultiply = 4, + /// `*` + #[oparg(display = "*")] + Multiply = 5, + /// `%` + #[oparg(display = "%")] + Remainder = 6, + /// `|` + #[oparg(display = "|")] + Or = 7, + /// `**` + #[oparg(display = "**")] + Power = 8, + /// `>>` + #[oparg(display = ">>")] + Rshift = 9, + /// `-` + #[oparg(display = "-")] + Subtract = 10, + /// `/` + #[oparg(display = "/")] + TrueDivide = 11, + /// `^` + #[oparg(display = "^")] + Xor = 12, + /// `+=` + #[oparg(display = "+=")] + InplaceAdd = 13, + /// `&=` + #[oparg(display = "&=")] + InplaceAnd = 14, + /// `//=` + #[oparg(display = "//=")] + InplaceFloorDivide = 15, + /// `<<=` + #[oparg(display = "<<=")] + InplaceLshift = 16, + /// `@=` + #[oparg(display = "@=")] + InplaceMatrixMultiply = 17, + /// `*=` + #[oparg(display = "*=")] + InplaceMultiply = 18, + /// `%=` + #[oparg(display = "%=")] + InplaceRemainder = 19, + /// `|=` + #[oparg(display = "|=")] + InplaceOr = 20, + /// `**=` + #[oparg(display = "**=")] + InplacePower = 21, + /// `>>=` + #[oparg(display = ">>=")] + InplaceRshift = 22, + /// `-=` + #[oparg(display = "-=")] + InplaceSubtract = 23, + /// `/=` + #[oparg(display = "/=")] + InplaceTrueDivide = 24, + /// `^=` + #[oparg(display = "^=")] + InplaceXor = 25, + /// `[]` subscript + #[oparg(display = "[]")] + Subscr = 26, +} impl BinaryOperator { /// Get the "inplace" version of the operator. @@ -596,131 +583,71 @@ impl BinaryOperator { } } -impl fmt::Display for BinaryOperator { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let op = match self { - Self::Add => "+", - Self::And => "&", - Self::FloorDivide => "//", - Self::Lshift => "<<", - Self::MatrixMultiply => "@", - Self::Multiply => "*", - Self::Remainder => "%", - Self::Or => "|", - Self::Power => "**", - Self::Rshift => ">>", - Self::Subtract => "-", - Self::TrueDivide => "/", - Self::Xor => "^", - Self::InplaceAdd => "+=", - Self::InplaceAnd => "&=", - Self::InplaceFloorDivide => "//=", - Self::InplaceLshift => "<<=", - Self::InplaceMatrixMultiply => "@=", - Self::InplaceMultiply => "*=", - Self::InplaceRemainder => "%=", - Self::InplaceOr => "|=", - Self::InplacePower => "**=", - Self::InplaceRshift => ">>=", - Self::InplaceSubtract => "-=", - Self::InplaceTrueDivide => "/=", - Self::InplaceXor => "^=", - Self::Subscr => "[]", - }; - write!(f, "{op}") - } +/// Whether or not to invert the operation. +#[newtype_oparg] +pub enum Invert { + /// ```py + /// foo is bar + /// x in lst + /// ``` + No = 0, + /// ```py + /// foo is not bar + /// x not in lst + /// ``` + Yes = 1, } -oparg_enum!( - /// Whether or not to invert the operation. - #[derive(Debug, Copy, Clone, PartialEq, Eq)] - pub enum Invert { - /// ```py - /// foo is bar - /// x in lst - /// ``` - No = 0, - /// ```py - /// foo is not bar - /// x not in lst - /// ``` - Yes = 1, - } -); - -oparg_enum!( - /// Special method for LOAD_SPECIAL opcode (context managers). - #[derive(Debug, Copy, Clone, PartialEq, Eq)] - pub enum SpecialMethod { - /// `__enter__` for sync context manager - Enter = 0, - /// `__exit__` for sync context manager - Exit = 1, - /// `__aenter__` for async context manager - AEnter = 2, - /// `__aexit__` for async context manager - AExit = 3, - } -); - -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}") - } +/// Special method for LOAD_SPECIAL opcode (context managers). +#[newtype_oparg] +pub enum SpecialMethod { + /// `__enter__` for sync context manager + #[oparg(display = "__enter__")] + Enter = 0, + /// `__exit__` for sync context manager + #[oparg(display = "__exit__")] + Exit = 1, + /// `__aenter__` for async context manager + #[oparg(display = "__aenter__")] + AEnter = 2, + /// `__aexit__` for async context manager + #[oparg(display = "__aexit__")] + AExit = 3, +} + +/// Common constants for LOAD_COMMON_CONSTANT opcode. +/// pycore_opcode_utils.h CONSTANT_* +#[newtype_oparg] +pub enum CommonConstant { + /// `AssertionError` exception type + #[oparg(display = "AssertionError")] + AssertionError = 0, + /// `NotImplementedError` exception type + #[oparg(display = "NotImplementedError")] + NotImplementedError = 1, + /// Built-in `tuple` type + #[oparg(display = "tuple")] + BuiltinTuple = 2, + /// Built-in `all` function + #[oparg(display = "all")] + BuiltinAll = 3, + /// Built-in `any` function + #[oparg(display = "any")] + BuiltinAny = 4, } -oparg_enum!( - /// Common constants for LOAD_COMMON_CONSTANT opcode. - /// pycore_opcode_utils.h CONSTANT_* - #[derive(Debug, Copy, Clone, PartialEq, Eq)] - pub enum CommonConstant { - /// `AssertionError` exception type - AssertionError = 0, - /// `NotImplementedError` exception type - NotImplementedError = 1, - /// Built-in `tuple` type - BuiltinTuple = 2, - /// Built-in `all` function - BuiltinAll = 3, - /// Built-in `any` function - BuiltinAny = 4, - } -); - -impl fmt::Display for CommonConstant { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let name = match self { - Self::AssertionError => "AssertionError", - Self::NotImplementedError => "NotImplementedError", - Self::BuiltinTuple => "tuple", - Self::BuiltinAll => "all", - Self::BuiltinAny => "any", - }; - write!(f, "{name}") - } +#[newtype_oparg] +pub enum BuildSliceArgCount { + /// ```py + /// x[5:10] + /// ``` + Two = 2, + /// ```py + /// x[5:10:2] + /// ``` + Three = 3, } -oparg_enum!( - /// Specifies if a slice is built with either 2 or 3 arguments. - #[derive(Clone, Copy, Debug, Eq, PartialEq)] - pub enum BuildSliceArgCount { - /// ```py - /// x[5:10] - /// ``` - Two = 2, - /// ```py - /// x[5:10:2] - /// ``` - Three = 3, - } -); - #[derive(Copy, Clone)] pub struct UnpackExArgs { pub before: u8, @@ -887,12 +814,3 @@ impl From for LoadSuperAttr { builder.build() } } - -#[newtype_oparg] -pub enum Foo { - A = 0, - #[oparg(display = "X")] - B = 1, - #[oparg(catch_all)] - C(u32), -} diff --git a/crates/macros/src/newtype_oparg.rs b/crates/macros/src/newtype_oparg.rs index 53ad62f4c44..d3419206723 100644 --- a/crates/macros/src/newtype_oparg.rs +++ b/crates/macros/src/newtype_oparg.rs @@ -37,7 +37,7 @@ pub(super) fn handle_struct(item: ItemStruct) -> syn::Result syn::Result { let found = dict.get_item_opt(a, vm)?.is_some(); self.pop_value(); self.pop_value(); - let invert = bytecode::Invert::try_from(u32::from(arg) as u8) - .unwrap_or(bytecode::Invert::No); + let invert = + bytecode::Invert::try_from(u32::from(arg)).unwrap_or(bytecode::Invert::No); let value = match invert { bytecode::Invert::No => found, bytecode::Invert::Yes => !found, @@ -5475,8 +5475,8 @@ impl ExecutingFrame<'_> { } else { let b = self.pop_value(); let a = self.pop_value(); - let invert = bytecode::Invert::try_from(u32::from(arg) as u8) - .unwrap_or(bytecode::Invert::No); + let invert = + bytecode::Invert::try_from(u32::from(arg)).unwrap_or(bytecode::Invert::No); let value = match invert { bytecode::Invert::No => self._in(vm, &a, &b)?, bytecode::Invert::Yes => self._not_in(vm, &a, &b)?, @@ -5494,8 +5494,8 @@ impl ExecutingFrame<'_> { let found = vm._contains(b, a)?; self.pop_value(); self.pop_value(); - let invert = bytecode::Invert::try_from(u32::from(arg) as u8) - .unwrap_or(bytecode::Invert::No); + let invert = + bytecode::Invert::try_from(u32::from(arg)).unwrap_or(bytecode::Invert::No); let value = match invert { bytecode::Invert::No => found, bytecode::Invert::Yes => !found, @@ -5505,8 +5505,8 @@ impl ExecutingFrame<'_> { } else { let b = self.pop_value(); let a = self.pop_value(); - let invert = bytecode::Invert::try_from(u32::from(arg) as u8) - .unwrap_or(bytecode::Invert::No); + let invert = + bytecode::Invert::try_from(u32::from(arg)).unwrap_or(bytecode::Invert::No); let value = match invert { bytecode::Invert::No => self._in(vm, &a, &b)?, bytecode::Invert::Yes => self._not_in(vm, &a, &b)?, From 94d66dd1550f702b347afb56eaee62e7ffae5403 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Mon, 23 Mar 2026 21:21:31 +0100 Subject: [PATCH 04/13] No need for full feature --- crates/macros/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/macros/Cargo.toml b/crates/macros/Cargo.toml index 2658b2f3e13..d0d3764376b 100644 --- a/crates/macros/Cargo.toml +++ b/crates/macros/Cargo.toml @@ -14,7 +14,7 @@ doctest = false [dependencies] proc-macro2 = { workspace = true } quote = { workspace = true } -syn = { workspace = true, features = ["full"] } +syn = { workspace = true } [lints] workspace = true From f5c2f9b4f0291b14dc37f2361fc9f61b81cc37e5 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Mon, 23 Mar 2026 21:22:33 +0100 Subject: [PATCH 05/13] Remove debug code --- crates/macros/src/newtype_oparg.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/crates/macros/src/newtype_oparg.rs b/crates/macros/src/newtype_oparg.rs index d3419206723..98666611777 100644 --- a/crates/macros/src/newtype_oparg.rs +++ b/crates/macros/src/newtype_oparg.rs @@ -1,13 +1,5 @@ use quote::quote; -use syn::{ - // Attribute, - Error, - // Expr, - Ident, - ItemEnum, - ItemStruct, - spanned::Spanned, -}; +use syn::{Error, Ident, ItemEnum, ItemStruct, spanned::Spanned}; pub(super) fn handle_struct(item: ItemStruct) -> syn::Result { if !item.fields.is_empty() { From 9fbc6d30e9e7bfcd62a20507fad9f3736d2b8b7e Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Mon, 23 Mar 2026 21:26:35 +0100 Subject: [PATCH 06/13] Impl hash --- crates/macros/src/newtype_oparg.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/macros/src/newtype_oparg.rs b/crates/macros/src/newtype_oparg.rs index 98666611777..b33c2f95805 100644 --- a/crates/macros/src/newtype_oparg.rs +++ b/crates/macros/src/newtype_oparg.rs @@ -29,7 +29,7 @@ pub(super) fn handle_struct(item: ItemStruct) -> syn::Result syn::Result Date: Tue, 24 Mar 2026 11:00:05 +0100 Subject: [PATCH 07/13] syn full feature --- crates/macros/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/macros/Cargo.toml b/crates/macros/Cargo.toml index d0d3764376b..2658b2f3e13 100644 --- a/crates/macros/Cargo.toml +++ b/crates/macros/Cargo.toml @@ -14,7 +14,7 @@ doctest = false [dependencies] proc-macro2 = { workspace = true } quote = { workspace = true } -syn = { workspace = true } +syn = { workspace = true, features = ["full"] } [lints] workspace = true From f21d855f77e1cdfbfe6c0f7ad0532f74ee2d638c Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Tue, 24 Mar 2026 11:08:03 +0100 Subject: [PATCH 08/13] Remove unwrap --- crates/macros/src/newtype_oparg.rs | 62 ++++++++++++++---------------- 1 file changed, 29 insertions(+), 33 deletions(-) diff --git a/crates/macros/src/newtype_oparg.rs b/crates/macros/src/newtype_oparg.rs index b33c2f95805..2427cbd2320 100644 --- a/crates/macros/src/newtype_oparg.rs +++ b/crates/macros/src/newtype_oparg.rs @@ -86,7 +86,6 @@ pub(super) fn handle_struct(item: ItemStruct) -> syn::Result, @@ -112,42 +111,39 @@ pub(super) fn handle_enum(item: ItemEnum) -> syn::Result()?; - display = Some(value.value()); - Ok(()) - } else if meta.path.is_ident("catch_all") { - catch_all = true; - Ok(()) - } else { - Err(meta.error("unknown oparg attribute")) - } - }) - .unwrap(); + let mut display = None; + let mut catch_all = false; + for attr in &variant.attrs { + if !attr.path().is_ident("oparg") { + continue; } - VariantInfo { - ident, - discriminant, - display, - catch_all, - } + attr.parse_nested_meta(|meta| { + if meta.path.is_ident("display") { + let value = meta.value()?.parse::()?; + display = Some(value.value()); + Ok(()) + } else if meta.path.is_ident("catch_all") { + catch_all = true; + Ok(()) + } else { + Err(meta.error("unknown oparg attribute")) + } + })? + } + + variants_info.push(VariantInfo { + ident, + discriminant, + display, + catch_all, }) - .collect::>(); + } let catch_all = variants_info.pop_if(|info| info.catch_all); From 77eae154111f8174dcbb9b3ae79870f3304a7252 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Tue, 24 Mar 2026 11:27:39 +0100 Subject: [PATCH 09/13] Use `TryFrom` --- crates/macros/src/newtype_oparg.rs | 89 +++++++++++++++--------------- 1 file changed, 46 insertions(+), 43 deletions(-) diff --git a/crates/macros/src/newtype_oparg.rs b/crates/macros/src/newtype_oparg.rs index 2427cbd2320..1ca33f0c2cc 100644 --- a/crates/macros/src/newtype_oparg.rs +++ b/crates/macros/src/newtype_oparg.rs @@ -93,29 +93,10 @@ struct VariantInfo { catch_all: bool, } -pub(super) fn handle_enum(item: ItemEnum) -> syn::Result { - if !item.generics.params.is_empty() { - return Err(Error::new( - item.span(), - "A new type oparg cannot be generic.", - )); - } - - let ItemEnum { - attrs, - vis, - enum_token, - ident, - generics: _, - brace_token: _, - variants, - } = item.clone(); - - let mut variants_info = vec![]; - for variant in &variants { - let ident = variant.ident.clone(); - let discriminant = variant.discriminant.as_ref().map(|(_, expr)| expr.clone()); +impl TryFrom for VariantInfo { + type Error = syn::Error; + fn try_from(variant: syn::Variant) -> Result { let mut display = None; let mut catch_all = false; for attr in &variant.attrs { @@ -137,13 +118,55 @@ pub(super) fn handle_enum(item: ItemEnum) -> syn::Result syn::Result { + if !item.generics.params.is_empty() { + return Err(Error::new( + item.span(), + "A new type oparg cannot be generic.", + )); + } + + let ItemEnum { + attrs, + vis, + enum_token, + ident, + generics: _, + brace_token: _, + variants, + } = item.clone(); + + let mut variants_info = variants + .iter() + .cloned() + .map(VariantInfo::try_from) + .collect::>>()?; let catch_all = variants_info.pop_if(|info| info.catch_all); @@ -155,26 +178,6 @@ pub(super) fn handle_enum(item: ItemEnum) -> syn::Result { - return Err(Error::new( - vinfo.ident.span(), - r#"Cannot define both `#[oparg(catch_all)`] and `#[oparg(display = "...")]` on the same variant"#, - )); - } - _ => {} - }; - - // Ensure all variants has a discriminant. - for vinfo in &variants_info { - if vinfo.discriminant.is_none() { - return Err(Error::new( - vinfo.ident.span(), - "Is a variant without an assigned value", - )); - } - } - let variants_def = variants.iter().cloned().map(|mut variant| { // Don't assign value. Enables more optimizations by the compiler. variant.discriminant = None; From 4d6c72573553bff1761d343b41318f02e1ee9990 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Tue, 24 Mar 2026 11:38:33 +0100 Subject: [PATCH 10/13] Convert `ConvertValueOparg` to new macro --- crates/compiler-core/src/bytecode/oparg.rs | 192 ++++----------------- 1 file changed, 35 insertions(+), 157 deletions(-) diff --git a/crates/compiler-core/src/bytecode/oparg.rs b/crates/compiler-core/src/bytecode/oparg.rs index aee0301b666..de09a17962e 100644 --- a/crates/compiler-core/src/bytecode/oparg.rs +++ b/crates/compiler-core/src/bytecode/oparg.rs @@ -116,167 +116,45 @@ impl OpArgState { } } -/// Helper macro for defining oparg enums in an optimal way. +/// Oparg values for [`Instruction::ConvertValue`]. /// -/// Will generate the following: +/// ## See also /// -/// - Enum which variant's aren't assigned any value (for optimizations). -/// - impl [`TryFrom`] -/// - impl [`TryFrom`] -/// - impl [`Into`] -/// - impl [`Into`] -/// - impl [`OpArgType`] -/// -/// # Note -/// If an enum variant has "alternative" values (i.e. `Foo = 0 | 1`), the first value will be the -/// result of converting to a number. -/// -/// # Examples -/// -/// ```ignore -/// oparg_enum!( -/// /// Oparg for the `X` opcode. -/// #[derive(Clone, Copy)] -/// pub enum MyOpArg { -/// /// Some doc. -/// Foo = 4, -/// Bar = 8, -/// Baz = 15 | 16, -/// Qux = 23 | 42 -/// } -/// ); -/// ``` -macro_rules! oparg_enum { - ( - $(#[$enum_meta:meta])* - $vis:vis enum $name:ident { - $( - $(#[$variant_meta:meta])* - $variant:ident = $value:literal $(| $alternatives:expr)* - ),* $(,)? - } - ) => { - $(#[$enum_meta])* - $vis enum $name { - $( - $(#[$variant_meta])* - $variant, // Do assign value to variant. - )* - } - - impl_oparg_enum!( - enum $name { - $( - $variant = $value $(| $alternatives)*, - )* - } - ); - }; -} - -macro_rules! impl_oparg_enum { - ( - enum $name:ident { - $( - $variant:ident = $value:literal $(| $alternatives:expr)* - ),* $(,)? - } - ) => { - impl TryFrom for $name { - type Error = $crate::marshal::MarshalError; - - fn try_from(value: u8) -> Result { - Ok(match value { - $( - $value $(| $alternatives)* => Self::$variant, - )* - _ => return Err(Self::Error::InvalidBytecode), - }) - } - } - - impl TryFrom for $name { - type Error = $crate::marshal::MarshalError; - - fn try_from(value: u32) -> Result { - u8::try_from(value) - .map_err(|_| Self::Error::InvalidBytecode) - .map(TryInto::try_into)? - } - } - - impl From<$name> for u8 { - fn from(value: $name) -> Self { - match value { - $( - $name::$variant => $value, - )* - } - } - } - - impl From<$name> for u32 { - fn from(value: $name) -> Self { - Self::from(u8::from(value)) - } - } - - impl OpArgType for $name {} - }; -} - -oparg_enum!( - /// Oparg values for [`Instruction::ConvertValue`]. +/// - [CPython FVC_* flags](https://github.com/python/cpython/blob/8183fa5e3f78ca6ab862de7fb8b14f3d929421e0/Include/ceval.h#L129-L132) +#[newtype_oparg] +pub enum ConvertValueOparg { + /// No conversion. /// - /// ## See also + /// ```python + /// f"{x}" + /// f"{x:4}" + /// ``` + #[oparg(display = "")] + None = 0, + /// Converts by calling `str()`. /// - /// - [CPython FVC_* flags](https://github.com/python/cpython/blob/8183fa5e3f78ca6ab862de7fb8b14f3d929421e0/Include/ceval.h#L129-L132) - #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] - pub enum ConvertValueOparg { - /// No conversion. - /// - /// ```python - /// f"{x}" - /// f"{x:4}" - /// ``` - // Ruff `ConversionFlag::None` is `-1i8`, when its converted to `u8` its value is `u8::MAX`. - None = 0 | 255, - /// Converts by calling `str()`. - /// - /// ```python - /// f"{x!s}" - /// f"{x!s:2}" - /// ``` - Str = 1, - /// Converts by calling `repr()`. - /// - /// ```python - /// f"{x!r}" - /// f"{x!r:2}" - /// ``` - Repr = 2, - /// Converts by calling `ascii()`. - /// - /// ```python - /// f"{x!a}" - /// f"{x!a:2}" - /// ``` - Ascii = 3, - } -); - -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}") - } + /// ```python + /// f"{x!s}" + /// f"{x!s:2}" + /// ``` + #[oparg(display = "1 (str)")] + Str = 1, + /// Converts by calling `repr()`. + /// + /// ```python + /// f"{x!r}" + /// f"{x!r:2}" + /// ``` + #[oparg(display = "2 (repr)")] + Repr = 2, + /// Converts by calling `ascii()`. + /// + /// ```python + /// f"{x!a}" + /// f"{x!a:2}" + /// ``` + #[oparg(display = "3 (ascii)")] + Ascii = 3, } /// Resume type for the RESUME instruction From c702fbefb9c7e396d39d03c627057a57fc5dccf8 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Tue, 24 Mar 2026 12:43:04 +0100 Subject: [PATCH 11/13] Fix bug where `catch_all` is not the last element --- crates/macros/src/newtype_oparg.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/macros/src/newtype_oparg.rs b/crates/macros/src/newtype_oparg.rs index 1ca33f0c2cc..5dd18ad20ee 100644 --- a/crates/macros/src/newtype_oparg.rs +++ b/crates/macros/src/newtype_oparg.rs @@ -162,21 +162,22 @@ pub(super) fn handle_enum(item: ItemEnum) -> syn::Result, Vec<_>) = variants .iter() .cloned() .map(VariantInfo::try_from) - .collect::>>()?; - - let catch_all = variants_info.pop_if(|info| info.catch_all); + .collect::>>()? + .into_iter() + .partition(|vinfo| vinfo.catch_all); // Ensure a no multiple `#[oparg(catch_all)]` - if catch_all.is_some() && variants_info.iter().any(|vinfo| vinfo.catch_all) { + let catch_all = catch_all_variants.pop(); + if !catch_all_variants.is_empty() { return Err(Error::new( item.span(), "Cannot define more than one `#[oparg(catch_all)]`", )); - }; + } let variants_def = variants.iter().cloned().map(|mut variant| { // Don't assign value. Enables more optimizations by the compiler. From 796b61c955701a55f9a6f8ad2b85d0ce6d462def Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Tue, 24 Mar 2026 12:50:49 +0100 Subject: [PATCH 12/13] Don't match on 255 --- crates/vm/src/stdlib/_ast/other.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/vm/src/stdlib/_ast/other.rs b/crates/vm/src/stdlib/_ast/other.rs index 5c0803ac594..0e3942790a8 100644 --- a/crates/vm/src/stdlib/_ast/other.rs +++ b/crates/vm/src/stdlib/_ast/other.rs @@ -12,9 +12,8 @@ impl Node for ast::ConversionFlag { object: PyObjectRef, ) -> PyResult { // Python's AST uses ASCII codes: 's', 'r', 'a', -1=None - // Note: 255 is -1i8 as u8 (ruff's ConversionFlag::None) match i32::try_from_object(vm, object)? { - -1 | 255 => Ok(Self::None), + -1 => Ok(Self::None), x if x == b's' as i32 => Ok(Self::Str), x if x == b'r' as i32 => Ok(Self::Repr), x if x == b'a' as i32 => Ok(Self::Ascii), From 9691476d2ee059d4f61993e44cff18ecd1c08f58 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Tue, 24 Mar 2026 13:11:39 +0100 Subject: [PATCH 13/13] Validate `catch_all` variant drfinition --- crates/macros/src/newtype_oparg.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/macros/src/newtype_oparg.rs b/crates/macros/src/newtype_oparg.rs index 5dd18ad20ee..71e5234a5a6 100644 --- a/crates/macros/src/newtype_oparg.rs +++ b/crates/macros/src/newtype_oparg.rs @@ -97,6 +97,9 @@ impl TryFrom for VariantInfo { type Error = syn::Error; fn try_from(variant: syn::Variant) -> Result { + let ident = variant.ident.clone(); + let discriminant = variant.discriminant.as_ref().map(|(_, expr)| expr.clone()); + let mut display = None; let mut catch_all = false; for attr in &variant.attrs { @@ -118,8 +121,14 @@ impl TryFrom for VariantInfo { })? } - let ident = variant.ident.clone(); - let discriminant = variant.discriminant.as_ref().map(|(_, expr)| expr.clone()); + if catch_all + && !matches!(&variant.fields, syn::Fields::Unnamed(fields) if fields.unnamed.len() == 1) + { + return Err(Error::new( + ident.span(), + "`#[oparg(catch_all)]` variant must have exactly one unnamed field, e.g., `Other(u32)`", + )); + } if catch_all && display.is_some() { return Err(Error::new( @@ -183,7 +192,7 @@ pub(super) fn handle_enum(item: ItemEnum) -> syn::Result