Skip to content

Fix #[pyclass(base=...)] notation #6242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 1 commit into from
Nov 10, 2025
Merged

Fix #[pyclass(base=...)] notation #6242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions derive-impl/src/pyclass.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ fn generate_class_def(
ident: &Ident,
name: &str,
module_name: Option<&str>,
base: Option<String>,
base: Option<syn::Path>,
metaclass: Option<String>,
unhashable: bool,
attrs: &[Attribute],
Expand Down Expand Up @@ -358,7 +358,6 @@ fn generate_class_def(
Some(quote! { rustpython_vm::builtins::PyTuple })
} else {
base.as_ref().map(|typ| {
let typ = Ident::new(typ, ident.span());
quote_spanned! { ident.span() => #typ }
})
}
Expand All @@ -382,7 +381,6 @@ fn generate_class_def(
});

let base_or_object = if let Some(base) = base {
let base = Ident::new(&base, ident.span());
quote! { #base }
} else {
quote! { ::rustpython_vm::builtins::PyBaseObject }
Expand Down
33 changes: 31 additions & 2 deletions derive-impl/src/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,35 @@ impl ItemMetaInner {
Ok(value)
}

pub fn _optional_path(&self, key: &str) -> Result<Option<syn::Path>> {
let value = if let Some((_, meta)) = self.meta_map.get(key) {
let Meta::NameValue(syn::MetaNameValue { value, .. }) = meta else {
bail_span!(
meta,
"#[{}({} = ...)] must be a name-value pair",
self.meta_name(),
key
)
};

// Try to parse as a Path (identifier or path like Foo or foo::Bar)
match syn::parse2::<syn::Path>(value.to_token_stream()) {
Ok(path) => Some(path),
Err(_) => {
bail_span!(
value,
"#[{}({} = ...)] must be a valid type path (e.g., PyBaseException)",
self.meta_name(),
key
)
}
}
} else {
None
};
Ok(value)
}

pub fn _has_key(&self, key: &str) -> Result<bool> {
Ok(matches!(self.meta_map.get(key), Some((_, _))))
}
Expand Down Expand Up @@ -384,8 +413,8 @@ impl ClassItemMeta {
self.inner()._optional_str("ctx")
}

pub fn base(&self) -> Result<Option<String>> {
self.inner()._optional_str("base")
pub fn base(&self) -> Result<Option<syn::Path>> {
self.inner()._optional_path("base")
}
Comment on lines +416 to 418
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify all call sites handle the new Path return type.

The change from String to syn::Path aligns with the PR objective. Ensure all consumers of base() have been updated to work with syn::Path instead of String.

🏁 Script executed:

#!/bin/bash
# Description: Find all usages of ClassItemMeta::base() and ExceptionItemMeta::base() to verify they handle syn::Path

# Search for calls to .base() method that might need updates
rg -nP --type=rust -C5 '\.base\s*\(\s*\)'

# Also search for ExceptionItemMeta usage since it derefs to ClassItemMeta
rg -nP --type=rust -C3 'ExceptionItemMeta|ClassItemMeta'

Length of output: 7276

🏁 Script executed:

#!/bin/bash
# Description: Get detailed context around base() usage to see how results are handled

# Show full context for line 426 usage in pyclass.rs
echo "=== Usage 1: Around line 426 in pyclass.rs ==="
sed -n '420,445p' derive-impl/src/pyclass.rs

echo ""
echo "=== Usage 2: Around line 539 in pyclass.rs ==="
sed -n '533,570p' derive-impl/src/pyclass.rs

Length of output: 2604

🏁 Script executed:

#!/bin/bash
# Description: Find generate_class_def signature and verify type expectations

# Find generate_class_def definition
echo "=== Searching for generate_class_def definition ==="
rg -nP --type=rust -B3 'fn generate_class_def' derive-impl/src/

echo ""
echo "=== Check the full signature of generate_class_def ==="
rg -nP --type=rust -A15 'fn generate_class_def'

Length of output: 1251

🏁 Script executed:

#!/bin/bash
# Description: Verify how base_class_name is used in the quote output and check for potential issues

echo "=== Full context of base_class_name usage in quote macro ==="
sed -n '539,600p' derive-impl/src/pyclass.rs

echo ""
echo "=== Search for how this generated code might be processed ==="
rg -nP --type=rust -C3 'base_class_name'

Length of output: 3041

🏁 Script executed:

#!/bin/bash
# Description: Verify how pyclass attribute expects base parameter and check for compilation

echo "=== Check if there are test cases for pyexception with base ==="
fd -e rs test -o example | xargs rg -l 'pyexception.*base|base.*pyexception' 2>/dev/null || echo "No tests found"

echo ""
echo "=== Search for how pyclass macro processes the base attribute ==="
rg -nP --type=rust 'base.*=' derive-impl/src/util.rs | head -20

Length of output: 303

🏁 Script executed:

#!/bin/bash
# Description: Search for similar base usage patterns and quote macro handling

echo "=== Search for all quote! blocks using base parameter ==="
rg -nP --type=rust -B5 -A5 'quote!.*base|base.*quote!' derive-impl/src/

echo ""
echo "=== Check util.rs for base handling patterns ==="
sed -n '410,425p' derive-impl/src/util.rs

Length of output: 1079

Fix pyexception code path to properly handle Option<syn::Path> return type from base().

The change from String to syn::Path at line 426 is correctly handled by generate_class_def(), which expects Option<syn::Path>. However, line 570 has a critical issue: it directly interpolates the Option type into the quote macro, which will generate invalid proc macro attribute syntax like base = Some(path) or base = None.

Line 570 must follow the pattern used at lines 383-387: match on the Option<syn::Path>, unwrap when present, and provide an appropriate default when absent. Either unwrap the option explicitly before quoting (if base is required for exceptions), or conditionally quote the attribute value.

🤖 Prompt for AI Agents 
In derive-impl/src/util.rs around lines 416-418 (and the pyexception emit site
near line 570), the pyexception attribute code currently interpolates an
Option<syn::Path> directly into quote causing generated attributes like `base =
Some(path)` or `base = None`; change this to match the pattern at lines
~383-387: match on the Option, map Some(path) to a token/value you will inject
(e.g. build a `base = path` TokenStream or the path token itself) and map None
to either omit the base attribute or emit an appropriate default token stream,
then use that prepared token/value in the quote invocation so the generated
attribute contains either the unwrapped path or nothing/default rather than an
Option.


pub fn unhashable(&self) -> Result<bool> {
Expand Down
2 changes: 1 addition & 1 deletion derive/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub fn derive_from_args(input: TokenStream) -> TokenStream {
/// - `IMMUTABLETYPE`: class attributes are immutable.
/// - `with`: which trait implementations are to be included in the python class.
/// ```rust, ignore
/// #[pyclass(module = "my_module", name = "MyClass", base = "BaseClass")]
/// #[pyclass(module = "my_module", name = "MyClass", base = BaseClass)]
/// struct MyStruct {
/// x: i32,
/// }
Expand Down
14 changes: 7 additions & 7 deletions stdlib/src/ssl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ mod _ssl {

/// An error occurred in the SSL implementation.
#[pyattr]
#[pyexception(name = "SSLError", base = "PyOSError")]
#[pyexception(name = "SSLError", base = PyOSError)]
#[derive(Debug)]
pub struct PySslError {}

Expand All @@ -269,7 +269,7 @@ mod _ssl {

/// A certificate could not be verified.
#[pyattr]
#[pyexception(name = "SSLCertVerificationError", base = "PySslError")]
#[pyexception(name = "SSLCertVerificationError", base = PySslError)]
#[derive(Debug)]
pub struct PySslCertVerificationError {}

Expand All @@ -278,7 +278,7 @@ mod _ssl {

/// SSL/TLS session closed cleanly.
#[pyattr]
#[pyexception(name = "SSLZeroReturnError", base = "PySslError")]
#[pyexception(name = "SSLZeroReturnError", base = PySslError)]
#[derive(Debug)]
pub struct PySslZeroReturnError {}

Expand All @@ -287,7 +287,7 @@ mod _ssl {

/// Non-blocking SSL socket needs to read more data.
#[pyattr]
#[pyexception(name = "SSLWantReadError", base = "PySslError")]
#[pyexception(name = "SSLWantReadError", base = PySslError)]
#[derive(Debug)]
pub struct PySslWantReadError {}

Expand All @@ -296,7 +296,7 @@ mod _ssl {

/// Non-blocking SSL socket needs to write more data.
#[pyattr]
#[pyexception(name = "SSLWantWriteError", base = "PySslError")]
#[pyexception(name = "SSLWantWriteError", base = PySslError)]
#[derive(Debug)]
pub struct PySslWantWriteError {}

Expand All @@ -305,7 +305,7 @@ mod _ssl {

/// System error when attempting SSL operation.
#[pyattr]
#[pyexception(name = "SSLSyscallError", base = "PySslError")]
#[pyexception(name = "SSLSyscallError", base = PySslError)]
#[derive(Debug)]
pub struct PySslSyscallError {}

Expand All @@ -314,7 +314,7 @@ mod _ssl {

/// SSL/TLS connection terminated abruptly.
#[pyattr]
#[pyexception(name = "SSLEOFError", base = "PySslError")]
#[pyexception(name = "SSLEOFError", base = PySslError)]
#[derive(Debug)]
pub struct PySslEOFError {}

Expand Down
2 changes: 1 addition & 1 deletion vm/src/builtins/bool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl PyObjectRef {
}
}

#[pyclass(name = "bool", module = false, base = "PyInt")]
#[pyclass(name = "bool", module = false, base = PyInt)]
pub struct PyBool;

impl PyPayload for PyBool {
Expand Down
2 changes: 1 addition & 1 deletion vm/src/builtins/builtin_func.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ impl Representable for PyNativeFunction {
impl Unconstructible for PyNativeFunction {}

// `PyCMethodObject` in CPython
#[pyclass(name = "builtin_method", module = false, base = "PyNativeFunction")]
#[pyclass(name = "builtin_method", module = false, base = PyNativeFunction)]
pub struct PyNativeMethod {
pub(crate) func: PyNativeFunction,
pub(crate) class: &'static Py<PyType>, // TODO: the actual life is &'self
Expand Down
Loading
Loading