Skip to content

rework extract_argument code to populate holders as intermediate values #5742

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

Draft
davidhewitt wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

rework extract_argument code to populate holders as intermediate values #5742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2c14bfb
rework `extract_argument` code to populate holders as intermediate va…
davidhewitt Jan 19, 2026
a9064a9
clean up error messages
davidhewitt Jan 19, 2026
5edda8e
cleanup errors further
davidhewitt Jan 19, 2026
7534ecb
fix error messages
davidhewitt Jan 19, 2026
9e184e1
fix duplicate error message
davidhewitt Jan 20, 2026
b6edb22
fix more duplicate errors
davidhewitt Jan 21, 2026
35b2838
use call-site hygiene for the extraction span
davidhewitt Jan 21, 2026
3f54f55
cleanup
davidhewitt Jan 21, 2026
efb9bb9
return `extract_argument` functions to original location
davidhewitt Jan 21, 2026
eb0e416
cleanup CI
davidhewitt Jan 21, 2026
07e9db8
remove `Error` type from `PyFunctionArgument`
davidhewitt Jan 21, 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
80 changes: 49 additions & 31 deletions pyo3-macros-backend/src/method.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ use quote::{quote, quote_spanned, ToTokens};
use syn::LitCStr;
use syn::{ext::IdentExt, spanned::Spanned, Ident, Result};

