Skip to content

Reduce the amount of code we generate from macros #6187

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

Open
alex wants to merge 6 commits into
base: main
Choose a base branch
from
+486 −327
Open

Reduce the amount of code we generate from macros #6187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f2a7dba
Reduce macro-generated code size
claude Jul 4, 2026
6d95dd5
Fix UI test failures from slimmed codegen
alex Jul 5, 2026
9f1b471
Remove dead map_into_ptr/map_into_pyobject converters
alex Jul 5, 2026
0968e9c
Restore compile-time class doc concatenation
alex Jul 5, 2026
d0e7920
Make fused required-argument extraction safe
alex Jul 5, 2026
3b2f821
Re-bless UI tests after rebase onto main
alex Jul 5, 2026
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
92 changes: 42 additions & 50 deletions pyo3-macros-backend/src/method.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,9 +435,12 @@ impl SelfType {
//
// The trailing `?` exists because if the extraction fails here it represents
// a genuine type error, should not fall back to e.g. `ExtractErrorMode::NotImplemented`.
//
// The cast is inlined (without a redundant nested `unsafe` block, unlike
// the `Checked` path below) to keep the generated code small.
quote! {
unsafe { #pyo3_path::impl_::extract_argument::#method::<#cls>(
#arg,
#pyo3_path::impl_::extract_argument::#cast_fn(#py, #slf),
&mut #holder,
) }?
}
Expand All @@ -462,9 +465,9 @@ impl SelfType {
}
SelfType::TryFromBoundRef { span, non_null } => {
let bound_ref = if *non_null {
quote! { unsafe { #pyo3_path::Bound::ref_from_non_null(#py, &#slf) } }
quote! { #pyo3_path::Bound::ref_from_non_null(#py, &#slf) }
} else {
quote! { unsafe { #pyo3_path::Bound::ref_from_ptr(#py, &#slf) } }
quote! { #pyo3_path::Bound::ref_from_ptr(#py, &#slf) }
};
let pyo3_path = pyo3_path.to_tokens_spanned(*span);
let receiver = match self_conversion.0 {
Expand All @@ -474,39 +477,27 @@ impl SelfType {
// an instance of the correct type (or a compatible subtype) before
// invoking the slot.
//
// The wrapping `Ok(...?)` here is because an error here should not
// be treated by e.g. `ExtractErrorMode::NotImplemented` as falling
// back to the default, but instead a genuine type error.
quote! {
unsafe {
#pyo3_path::PyResult::Ok(
#pyo3_path::impl_::extract_argument::cast_bound_ref_trusted::<#cls>(#bound_ref)?
)
}
}
// Note: cast failures inside the fused helper can only occur on
// PyPy (CPython's slot dispatch contract guarantees the receiver
// type); on PyPy under `ExtractErrorMode::NotImplemented` they now
// fall back to the default like `TryFrom` failures always have,
// instead of raising directly.
Comment thread
alex marked this conversation as resolved.
// The `unsafe` token is deliberately spanned at the macro call site
// (plain `quote!`) so that `#![forbid(unsafe_code)]` in user code
// doesn't reject the generated unsafe block.
let call = quote_spanned! { *span =>
#pyo3_path::impl_::extract_argument::extract_receiver_trusted::<#cls, _>(#bound_ref)
};
quote! { unsafe { #call } }
}
SelfConversionPolicyInner::Checked => {
quote_spanned! { *span =>
#bound_ref.cast::<#cls>()
.map_err(::std::convert::Into::<#pyo3_path::PyErr>::into)
}
let call = quote_spanned! { *span =>
#pyo3_path::impl_::extract_argument::extract_receiver::<#cls, _>(#bound_ref)
};
quote! { unsafe { #call } }
}
};
error_mode.handle_error(
quote_spanned! { *span =>
#receiver
.and_then(
#[allow(
clippy::unnecessary_fallible_conversions,
clippy::useless_conversion,
reason = "anything implementing `TryFrom<&Bound>` is permitted"
)]
|bound| ::std::convert::TryFrom::try_from(bound).map_err(::std::convert::Into::into)
)

},
ctx
)
error_mode.handle_error(receiver, ctx)
}
}
}
Expand Down Expand Up @@ -900,7 +891,11 @@ impl<'a> FnSpec<'a> {
(None, None)
};
let args = self_arg.into_iter().chain(args);
let ok_wrap = quotes::ok_wrap(ret_ident.to_token_stream(), ctx);
let return_conversion = quotes::wrap_into_pyobject(
ret_ident.to_token_stream(),
quote!(assume_attached.py()),
ctx,
);
quote! {
{
let coroutine = {
Expand All @@ -926,8 +921,7 @@ impl<'a> FnSpec<'a> {
function(#(#args),*)
};
let #ret_ident = future.await;
let #ret_ident = #ok_wrap;
#pyo3_path::impl_::wrap::converter(&#ret_ident).map_into_pyobject(assume_attached.py(), #ret_ident)
#return_conversion
},
)
};
Expand All @@ -936,10 +930,7 @@ impl<'a> FnSpec<'a> {
}
} else {
let args = self_arg.into_iter().chain(args);
let return_conversion = quotes::map_result_into_ptr(
quotes::ok_wrap(ret_ident.to_token_stream(), ctx),
ctx,
);
let return_conversion = quotes::wrap_into_ptr(ret_ident.to_token_stream(), ctx);
quote! {
{
#init_holders
Expand Down Expand Up @@ -986,8 +977,7 @@ impl<'a> FnSpec<'a> {
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
let function = #rust_name; // Shadow the function name to avoid #3017
#warnings
let result = #call;
result
#call
}
}
}
Expand All @@ -1001,8 +991,7 @@ impl<'a> FnSpec<'a> {
let function = #rust_name; // Shadow the function name to avoid #3017
#arg_convert
#warnings
let result = #call;
result
#call
}
);
}
Expand All @@ -1021,8 +1010,7 @@ impl<'a> FnSpec<'a> {
let function = #rust_name; // Shadow the function name to avoid #3017
#arg_convert
#warnings
let result = #call;
result
#call
}
}
}
Expand All @@ -1045,20 +1033,24 @@ impl<'a> FnSpec<'a> {
FnType::FnStatic => quote! { .flags(#pyo3_path::ffi::METH_STATIC) },
_ => quote! {},
};
let trampoline = match convention {
CallingConvention::Noargs => Ident::new("noargs", Span::call_site()),
// Constructor names on `PyMethodDef` and (shorter) trampoline aliases in
// `impl_::trampoline` - the short aliases keep the generated code small.
let (constructor, trampoline) = match convention {
CallingConvention::Noargs => ("noargs", "noargs"),
CallingConvention::Fastcall => {
Ident::new("maybe_fastcall_cfunction_with_keywords", Span::call_site())
("maybe_fastcall_cfunction_with_keywords", "fastcall_kw")
}
CallingConvention::Varargs => Ident::new("cfunction_with_keywords", Span::call_site()),
CallingConvention::Varargs => ("cfunction_with_keywords", "cfunc_kw"),
};
let constructor = Ident::new(constructor, Span::call_site());
let trampoline = Ident::new(trampoline, Span::call_site());
let doc = if let Some(doc) = doc {
doc.to_cstr_stream(ctx)?
} else {
c"".to_token_stream()
};
Ok(quote! {
#pyo3_path::impl_::pymethods::PyMethodDef::#trampoline(
#pyo3_path::impl_::pymethods::PyMethodDef::#constructor(
#python_name,
#pyo3_path::impl_::trampoline::get_trampoline_function!(#trampoline, #wrapper),
#doc,
Expand Down
54 changes: 28 additions & 26 deletions pyo3-macros-backend/src/params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,18 @@ impl Holders {
pub fn init_holders(&self, ctx: &Ctx) -> TokenStream {
let Ctx { pyo3_path, .. } = ctx;
let holders = &self.holders;
if holders.is_empty() {
return TokenStream::new();
}
// The holders are declared with a single tuple pattern to keep the generated code
// small (this also avoids triggering `clippy::let_unit_value` for the holders which
// are just `()`).
let holders_init = holders
.iter()
.map(|_| quote!(#pyo3_path::impl_::extract_argument::FunctionArgumentHolder::INIT))
.collect::<Vec<_>>();
quote! {
#[allow(clippy::let_unit_value, reason = "many holders are just `()`")]
#(let mut #holders = #pyo3_path::impl_::extract_argument::FunctionArgumentHolder::INIT;)*
let (#(mut #holders,)*) = (#(#holders_init,)*);
}
}
}
Expand Down Expand Up @@ -102,10 +111,7 @@ pub fn impl_arg_params(
.map(|(name, default_value)| {
let required = default_value.is_none();
quote! {
#pyo3_path::impl_::extract_argument::KeywordOnlyParameterDescription {
name: #name,
required: #required,
}
#pyo3_path::impl_::extract_argument::KeywordOnlyParameterDescription::new(#name, #required)
}
});

Expand Down Expand Up @@ -165,14 +171,15 @@ pub fn impl_arg_params(
// create array of arguments, and then parse
(
quote! {
const DESCRIPTION: #pyo3_path::impl_::extract_argument::FunctionDescription = #pyo3_path::impl_::extract_argument::FunctionDescription {
cls_name: #cls_name,
func_name: stringify!(#python_name),
positional_parameter_names: &[#(#positional_parameter_names),*],
positional_only_parameters: #positional_only_parameters,
required_positional_parameters: #required_positional_parameters,
keyword_only_parameters: &[#(#keyword_only_parameters),*],
};
const DESCRIPTION: #pyo3_path::impl_::extract_argument::FunctionDescription =
#pyo3_path::impl_::extract_argument::FunctionDescription::new(
#cls_name,
stringify!(#python_name),
&[#(#positional_parameter_names),*],
#positional_only_parameters,
#required_positional_parameters,
&[#(#keyword_only_parameters),*],
);
let mut #args_array = [::std::option::Option::None; #num_params];
let (_args, _kwargs) = #extract_expression;
#from_py_with
Expand Down Expand Up @@ -247,13 +254,10 @@ pub(crate) fn impl_regular_arg_param(
let pyo3_path = pyo3_path.to_tokens_spanned(arg.ty.span());

// Use this macro inside this function, to ensure that all code generated here is associated
// with the function argument
let use_probe = quote! {
#[allow(unused_imports, reason = "`Probe` trait used on negative case only")]
use #pyo3_path::impl_::pyclass::Probe as _;
};
// with the function argument. The `Probe` trait used by some of the extraction machinery is
// imported once per wrapper function (see `Holders::init_holders`).
macro_rules! quote_arg_span {
($($tokens:tt)*) => { quote_spanned!(arg.ty.span() => { #use_probe $($tokens)* }) }
($($tokens:tt)*) => { quote_spanned!(arg.ty.span() => $($tokens)*) }
}

let name_str = arg.name.to_string();
Expand Down Expand Up @@ -282,10 +286,9 @@ pub(crate) fn impl_regular_arg_param(
)?
}
} else {
let unwrap = quote! {unsafe { #pyo3_path::impl_::extract_argument::unwrap_required_argument_bound(#arg_value.as_deref()) }};
quote_arg_span! {
#pyo3_path::impl_::extract_argument::from_py_with(
#unwrap,
#pyo3_path::impl_::extract_argument::from_py_with_required(
#arg_value.as_deref(),
#name_str,
#extractor,
)?
Expand All @@ -306,10 +309,9 @@ pub(crate) fn impl_regular_arg_param(
}
} else {
let holder = holders.push_holder(arg.ty.span());
let unwrap = quote! { unsafe { #pyo3_path::impl_::extract_argument::unwrap_required_argument(#arg_value) } };
quote_arg_span! {
#pyo3_path::impl_::extract_argument::extract_argument(
#unwrap,
#pyo3_path::impl_::extract_argument::extract_required_argument(
#arg_value,
&mut #holder,
#name_str
)?
Expand Down
Loading
Loading