use crate::params::Param;
use crate::pyfunction::{PyFunctionWarning, WarningFactory};
use crate::pymethod::{type_inferred_fncall, FnCall};
use crate::pyversions::is_abi3_before;
use crate::utils::Ctx;
use crate::utils::{locate_tokens_at, Ctx};
use crate::{
attributes::{FromPyWithAttribute, TextSignatureAttribute, TextSignatureAttributeValue},
params::{impl_arg_params, Holders},
Expand Down Expand Up @@ -264,7 +266,7 @@ impl FnType {
error_mode: ExtractErrorMode,
holders: &mut Holders,
ctx: &Ctx,
) -> Option<TokenStream> {
) -> Option<Param> {
let Ctx { pyo3_path, .. } = ctx;
match self {
FnType::Getter(st) | FnType::Setter(st) | FnType::Deleter(st) | FnType::Fn(st) => {
Expand All @@ -286,7 +288,7 @@ impl FnType {
.cast_unchecked::<#pyo3_path::types::PyType>()
)
};
Some(quote! { unsafe { #ret } })
Some(Param::arg_expr(quote! { unsafe { #ret } }))
}
FnType::FnModule(span) => {
let py = syn::Ident::new("py", Span::call_site());
Expand All @@ -299,7 +301,7 @@ impl FnType {
.cast_unchecked::<#pyo3_path::types::PyModule>()
)
};
Some(quote! { unsafe { #ret } })
Some(Param::arg_expr(quote! { unsafe { #ret } }))
}
FnType::FnStatic | FnType::ClassAttribute => None,
}
Expand Down Expand Up @@ -340,36 +342,51 @@ impl SelfType {
error_mode: ExtractErrorMode,
holders: &mut Holders,
ctx: &Ctx,
) -> TokenStream {
) -> Param {
// Due to use of quote_spanned in this function, need to bind these idents to the
// main macro callsite.
let py = syn::Ident::new("py", Span::call_site());
let slf = syn::Ident::new("_slf", Span::call_site());
let Ctx { pyo3_path, .. } = ctx;
match self {
SelfType::Receiver { span, mutable } => {
// NB it's possible to use type inference with "extractor" here, but then if the class is frozen
// the &mut self receivers get horrible error messages. This approach with concrete calls produces
// better error messages.
let holder = holders.push_holder(*span);
// when interpolating cls into expressions below, need to locate the span at the receiver span to
// avoid duplicate error messages
let cls = locate_tokens_at(cls.to_token_stream(), *span);
// unsafe cast done at call_site span to avoid user code being flagged as using unsafe
let arg = quote! { unsafe { #pyo3_path::impl_::extract_argument::cast_function_argument(#py, #slf) } };
let method = if *mutable {
syn::Ident::new("extract_pyclass_ref_mut", *span)
let (extract, unpack, mut_kw) = if *mutable {
(
Ident::new("extract_pyclass_guard_mut", *span),
Ident::new("pyclass_guard_to_ref_mut", *span),
Some(syn::Token![mut](*span)),
)
} else {
syn::Ident::new("extract_pyclass_ref", *span)
(
Ident::new("extract_pyclass_guard", *span),
Ident::new("pyclass_guard_to_ref", *span),
None,
)
};
let holder = holders.push_holder(*span);
let pyo3_path = pyo3_path.to_tokens_spanned(*span);
error_mode.handle_error(
quote_spanned! { *span =>
#pyo3_path::impl_::extract_argument::#method::<#cls>(
#arg,
&mut #holder,
)
},
let extract = error_mode.handle_error(
quote_spanned! { *span => #pyo3_path::impl_::extract_argument::#extract::<#cls>(#arg) },
ctx,
)
);
Param {
resolve: quote_spanned! { *span => ::std::unreachable!() },
extract: Some(quote_spanned! { *span => #holder = #extract; }),
arg: quote_spanned! { *span => #pyo3_path::impl_::extract_argument::#unpack(& #mut_kw #holder) },
}
}
SelfType::TryFromBoundRef(span) => {
let bound_ref = quote! { unsafe { #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(#py, &#slf) } };
let pyo3_path = pyo3_path.to_tokens_spanned(*span);
error_mode.handle_error(
Param::arg_expr(error_mode.handle_error(
quote_spanned! { *span =>
#bound_ref.cast::<#cls>()
.map_err(::std::convert::Into::<#pyo3_path::PyErr>::into)
Expand All @@ -380,7 +397,7 @@ impl SelfType {

},
ctx
)
))
}
}
}
Expand Down Expand Up @@ -664,7 +681,7 @@ impl<'a> FnSpec<'a> {
}
}

let rust_call = |args: Vec<TokenStream>, mut holders: Holders| {
let rust_call = |args: Vec<Param>, mut holders: Holders| {
let self_arg = self
.tp
.self_arg(cls, ExtractErrorMode::Raise, &mut holders, ctx);
Expand All @@ -674,6 +691,10 @@ impl<'a> FnSpec<'a> {
// but *not* of the call itself otherwise the spans get really weird
let ret_ident = Ident::new("ret", *output_span);

let FnCall { setup, call } =
type_inferred_fncall(quote!(function), self_arg.iter().chain(&args), ctx);
let ok_wrap = quotes::ok_wrap(ret_ident.to_token_stream(), ctx);

if self.asyncness.is_some() {
// For async functions, we need to build up a coroutine object to return from the initial function call.
//
Expand Down Expand Up @@ -731,8 +752,7 @@ impl<'a> FnSpec<'a> {
} else {
(None, None)
};
let args = self_arg.into_iter().chain(args);
let ok_wrap = quotes::ok_wrap(ret_ident.to_token_stream(), ctx);

quote! {
{
let coroutine = {
Expand All @@ -751,7 +771,8 @@ impl<'a> FnSpec<'a> {
let py = assume_attached.py();
#slf_ptr
#output_args
function(#(#args),*)
#setup
#call
};
let #ret_ident = future.await;
let #ret_ident = #ok_wrap;
Expand All @@ -763,15 +784,12 @@ 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::map_result_into_ptr(ok_wrap, ctx);
quote! {
{
#init_holders
let #ret_ident = function(#(#args),*);
#setup
let #ret_ident = #call;
#return_conversion
}
}
Expand All @@ -795,8 +813,8 @@ impl<'a> FnSpec<'a> {
.arguments
.iter()
.map(|arg| match arg {
FnArg::Py(..) => quote!(py),
FnArg::CancelHandle(..) => quote!(__cancel_handle),
FnArg::Py(..) => Param::arg_expr(quote!(py)),
FnArg::CancelHandle(..) => Param::arg_expr(quote!(__cancel_handle)),
_ => unreachable!("`CallingConvention::Noargs` should not contain any arguments (reaching Python) except for `self`, which is handled below."),
})
.collect();
Expand Down
Loading
Loading