From a8d26490327ca90f16dad3ef5ed637ae9ff191e0 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Sat, 5 Jun 2021 17:04:56 +0200 Subject: [PATCH 1/3] Refactor wrapper generation, and enable fastcall for static/class methods. --- pyo3-macros-backend/src/lib.rs | 1 + pyo3-macros-backend/src/method.rs | 224 ++++++++-- pyo3-macros-backend/src/params.rs | 257 ++++++++++++ pyo3-macros-backend/src/pyfunction.rs | 117 +----- pyo3-macros-backend/src/pymethod.rs | 568 +++----------------------- pyo3-macros-backend/src/pyproto.rs | 4 +- 6 files changed, 533 insertions(+), 638 deletions(-) create mode 100644 pyo3-macros-backend/src/params.rs diff --git a/pyo3-macros-backend/src/lib.rs b/pyo3-macros-backend/src/lib.rs index b8ae6866fe7..e8645659340 100644 --- a/pyo3-macros-backend/src/lib.rs +++ b/pyo3-macros-backend/src/lib.rs @@ -15,6 +15,7 @@ mod from_pyobject; mod konst; mod method; mod module; +mod params; mod proto_method; mod pyclass; mod pyfunction; diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 0e9c34bef9e..4d78ace7c5c 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -1,15 +1,17 @@ // Copyright (c) 2017-present PyO3 Project and Contributors use crate::attributes::TextSignatureAttribute; +use crate::params::impl_arg_params; use crate::pyfunction::PyFunctionOptions; use crate::pyfunction::{PyFunctionArgPyO3Attributes, PyFunctionSignature}; use crate::utils; use crate::{deprecations::Deprecations, pyfunction::Argument}; -use proc_macro2::TokenStream; +use proc_macro2::{Span, TokenStream}; use quote::ToTokens; use quote::{quote, quote_spanned}; use syn::ext::IdentExt; use syn::spanned::Spanned; +use syn::Result; #[derive(Clone, PartialEq, Debug)] pub struct FnArg<'a> { @@ -24,7 +26,7 @@ pub struct FnArg<'a> { impl<'a> FnArg<'a> { /// Transforms a rust fn arg parsed with syn into a method::FnArg - pub fn parse(arg: &'a mut syn::FnArg) -> syn::Result { + pub fn parse(arg: &'a mut syn::FnArg) -> Result { match arg { syn::FnArg::Receiver(recv) => { bail_spanned!(recv.span() => "unexpected receiver") @@ -86,13 +88,44 @@ pub enum FnType { FnNew, FnClass, FnStatic, + FnModule, ClassAttribute, } +impl FnType { + pub fn self_conversion(&self, cls: Option<&syn::Type>) -> TokenStream { + match self { + FnType::Getter(st) | FnType::Setter(st) | FnType::Fn(st) | FnType::FnCall(st) => { + st.receiver(cls.expect("no class given for Fn with a \"self\" receiver")) + } + FnType::FnNew | FnType::FnStatic | FnType::ClassAttribute => { + quote!() + } + FnType::FnClass => { + quote! { + let _slf = pyo3::types::PyType::from_type_ptr(_py, _slf as *mut pyo3::ffi::PyTypeObject); + } + } + FnType::FnModule => { + quote! { + let _slf = _py.from_borrowed_ptr::(_slf); + } + } + } + } + + pub fn self_arg(&self) -> TokenStream { + match self { + FnType::FnNew | FnType::FnStatic | FnType::ClassAttribute => quote!(), + _ => quote!(_slf,), + } + } +} + #[derive(Clone, Debug)] pub enum SelfType { Receiver { mutable: bool }, - TryFromPyCell(proc_macro2::Span), + TryFromPyCell(Span), } impl SelfType { @@ -123,6 +156,31 @@ impl SelfType { } } +/// Determines which CPython calling convention a given FnSpec uses. +#[derive(Clone, Debug)] +pub enum CallingConvention { + Noargs, // METH_NOARGS + Varargs, // METH_VARARGS | METH_KEYWORDS + Fastcall, // METH_FASTCALL | METH_KEYWORDS (Py3.7+ and !abi3) + TpNew, // special convention for tp_new +} + +impl CallingConvention { + /// Determine default calling convention from an argument signature. + /// + /// Different other slots (tp_call, tp_new) can have other requirements + /// and are set manually (see `parse_fn_type` below). + pub fn from_args(args: &[FnArg<'_>]) -> Self { + if args.is_empty() { + Self::Noargs + } else if cfg!(all(Py_3_7, not(Py_LIMITED_API))) { + Self::Fastcall + } else { + Self::Varargs + } + } +} + pub struct FnSpec<'a> { pub tp: FnType, // Rust function name @@ -135,6 +193,7 @@ pub struct FnSpec<'a> { pub output: syn::Type, pub doc: syn::LitStr, pub deprecations: Deprecations, + pub convention: CallingConvention, } pub fn get_return_info(output: &syn::ReturnType) -> syn::Type { @@ -144,7 +203,7 @@ pub fn get_return_info(output: &syn::ReturnType) -> syn::Type { } } -pub fn parse_method_receiver(arg: &syn::FnArg) -> syn::Result { +pub fn parse_method_receiver(arg: &syn::FnArg) -> Result { match arg { syn::FnArg::Receiver(recv) => Ok(SelfType::Receiver { mutable: recv.mutability.is_some(), @@ -174,19 +233,12 @@ impl<'a> FnSpec<'a> { (accept_args, accept_kwargs) } - /// Return true if the function can use METH_FASTCALL. - /// - /// This is true on Py3.7+, except with the stable ABI (abi3). - pub fn can_use_fastcall(&self) -> bool { - cfg!(all(Py_3_7, not(Py_LIMITED_API))) - } - /// Parser function signature and function attributes pub fn parse( sig: &'a mut syn::Signature, meth_attrs: &mut Vec, options: PyFunctionOptions, - ) -> syn::Result> { + ) -> Result> { let MethodAttributes { ty: fn_type_attr, args: fn_attrs, @@ -198,18 +250,19 @@ impl<'a> FnSpec<'a> { if let Some(name) = &python_name { bail_spanned!(name.span() => "`name` not allowed with `#[new]`"); } - python_name = Some(syn::Ident::new("__new__", proc_macro2::Span::call_site())) + python_name = Some(syn::Ident::new("__new__", Span::call_site())) } Some(MethodTypeAttribute::Call) => { if let Some(name) = &python_name { bail_spanned!(name.span() => "`name` not allowed with `#[call]`"); } - python_name = Some(syn::Ident::new("__call__", proc_macro2::Span::call_site())) + python_name = Some(syn::Ident::new("__call__", Span::call_site())) } _ => {} } - let (fn_type, skip_first_arg) = Self::parse_fn_type(sig, fn_type_attr, &mut python_name)?; + let (fn_type, skip_first_arg, fixed_convention) = + Self::parse_fn_type(sig, fn_type_attr, &mut python_name)?; Self::ensure_text_signature_on_valid_method(&fn_type, options.text_signature.as_ref())?; let name = &sig.ident; @@ -224,22 +277,26 @@ impl<'a> FnSpec<'a> { .map(|attr| (&python_name, attr)), )?; - let arguments = if skip_first_arg { + let arguments: Vec<_> = if skip_first_arg { sig.inputs .iter_mut() .skip(1) .map(FnArg::parse) - .collect::>()? + .collect::>()? } else { sig.inputs .iter_mut() .map(FnArg::parse) - .collect::>()? + .collect::>()? }; + let convention = + fixed_convention.unwrap_or_else(|| CallingConvention::from_args(&arguments)); + Ok(FnSpec { tp: fn_type, name, + convention, python_name, attrs: fn_attrs, args: arguments, @@ -280,7 +337,7 @@ impl<'a> FnSpec<'a> { sig: &syn::Signature, fn_type_attr: Option, python_name: &mut Option, - ) -> syn::Result<(FnType, bool)> { + ) -> Result<(FnType, bool, Option)> { let name = &sig.ident; let parse_receiver = |msg: &'static str| { let first_arg = sig @@ -301,20 +358,23 @@ impl<'a> FnSpec<'a> { } }; - let (fn_type, skip_first_arg) = match fn_type_attr { - Some(MethodTypeAttribute::StaticMethod) => (FnType::FnStatic, false), + let (fn_type, skip_first_arg, fixed_convention) = match fn_type_attr { + Some(MethodTypeAttribute::StaticMethod) => (FnType::FnStatic, false, None), Some(MethodTypeAttribute::ClassAttribute) => { ensure_spanned!( sig.inputs.is_empty(), sig.inputs.span() => "class attribute methods cannot take arguments" ); - (FnType::ClassAttribute, false) + (FnType::ClassAttribute, false, None) } - Some(MethodTypeAttribute::New) => (FnType::FnNew, false), - Some(MethodTypeAttribute::ClassMethod) => (FnType::FnClass, true), + Some(MethodTypeAttribute::New) => { + (FnType::FnNew, false, Some(CallingConvention::TpNew)) + } + Some(MethodTypeAttribute::ClassMethod) => (FnType::FnClass, true, None), Some(MethodTypeAttribute::Call) => ( FnType::FnCall(parse_receiver("expected receiver for #[call]")?), true, + Some(CallingConvention::Varargs), ), Some(MethodTypeAttribute::Getter) => { // Strip off "get_" prefix if needed @@ -325,6 +385,7 @@ impl<'a> FnSpec<'a> { ( FnType::Getter(parse_receiver("expected receiver for #[getter]")?), true, + None, ) } Some(MethodTypeAttribute::Setter) => { @@ -336,6 +397,7 @@ impl<'a> FnSpec<'a> { ( FnType::Setter(parse_receiver("expected receiver for #[setter]")?), true, + None, ) } None => ( @@ -343,9 +405,10 @@ impl<'a> FnSpec<'a> { "static method needs #[staticmethod] attribute", )?), true, + None, ), }; - Ok((fn_type, skip_first_arg)) + Ok((fn_type, skip_first_arg, fixed_convention)) } pub fn is_args(&self, name: &syn::Ident) -> bool { @@ -393,6 +456,115 @@ impl<'a> FnSpec<'a> { } false } + + /// Return a C wrapper function for this signature. + pub fn get_wrapper_function( + &self, + ident: &proc_macro2::Ident, + cls: Option<&syn::Type>, + ) -> Result { + let deprecations = &self.deprecations; + let self_conversion = self.tp.self_conversion(cls); + let self_arg = self.tp.self_arg(); + let arg_names = (0..self.args.len()) + .map(|pos| syn::Ident::new(&format!("arg{}", pos), Span::call_site())) + .collect::>(); + let py = syn::Ident::new("_py", Span::call_site()); + let func_name = &self.name; + let rust_name = if let Some(cls) = cls { + quote!(#cls::#func_name) + } else { + quote!(#func_name) + }; + let rust_call = + quote! { pyo3::callback::convert(#py, #rust_name(#self_arg #(#arg_names),*)) }; + Ok(match self.convention { + CallingConvention::Noargs => { + quote! { + unsafe extern "C" fn #ident ( + _slf: *mut pyo3::ffi::PyObject, + _args: *mut pyo3::ffi::PyObject, + ) -> *mut pyo3::ffi::PyObject + { + #deprecations + pyo3::callback::handle_panic(|#py| { + #self_conversion + #rust_call + }) + } + } + } + CallingConvention::Fastcall => { + let arg_convert_and_rust_call = impl_arg_params(self, cls, rust_call, &py, true)?; + quote! { + unsafe extern "C" fn #ident ( + _slf: *mut pyo3::ffi::PyObject, + _args: *const *mut pyo3::ffi::PyObject, + _nargs: pyo3::ffi::Py_ssize_t, + _kwnames: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject + { + #deprecations + pyo3::callback::handle_panic(|#py| { + #self_conversion + let _kwnames: Option<&pyo3::types::PyTuple> = #py.from_borrowed_ptr_or_opt(_kwnames); + // Safety: &PyAny has the same memory layout as `*mut ffi::PyObject` + let _args = _args as *const &pyo3::PyAny; + let _kwargs = if let Some(kwnames) = _kwnames { + std::slice::from_raw_parts(_args.offset(_nargs), kwnames.len()) + } else { + &[] + }; + let _args = std::slice::from_raw_parts(_args, _nargs as usize); + + #arg_convert_and_rust_call + }) + } + } + } + CallingConvention::Varargs => { + let arg_convert_and_rust_call = impl_arg_params(self, cls, rust_call, &py, false)?; + quote! { + unsafe extern "C" fn #ident ( + _slf: *mut pyo3::ffi::PyObject, + _args: *mut pyo3::ffi::PyObject, + _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject + { + #deprecations + pyo3::callback::handle_panic(|#py| { + #self_conversion + let _args = #py.from_borrowed_ptr::(_args); + let _kwargs: Option<&pyo3::types::PyDict> = #py.from_borrowed_ptr_or_opt(_kwargs); + + #arg_convert_and_rust_call + }) + } + } + } + CallingConvention::TpNew => { + let rust_call = quote! { #rust_name(#(#arg_names),*) }; + let arg_convert_and_rust_call = impl_arg_params(self, cls, rust_call, &py, false)?; + quote! { + unsafe extern "C" fn __wrap( + subtype: *mut pyo3::ffi::PyTypeObject, + _args: *mut pyo3::ffi::PyObject, + _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject + { + #deprecations + use pyo3::callback::IntoPyCallbackOutput; + pyo3::callback::handle_panic(|#py| { + let _args = #py.from_borrowed_ptr::(_args); + let _kwargs: Option<&pyo3::types::PyDict> = #py.from_borrowed_ptr_or_opt(_kwargs); + + let result = #arg_convert_and_rust_call; + let initializer: pyo3::PyClassInitializer::<#cls> = result.convert(#py)?; + let cell = initializer.create_cell_from_subtype(#py, subtype)?; + Ok(cell as *mut pyo3::ffi::PyObject) + }) + } + } + } + }) + } } #[derive(Clone, PartialEq, Debug)] @@ -405,7 +577,7 @@ struct MethodAttributes { fn parse_method_attributes( attrs: &mut Vec, mut python_name: Option, -) -> syn::Result { +) -> Result { let mut new_attrs = Vec::new(); let mut args = Vec::new(); let mut ty: Option = None; diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs new file mode 100644 index 00000000000..5745b3234b2 --- /dev/null +++ b/pyo3-macros-backend/src/params.rs @@ -0,0 +1,257 @@ +// Copyright (c) 2017-present PyO3 Project and Contributors + +use crate::{ + attributes::FromPyWithAttribute, + method::{FnArg, FnSpec}, +}; +use proc_macro2::{Span, TokenStream}; +use quote::{quote, quote_spanned}; +use syn::ext::IdentExt; +use syn::spanned::Spanned; +use syn::Result; + +pub fn impl_arg_params( + spec: &FnSpec<'_>, + self_: Option<&syn::Type>, + body: TokenStream, + py: &syn::Ident, + fastcall: bool, +) -> Result { + if spec.args.is_empty() { + return Ok(body); + } + + let mut positional_parameter_names = Vec::new(); + let mut required_positional_parameters = 0usize; + let mut keyword_only_parameters = Vec::new(); + + for arg in spec.args.iter() { + if arg.py || spec.is_args(&arg.name) || spec.is_kwargs(&arg.name) { + continue; + } + let name = arg.name.unraw().to_string(); + let kwonly = spec.is_kw_only(&arg.name); + let required = !(arg.optional.is_some() || spec.default_value(&arg.name).is_some()); + + if kwonly { + keyword_only_parameters.push(quote! { + pyo3::derive_utils::KeywordOnlyParameterDescription { + name: #name, + required: #required, + } + }); + } else { + if required { + required_positional_parameters += 1; + } + positional_parameter_names.push(name); + } + } + + let num_params = positional_parameter_names.len() + keyword_only_parameters.len(); + let args_array = syn::Ident::new("output", Span::call_site()); + + let mut param_conversion = Vec::new(); + let mut option_pos = 0; + for (idx, arg) in spec.args.iter().enumerate() { + param_conversion.push(impl_arg_param( + &arg, + &spec, + idx, + self_, + &mut option_pos, + py, + &args_array, + )?); + } + + let (accept_args, accept_kwargs) = spec.accept_args_kwargs(); + + let cls_name = if let Some(cls) = self_ { + quote! { Some(<#cls as pyo3::type_object::PyTypeInfo>::NAME) } + } else { + quote! { None } + }; + let python_name = &spec.python_name; + + let (args_to_extract, kwargs_to_extract) = if fastcall { + // _args is a &[&PyAny], _kwnames is a Option<&PyTuple> containing the + // keyword names of the keyword args in _kwargs + ( + // need copied() for &&PyAny -> &PyAny + quote! { _args.iter().copied() }, + quote! { _kwnames.map(|kwnames| { + kwnames.as_slice().iter().copied().zip(_kwargs.iter().copied()) + }) }, + ) + } else { + // _args is a &PyTuple, _kwargs is an Option<&PyDict> + ( + quote! { _args.iter() }, + quote! { _kwargs.map(|dict| dict.iter()) }, + ) + }; + + // create array of arguments, and then parse + Ok(quote! {{ + const DESCRIPTION: pyo3::derive_utils::FunctionDescription = pyo3::derive_utils::FunctionDescription { + cls_name: #cls_name, + func_name: stringify!(#python_name), + positional_parameter_names: &[#(#positional_parameter_names),*], + // TODO: https://github.com/PyO3/pyo3/issues/1439 - support specifying these + positional_only_parameters: 0, + required_positional_parameters: #required_positional_parameters, + keyword_only_parameters: &[#(#keyword_only_parameters),*], + accept_varargs: #accept_args, + accept_varkeywords: #accept_kwargs, + }; + + let mut #args_array = [None; #num_params]; + let (_args, _kwargs) = DESCRIPTION.extract_arguments( + #py, + #args_to_extract, + #kwargs_to_extract, + &mut #args_array + )?; + + #(#param_conversion)* + + #body + }}) +} + +/// Re option_pos: The option slice doesn't contain the py: Python argument, so the argument +/// index and the index in option diverge when using py: Python +fn impl_arg_param( + arg: &FnArg<'_>, + spec: &FnSpec<'_>, + idx: usize, + self_: Option<&syn::Type>, + option_pos: &mut usize, + py: &syn::Ident, + args_array: &syn::Ident, +) -> Result { + // Use this macro inside this function, to ensure that all code generated here is associated + // with the function argument + macro_rules! quote_arg_span { + ($($tokens:tt)*) => { quote_spanned!(arg.ty.span() => $($tokens)*) } + } + + let arg_name = syn::Ident::new(&format!("arg{}", idx), Span::call_site()); + + if arg.py { + return Ok(quote_arg_span! { let #arg_name = #py; }); + } + + let ty = arg.ty; + let name = arg.name; + let transform_error = quote_arg_span! { + |e| pyo3::derive_utils::argument_extraction_error(#py, stringify!(#name), e) + }; + + if spec.is_args(&name) { + ensure_spanned!( + arg.optional.is_none(), + arg.name.span() => "args cannot be optional" + ); + return Ok(quote_arg_span! { + let #arg_name = _args.unwrap().extract().map_err(#transform_error)?; + }); + } else if spec.is_kwargs(&name) { + ensure_spanned!( + arg.optional.is_some(), + arg.name.span() => "kwargs must be Option<_>" + ); + return Ok(quote_arg_span! { + let #arg_name = _kwargs.map(|kwargs| kwargs.extract()) + .transpose() + .map_err(#transform_error)?; + }); + } + + let arg_value = quote_arg_span!(#args_array[#option_pos]); + *option_pos += 1; + + let extract = if let Some(FromPyWithAttribute(expr_path)) = &arg.attrs.from_py_with { + quote_arg_span! { #expr_path(_obj).map_err(#transform_error) } + } else { + quote_arg_span! { _obj.extract().map_err(#transform_error) } + }; + + let arg_value_or_default = match (spec.default_value(name), arg.optional.is_some()) { + (Some(default), true) if default.to_string() != "None" => { + quote_arg_span! { #arg_value.map_or_else(|| Ok(Some(#default)), |_obj| #extract)? } + } + (Some(default), _) => { + quote_arg_span! { #arg_value.map_or_else(|| Ok(#default), |_obj| #extract)? } + } + (None, true) => quote_arg_span! { #arg_value.map_or(Ok(None), |_obj| #extract)? }, + (None, false) => { + quote_arg_span! { + { + let _obj = #arg_value.expect("Failed to extract required method argument"); + #extract? + } + } + } + }; + + return if let syn::Type::Reference(tref) = arg.optional.as_ref().unwrap_or(&ty) { + let (tref, mut_) = preprocess_tref(tref, self_); + let (target_ty, borrow_tmp) = if arg.optional.is_some() { + // Get Option<&T> from Option> + ( + quote_arg_span! { Option<<#tref as pyo3::derive_utils::ExtractExt>::Target> }, + if mut_.is_some() { + quote_arg_span! { _tmp.as_deref_mut() } + } else { + quote_arg_span! { _tmp.as_deref() } + }, + ) + } else { + // Get &T from PyRef + ( + quote_arg_span! { <#tref as pyo3::derive_utils::ExtractExt>::Target }, + quote_arg_span! { &#mut_ *_tmp }, + ) + }; + + Ok(quote_arg_span! { + let #mut_ _tmp: #target_ty = #arg_value_or_default; + let #arg_name = #borrow_tmp; + }) + } else { + Ok(quote_arg_span! { + let #arg_name = #arg_value_or_default; + }) + }; + + /// Replace `Self`, remove lifetime and get mutability from the type + fn preprocess_tref( + tref: &syn::TypeReference, + self_: Option<&syn::Type>, + ) -> (syn::TypeReference, Option) { + let mut tref = tref.to_owned(); + if let Some(syn::Type::Path(tpath)) = self_ { + replace_self(&mut tref, &tpath.path); + } + tref.lifetime = None; + let mut_ = tref.mutability; + (tref, mut_) + } + + /// Replace `Self` with the exact type name since it is used out of the impl block + fn replace_self(tref: &mut syn::TypeReference, self_path: &syn::Path) { + match &mut *tref.elem { + syn::Type::Reference(tref_inner) => replace_self(tref_inner, self_path), + syn::Type::Path(tpath) => { + if let Some(ident) = tpath.path.get_ident() { + if ident == "Self" { + tpath.path = self_path.to_owned(); + } + } + } + _ => {} + } + } +} diff --git a/pyo3-macros-backend/src/pyfunction.rs b/pyo3-macros-backend/src/pyfunction.rs index 3e3efe3c311..d0901769124 100644 --- a/pyo3-macros-backend/src/pyfunction.rs +++ b/pyo3-macros-backend/src/pyfunction.rs @@ -7,8 +7,8 @@ use crate::{ TextSignatureAttribute, }, deprecations::Deprecations, - method::{self, FnArg, FnSpec}, - pymethod::{check_generic, get_arg_names, impl_arg_params}, + method::{self, CallingConvention, FnArg}, + pymethod::check_generic, utils::{self, ensure_not_async_fn}, }; use proc_macro2::{Span, TokenStream}; @@ -411,8 +411,13 @@ pub fn impl_wrap_pyfunction( let function_wrapper_ident = function_wrapper_ident(&func.sig.ident); let spec = method::FnSpec { - tp: method::FnType::FnStatic, - name: &function_wrapper_ident, + tp: if options.pass_module.is_some() { + method::FnType::FnModule + } else { + method::FnType::FnStatic + }, + name: &func.sig.ident, + convention: CallingConvention::from_args(&arguments), python_name, attrs: signature.arguments, args: arguments, @@ -426,19 +431,17 @@ pub fn impl_wrap_pyfunction( let name = &func.sig.ident; let wrapper_ident = format_ident!("__pyo3_raw_{}", name); - let wrapper = function_c_wrapper(name, &wrapper_ident, &spec, options.pass_module.is_some())?; - let (methoddef_meth, cfunc_variant) = if spec.args.is_empty() { - (quote!(noargs), quote!(PyCFunction)) - } else if spec.can_use_fastcall() { - ( + let wrapper = spec.get_wrapper_function(&wrapper_ident, None)?; + let (methoddef_meth, cfunc_variant) = match spec.convention { + CallingConvention::Noargs => (quote!(noargs), quote!(PyCFunction)), + CallingConvention::Fastcall => ( quote!(fastcall_cfunction_with_keywords), quote!(PyCFunctionFastWithKeywords), - ) - } else { - ( + ), + _ => ( quote!(cfunction_with_keywords), quote!(PyCFunctionWithKeywords), - ) + ), }; let wrapped_pyfunction = quote! { @@ -459,94 +462,6 @@ pub fn impl_wrap_pyfunction( Ok((function_wrapper_ident, wrapped_pyfunction)) } -/// Generate static function wrapper (PyCFunction, PyCFunctionWithKeywords) -fn function_c_wrapper( - name: &Ident, - wrapper_ident: &Ident, - spec: &FnSpec<'_>, - pass_module: bool, -) -> Result { - let names: Vec = get_arg_names(&spec); - let (cb, slf_module) = if pass_module { - ( - quote! { - pyo3::callback::convert(_py, #name(_slf, #(#names),*)) - }, - Some(quote! { - let _slf = _py.from_borrowed_ptr::(_slf); - }), - ) - } else { - ( - quote! { - pyo3::callback::convert(_py, #name(#(#names),*)) - }, - None, - ) - }; - let py = syn::Ident::new("_py", Span::call_site()); - let deprecations = &spec.deprecations; - if spec.args.is_empty() { - Ok(quote! { - unsafe extern "C" fn #wrapper_ident( - _slf: *mut pyo3::ffi::PyObject, - _unused: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject - { - #deprecations - pyo3::callback::handle_panic(|#py| { - #slf_module - #cb - }) - } - }) - } else if spec.can_use_fastcall() { - let body = impl_arg_params(spec, None, cb, &py, true)?; - Ok(quote! { - unsafe extern "C" fn #wrapper_ident( - _slf: *mut pyo3::ffi::PyObject, - _args: *const *mut pyo3::ffi::PyObject, - _nargs: pyo3::ffi::Py_ssize_t, - _kwnames: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject - { - pyo3::callback::handle_panic(|#py| { - #slf_module - // _nargs is the number of positional arguments in the _args array, - // the number of KW args is given by the length of _kwnames - let _kwnames: Option<&pyo3::types::PyTuple> = #py.from_borrowed_ptr_or_opt(_kwnames); - // Safety: &PyAny has the same memory layout as `*mut ffi::PyObject` - let _args = _args as *const &pyo3::PyAny; - let _kwargs = if let Some(kwnames) = _kwnames { - std::slice::from_raw_parts(_args.offset(_nargs), kwnames.len()) - } else { - &[] - }; - let _args = std::slice::from_raw_parts(_args, _nargs as usize); - - #body - }) - } - - }) - } else { - let body = impl_arg_params(spec, None, cb, &py, false)?; - Ok(quote! { - unsafe extern "C" fn #wrapper_ident( - _slf: *mut pyo3::ffi::PyObject, - _args: *mut pyo3::ffi::PyObject, - _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject - { - #deprecations - pyo3::callback::handle_panic(|#py| { - #slf_module - let _args = #py.from_borrowed_ptr::(_args); - let _kwargs: Option<&pyo3::types::PyDict> = #py.from_borrowed_ptr_or_opt(_kwargs); - #body - }) - } - }) - } -} - fn type_is_pymodule(ty: &syn::Type) -> bool { if let syn::Type::Reference(tyref) = ty { if let syn::Type::Path(typath) = tyref.elem.as_ref() { diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 850cb2b4ca5..b443ddcadf3 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -1,16 +1,18 @@ +// Copyright (c) 2017-present PyO3 Project and Contributors + use std::borrow::Cow; use crate::attributes::NameAttribute; +use crate::konst::ConstSpec; +use crate::method::CallingConvention; use crate::utils::ensure_not_async_fn; -// Copyright (c) 2017-present PyO3 Project and Contributors -use crate::{attributes::FromPyWithAttribute, konst::ConstSpec}; use crate::{deprecations::Deprecations, utils}; use crate::{ method::{FnArg, FnSpec, FnType, SelfType}, pyfunction::PyFunctionOptions, }; use proc_macro2::{Span, TokenStream}; -use quote::{quote, quote_spanned}; +use quote::quote; use syn::{ext::IdentExt, spanned::Spanned, Result}; pub enum GeneratedPyMethod { @@ -31,15 +33,21 @@ pub fn gen_py_method( let spec = FnSpec::parse(sig, &mut *meth_attrs, options)?; Ok(match &spec.tp { - FnType::Fn(self_ty) => { - GeneratedPyMethod::Method(impl_py_method_def(cls, &spec, self_ty, None)?) - } + // ordinary functions (with some specialties) + FnType::Fn(_) => GeneratedPyMethod::Method(impl_py_method_def(cls, &spec, None)?), + FnType::FnClass => GeneratedPyMethod::Method(impl_py_method_def( + cls, + &spec, + Some(quote!(pyo3::ffi::METH_CLASS)), + )?), + FnType::FnStatic => GeneratedPyMethod::Method(impl_py_method_def( + cls, + &spec, + Some(quote!(pyo3::ffi::METH_STATIC)), + )?), + // special prototypes FnType::FnNew => GeneratedPyMethod::New(impl_py_method_def_new(cls, &spec)?), - FnType::FnCall(self_ty) => { - GeneratedPyMethod::Call(impl_py_method_def_call(cls, &spec, self_ty)?) - } - FnType::FnClass => GeneratedPyMethod::Method(impl_py_method_def_class(cls, &spec)?), - FnType::FnStatic => GeneratedPyMethod::Method(impl_py_method_def_static(cls, &spec)?), + FnType::FnCall(_) => GeneratedPyMethod::Call(impl_py_method_def_call(cls, &spec)?), FnType::ClassAttribute => { GeneratedPyMethod::Method(impl_py_method_class_attribute(cls, &spec)) } @@ -57,10 +65,13 @@ pub fn gen_py_method( spec: &spec, }, )?), + FnType::FnModule => { + unreachable!("methods cannot be FnModule") + } }) } -pub(crate) fn check_generic(sig: &syn::Signature) -> syn::Result<()> { +pub fn check_generic(sig: &syn::Signature) -> syn::Result<()> { let err_msg = |typ| format!("Python functions cannot have generic {} parameters", typ); for param in &sig.generics.params { match param { @@ -92,183 +103,10 @@ pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec) -> TokenStream { impl_py_const_class_attribute(&spec, &wrapper) } -/// Generate function wrapper for PyCFunctionWithKeywords -pub fn impl_wrap_cfunction_with_keywords( - cls: &syn::Type, - spec: &FnSpec<'_>, - self_ty: &SelfType, -) -> Result { - let body = impl_call(cls, &spec); - let slf = self_ty.receiver(cls); - let py = syn::Ident::new("_py", Span::call_site()); - let body = impl_arg_params(&spec, Some(cls), body, &py, false)?; - let deprecations = &spec.deprecations; - Ok(quote! {{ - unsafe extern "C" fn __wrap( - _slf: *mut pyo3::ffi::PyObject, - _args: *mut pyo3::ffi::PyObject, - _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject - { - #deprecations - pyo3::callback::handle_panic(|#py| { - #slf - let _args = #py.from_borrowed_ptr::(_args); - let _kwargs: Option<&pyo3::types::PyDict> = #py.from_borrowed_ptr_or_opt(_kwargs); - - #body - }) - } - __wrap - }}) -} - -/// Generate function wrapper for PyCFunctionFastWithKeywords -pub fn impl_wrap_fastcall_cfunction_with_keywords( - cls: &syn::Type, - spec: &FnSpec<'_>, - self_ty: &SelfType, -) -> Result { - let body = impl_call(cls, &spec); - let slf = self_ty.receiver(cls); - let py = syn::Ident::new("_py", Span::call_site()); - let body = impl_arg_params(&spec, Some(cls), body, &py, true)?; - Ok(quote! {{ - unsafe extern "C" fn __wrap( - _slf: *mut pyo3::ffi::PyObject, - _args: *const *mut pyo3::ffi::PyObject, - _nargs: pyo3::ffi::Py_ssize_t, - _kwnames: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject - { - pyo3::callback::handle_panic(|#py| { - #slf - let _kwnames: Option<&pyo3::types::PyTuple> = #py.from_borrowed_ptr_or_opt(_kwnames); - // Safety: &PyAny has the same memory layout as `*mut ffi::PyObject` - let _args = _args as *const &pyo3::PyAny; - let _kwargs = if let Some(kwnames) = _kwnames { - std::slice::from_raw_parts(_args.offset(_nargs), kwnames.len()) - } else { - &[] - }; - let _args = std::slice::from_raw_parts(_args, _nargs as usize); - - #body - }) - } - __wrap - }}) -} - -/// Generate function wrapper PyCFunction -pub fn impl_wrap_noargs(cls: &syn::Type, spec: &FnSpec<'_>, self_ty: &SelfType) -> TokenStream { - let body = impl_call(cls, &spec); - let slf = self_ty.receiver(cls); - let deprecations = &spec.deprecations; - assert!(spec.args.is_empty()); - quote! {{ - unsafe extern "C" fn __wrap( - _slf: *mut pyo3::ffi::PyObject, - _args: *mut pyo3::ffi::PyObject, - ) -> *mut pyo3::ffi::PyObject - { - #deprecations - pyo3::callback::handle_panic(|_py| { - #slf - #body - }) - } - __wrap - }} -} - -/// Generate class method wrapper (PyCFunction, PyCFunctionWithKeywords) -pub fn impl_wrap_new(cls: &syn::Type, spec: &FnSpec<'_>) -> Result { - let name = &spec.name; - let names: Vec = get_arg_names(&spec); - let cb = quote! { #cls::#name(#(#names),*) }; - let py = syn::Ident::new("_py", Span::call_site()); - let body = impl_arg_params(spec, Some(cls), cb, &py, false)?; - let deprecations = &spec.deprecations; - Ok(quote! {{ - #[allow(unused_mut)] - unsafe extern "C" fn __wrap( - subtype: *mut pyo3::ffi::PyTypeObject, - _args: *mut pyo3::ffi::PyObject, - _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject - { - #deprecations - use pyo3::callback::IntoPyCallbackOutput; - pyo3::callback::handle_panic(|#py| { - let _args = #py.from_borrowed_ptr::(_args); - let _kwargs: Option<&pyo3::types::PyDict> = #py.from_borrowed_ptr_or_opt(_kwargs); - - let initializer: pyo3::PyClassInitializer::<#cls> = #body.convert(#py)?; - let cell = initializer.create_cell_from_subtype(#py, subtype)?; - Ok(cell as *mut pyo3::ffi::PyObject) - }) - } - __wrap - }}) -} - -/// Generate class method wrapper (PyCFunction, PyCFunctionWithKeywords) -pub fn impl_wrap_class(cls: &syn::Type, spec: &FnSpec<'_>) -> Result { - let name = &spec.name; - let names: Vec = get_arg_names(&spec); - let cb = quote! { pyo3::callback::convert(_py, #cls::#name(&_cls, #(#names),*)) }; - let py = syn::Ident::new("_py", Span::call_site()); - let body = impl_arg_params(spec, Some(cls), cb, &py, false)?; - let deprecations = &spec.deprecations; - Ok(quote! {{ - #[allow(unused_mut)] - unsafe extern "C" fn __wrap( - _cls: *mut pyo3::ffi::PyObject, - _args: *mut pyo3::ffi::PyObject, - _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject - { - #deprecations - pyo3::callback::handle_panic(|#py| { - let _cls = pyo3::types::PyType::from_type_ptr(#py, _cls as *mut pyo3::ffi::PyTypeObject); - let _args = #py.from_borrowed_ptr::(_args); - let _kwargs: Option<&pyo3::types::PyDict> = #py.from_borrowed_ptr_or_opt(_kwargs); - - #body - }) - } - __wrap - }}) -} - -/// Generate static method wrapper (PyCFunction, PyCFunctionWithKeywords) -pub fn impl_wrap_static(cls: &syn::Type, spec: &FnSpec<'_>) -> Result { - let name = &spec.name; - let names: Vec = get_arg_names(&spec); - let cb = quote! { pyo3::callback::convert(_py, #cls::#name(#(#names),*)) }; - let py = syn::Ident::new("_py", Span::call_site()); - let body = impl_arg_params(spec, Some(cls), cb, &py, false)?; - let deprecations = &spec.deprecations; - Ok(quote! {{ - #[allow(unused_mut)] - unsafe extern "C" fn __wrap( - _slf: *mut pyo3::ffi::PyObject, - _args: *mut pyo3::ffi::PyObject, - _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject - { - #deprecations - pyo3::callback::handle_panic(|#py| { - let _args = #py.from_borrowed_ptr::(_args); - let _kwargs: Option<&pyo3::types::PyDict> = #py.from_borrowed_ptr_or_opt(_kwargs); - - #body - }) - } - __wrap - }}) -} - /// Generate a wrapper for initialization of a class attribute from a method /// annotated with `#[classattr]`. /// To be called in `pyo3::pyclass::initialize_type_object`. -pub fn impl_wrap_class_attribute(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream { +fn impl_wrap_class_attribute(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream { let name = &spec.name; let cb = quote! { #cls::#name() }; let deprecations = &spec.deprecations; @@ -388,7 +226,6 @@ pub(crate) fn impl_wrap_setter( PropertyType::Function { self_type, .. } => self_type.receiver(cls), }; Ok(quote! {{ - #[allow(unused_mut)] unsafe extern "C" fn __wrap( _slf: *mut pyo3::ffi::PyObject, _value: *mut pyo3::ffi::PyObject, _: *mut std::os::raw::c_void) -> std::os::raw::c_int @@ -405,301 +242,40 @@ pub(crate) fn impl_wrap_setter( }}) } -/// This function abstracts away some copied code and can propably be simplified itself -pub fn get_arg_names(spec: &FnSpec) -> Vec { - (0..spec.args.len()) - .map(|pos| syn::Ident::new(&format!("arg{}", pos), Span::call_site())) - .collect() -} - -fn impl_call(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream { - let fname = &spec.name; - let names = get_arg_names(spec); - quote! { pyo3::callback::convert(_py, #cls::#fname(_slf, #(#names),*)) } -} - -pub fn impl_arg_params( - spec: &FnSpec<'_>, - self_: Option<&syn::Type>, - body: TokenStream, - py: &syn::Ident, - fastcall: bool, -) -> Result { - if spec.args.is_empty() { - return Ok(body); - } - - let mut positional_parameter_names = Vec::new(); - let mut required_positional_parameters = 0usize; - let mut keyword_only_parameters = Vec::new(); - - for arg in spec.args.iter() { - if arg.py || spec.is_args(&arg.name) || spec.is_kwargs(&arg.name) { - continue; - } - let name = arg.name.unraw().to_string(); - let kwonly = spec.is_kw_only(&arg.name); - let required = !(arg.optional.is_some() || spec.default_value(&arg.name).is_some()); - - if kwonly { - keyword_only_parameters.push(quote! { - pyo3::derive_utils::KeywordOnlyParameterDescription { - name: #name, - required: #required, - } - }); - } else { - if required { - required_positional_parameters += 1; - } - positional_parameter_names.push(name); - } - } - - let num_params = positional_parameter_names.len() + keyword_only_parameters.len(); - let args_array = syn::Ident::new("output", Span::call_site()); - - let mut param_conversion = Vec::new(); - let mut option_pos = 0; - for (idx, arg) in spec.args.iter().enumerate() { - param_conversion.push(impl_arg_param( - &arg, - &spec, - idx, - self_, - &mut option_pos, - py, - &args_array, - )?); - } - - let (accept_args, accept_kwargs) = spec.accept_args_kwargs(); - - let cls_name = if let Some(cls) = self_ { - quote! { Some(<#cls as pyo3::type_object::PyTypeInfo>::NAME) } - } else { - quote! { None } - }; - let python_name = &spec.python_name; - - let (args_to_extract, kwargs_to_extract) = if fastcall { - // _args is a &[&PyAny], _kwnames is a Option<&PyTuple> containing the - // keyword names of the keyword args in _kwargs - ( - // need copied() for &&PyAny -> &PyAny - quote! { _args.iter().copied() }, - quote! { _kwnames.map(|kwnames| { - kwnames.as_slice().iter().copied().zip(_kwargs.iter().copied()) - }) }, - ) - } else { - // _args is a &PyTuple, _kwargs is an Option<&PyDict> - ( - quote! { _args.iter() }, - quote! { _kwargs.map(|dict| dict.iter()) }, - ) - }; - - // create array of arguments, and then parse - Ok(quote! { - { - const DESCRIPTION: pyo3::derive_utils::FunctionDescription = pyo3::derive_utils::FunctionDescription { - cls_name: #cls_name, - func_name: stringify!(#python_name), - positional_parameter_names: &[#(#positional_parameter_names),*], - // TODO: https://github.com/PyO3/pyo3/issues/1439 - support specifying these - positional_only_parameters: 0, - required_positional_parameters: #required_positional_parameters, - keyword_only_parameters: &[#(#keyword_only_parameters),*], - accept_varargs: #accept_args, - accept_varkeywords: #accept_kwargs, - }; - - let mut #args_array = [None; #num_params]; - let (_args, _kwargs) = DESCRIPTION.extract_arguments( - #py, - #args_to_extract, - #kwargs_to_extract, - &mut #args_array - )?; - - #(#param_conversion)* - - #body - } - }) -} - -/// Re option_pos: The option slice doesn't contain the py: Python argument, so the argument -/// index and the index in option diverge when using py: Python -fn impl_arg_param( - arg: &FnArg<'_>, - spec: &FnSpec<'_>, - idx: usize, - self_: Option<&syn::Type>, - option_pos: &mut usize, - py: &syn::Ident, - args_array: &syn::Ident, -) -> Result { - // Use this macro inside this function, to ensure that all code generated here is associated - // with the function argument - macro_rules! quote_arg_span { - ($($tokens:tt)*) => { quote_spanned!(arg.ty.span() => $($tokens)*) } - } - - let arg_name = syn::Ident::new(&format!("arg{}", idx), Span::call_site()); - - if arg.py { - return Ok(quote_arg_span! { let #arg_name = #py; }); - } - - let ty = arg.ty; - let name = arg.name; - let transform_error = quote_arg_span! { - |e| pyo3::derive_utils::argument_extraction_error(#py, stringify!(#name), e) - }; - - if spec.is_args(&name) { - ensure_spanned!( - arg.optional.is_none(), - arg.name.span() => "args cannot be optional" - ); - return Ok(quote_arg_span! { - let #arg_name = _args.unwrap().extract().map_err(#transform_error)?; - }); - } else if spec.is_kwargs(&name) { - ensure_spanned!( - arg.optional.is_some(), - arg.name.span() => "kwargs must be Option<_>" - ); - return Ok(quote_arg_span! { - let #arg_name = _kwargs.map(|kwargs| kwargs.extract()) - .transpose() - .map_err(#transform_error)?; - }); - } - - let arg_value = quote_arg_span!(#args_array[#option_pos]); - *option_pos += 1; - - let extract = if let Some(FromPyWithAttribute(expr_path)) = &arg.attrs.from_py_with { - quote_arg_span! { #expr_path(_obj).map_err(#transform_error) } - } else { - quote_arg_span! { _obj.extract().map_err(#transform_error) } - }; - - let arg_value_or_default = match (spec.default_value(name), arg.optional.is_some()) { - (Some(default), true) if default.to_string() != "None" => { - quote_arg_span! { #arg_value.map_or_else(|| Ok(Some(#default)), |_obj| #extract)? } - } - (Some(default), _) => { - quote_arg_span! { #arg_value.map_or_else(|| Ok(#default), |_obj| #extract)? } - } - (None, true) => quote_arg_span! { #arg_value.map_or(Ok(None), |_obj| #extract)? }, - (None, false) => { - quote_arg_span! { - { - let _obj = #arg_value.expect("Failed to extract required method argument"); - #extract? - } - } - } - }; - - return if let syn::Type::Reference(tref) = arg.optional.as_ref().unwrap_or(&ty) { - let (tref, mut_) = preprocess_tref(tref, self_); - let (target_ty, borrow_tmp) = if arg.optional.is_some() { - // Get Option<&T> from Option> - ( - quote_arg_span! { Option<<#tref as pyo3::derive_utils::ExtractExt>::Target> }, - if mut_.is_some() { - quote_arg_span! { _tmp.as_deref_mut() } - } else { - quote_arg_span! { _tmp.as_deref() } - }, - ) - } else { - // Get &T from PyRef - ( - quote_arg_span! { <#tref as pyo3::derive_utils::ExtractExt>::Target }, - quote_arg_span! { &#mut_ *_tmp }, - ) - }; - - Ok(quote_arg_span! { - let #mut_ _tmp: #target_ty = #arg_value_or_default; - let #arg_name = #borrow_tmp; - }) - } else { - Ok(quote_arg_span! { - let #arg_name = #arg_value_or_default; - }) - }; - - /// Replace `Self`, remove lifetime and get mutability from the type - fn preprocess_tref( - tref: &syn::TypeReference, - self_: Option<&syn::Type>, - ) -> (syn::TypeReference, Option) { - let mut tref = tref.to_owned(); - if let Some(syn::Type::Path(tpath)) = self_ { - replace_self(&mut tref, &tpath.path); - } - tref.lifetime = None; - let mut_ = tref.mutability; - (tref, mut_) - } - - /// Replace `Self` with the exact type name since it is used out of the impl block - fn replace_self(tref: &mut syn::TypeReference, self_path: &syn::Path) { - match &mut *tref.elem { - syn::Type::Reference(tref_inner) => replace_self(tref_inner, self_path), - syn::Type::Path(tpath) => { - if let Some(ident) = tpath.path.get_ident() { - if ident == "Self" { - tpath.path = self_path.to_owned(); - } - } - } - _ => {} - } - } -} - pub fn impl_py_method_def( cls: &syn::Type, spec: &FnSpec, - self_ty: &SelfType, flags: Option, ) -> Result { + let wrapper_ident = syn::Ident::new("__wrap", Span::call_site()); + let wrapper_def = spec.get_wrapper_function(&wrapper_ident, Some(cls))?; let add_flags = flags.map(|flags| quote!(.flags(#flags))); - let python_name = spec.null_terminated_python_name(); let doc = &spec.doc; - let (methoddef_meth, cfunc_variant) = if spec.args.is_empty() { - (quote!(noargs), quote!(PyCFunction)) - } else if spec.can_use_fastcall() { - ( + let python_name = spec.null_terminated_python_name(); + let methoddef_type = match spec.tp { + FnType::FnStatic => quote!(Static), + FnType::FnClass => quote!(Class), + _ => quote!(Method), + }; + let (methoddef_meth, cfunc_variant) = match spec.convention { + CallingConvention::Noargs => (quote!(noargs), quote!(PyCFunction)), + CallingConvention::Fastcall => ( quote!(fastcall_cfunction_with_keywords), quote!(PyCFunctionFastWithKeywords), - ) - } else { - ( + ), + _ => ( quote!(cfunction_with_keywords), quote!(PyCFunctionWithKeywords), - ) - }; - let wrapper = if spec.args.is_empty() { - impl_wrap_noargs(cls, spec, self_ty) - } else if spec.can_use_fastcall() { - impl_wrap_fastcall_cfunction_with_keywords(cls, &spec, self_ty)? - } else { - impl_wrap_cfunction_with_keywords(cls, &spec, self_ty)? + ), }; Ok(quote! { - pyo3::class::PyMethodDefType::Method({ + pyo3::class::PyMethodDefType:: #methoddef_type ({ pyo3::class::PyMethodDef:: #methoddef_meth ( #python_name, - pyo3::class::methods:: #cfunc_variant (#wrapper), + pyo3::class::methods:: #cfunc_variant ({ + #wrapper_def + #wrapper_ident + }), #doc ) #add_flags @@ -707,48 +283,22 @@ pub fn impl_py_method_def( }) } -pub fn impl_py_method_def_new(cls: &syn::Type, spec: &FnSpec) -> Result { - let wrapper = impl_wrap_new(cls, &spec)?; +fn impl_py_method_def_new(cls: &syn::Type, spec: &FnSpec) -> Result { + let wrapper_ident = syn::Ident::new("__wrap", Span::call_site()); + let wrapper = spec.get_wrapper_function(&wrapper_ident, Some(cls))?; Ok(quote! { impl pyo3::class::impl_::PyClassNewImpl<#cls> for pyo3::class::impl_::PyClassImplCollector<#cls> { fn new_impl(self) -> Option { - Some(#wrapper) + Some({ + #wrapper + #wrapper_ident + }) } } }) } -pub fn impl_py_method_def_class(cls: &syn::Type, spec: &FnSpec) -> Result { - let wrapper = impl_wrap_class(cls, &spec)?; - let python_name = spec.null_terminated_python_name(); - let doc = &spec.doc; - Ok(quote! { - pyo3::class::PyMethodDefType::Class({ - pyo3::class::PyMethodDef::cfunction_with_keywords( - #python_name, - pyo3::class::methods::PyCFunctionWithKeywords(#wrapper), - #doc - ).flags(pyo3::ffi::METH_CLASS) - }) - }) -} - -pub fn impl_py_method_def_static(cls: &syn::Type, spec: &FnSpec) -> Result { - let wrapper = impl_wrap_static(cls, &spec)?; - let python_name = spec.null_terminated_python_name(); - let doc = &spec.doc; - Ok(quote! { - pyo3::class::PyMethodDefType::Static({ - pyo3::class::PyMethodDef::cfunction_with_keywords( - #python_name, - pyo3::class::methods::PyCFunctionWithKeywords(#wrapper), - #doc - ).flags(pyo3::ffi::METH_STATIC) - }) - }) -} - -pub fn impl_py_method_class_attribute(cls: &syn::Type, spec: &FnSpec) -> TokenStream { +fn impl_py_method_class_attribute(cls: &syn::Type, spec: &FnSpec) -> TokenStream { let wrapper = impl_wrap_class_attribute(cls, &spec); let python_name = spec.null_terminated_python_name(); quote! { @@ -761,7 +311,7 @@ pub fn impl_py_method_class_attribute(cls: &syn::Type, spec: &FnSpec) -> TokenSt } } -pub fn impl_py_const_class_attribute(spec: &ConstSpec, wrapper: &TokenStream) -> TokenStream { +fn impl_py_const_class_attribute(spec: &ConstSpec, wrapper: &TokenStream) -> TokenStream { let python_name = &spec.null_terminated_python_name(); quote! { { @@ -775,16 +325,16 @@ pub fn impl_py_const_class_attribute(spec: &ConstSpec, wrapper: &TokenStream) -> } } -pub fn impl_py_method_def_call( - cls: &syn::Type, - spec: &FnSpec, - self_ty: &SelfType, -) -> Result { - let wrapper = impl_wrap_cfunction_with_keywords(cls, &spec, self_ty)?; +fn impl_py_method_def_call(cls: &syn::Type, spec: &FnSpec) -> Result { + let wrapper_ident = syn::Ident::new("__wrap", Span::call_site()); + let wrapper = spec.get_wrapper_function(&wrapper_ident, Some(cls))?; Ok(quote! { impl pyo3::class::impl_::PyClassCallImpl<#cls> for pyo3::class::impl_::PyClassImplCollector<#cls> { fn call_impl(self) -> Option { - Some(#wrapper) + Some({ + #wrapper + #wrapper_ident + }) } } }) diff --git a/pyo3-macros-backend/src/pyproto.rs b/pyo3-macros-backend/src/pyproto.rs index a2f62ae37bd..96f5052e0d1 100644 --- a/pyo3-macros-backend/src/pyproto.rs +++ b/pyo3-macros-backend/src/pyproto.rs @@ -73,8 +73,8 @@ fn impl_proto_impl( None }; - let method = if let FnType::Fn(self_ty) = &fn_spec.tp { - pymethod::impl_py_method_def(ty, &fn_spec, &self_ty, flags)? + let method = if let FnType::Fn(_) = &fn_spec.tp { + pymethod::impl_py_method_def(ty, &fn_spec, flags)? } else { bail_spanned!( met.sig.span() => "expected method with receiver for #[pyproto] method" From c670c57ddb139cad5dbfd44a8001379ee5b7e96e Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Sat, 5 Jun 2021 17:53:35 +0200 Subject: [PATCH 2/3] Refactor PyMethodDef creation too. --- pyo3-macros-backend/src/method.rs | 33 ++++++++++++++++++++++++++- pyo3-macros-backend/src/pyfunction.rs | 27 +++------------------- pyo3-macros-backend/src/pymethod.rs | 27 ++-------------------- 3 files changed, 37 insertions(+), 50 deletions(-) diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 4d78ace7c5c..37bed975267 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -544,7 +544,7 @@ impl<'a> FnSpec<'a> { let rust_call = quote! { #rust_name(#(#arg_names),*) }; let arg_convert_and_rust_call = impl_arg_params(self, cls, rust_call, &py, false)?; quote! { - unsafe extern "C" fn __wrap( + unsafe extern "C" fn #ident ( subtype: *mut pyo3::ffi::PyTypeObject, _args: *mut pyo3::ffi::PyObject, _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject @@ -565,6 +565,37 @@ impl<'a> FnSpec<'a> { } }) } + + /// Return a `PyMethodDef` constructor for this function, matching the selected + /// calling convention. + pub fn get_methoddef(&self, wrapper: impl ToTokens) -> TokenStream { + let python_name = self.null_terminated_python_name(); + let doc = &self.doc; + match self.convention { + CallingConvention::Noargs => quote! { + pyo3::class::methods::PyMethodDef::noargs( + #python_name, + pyo3::class::methods::PyCFunction(#wrapper), + #doc, + ) + }, + CallingConvention::Fastcall => quote! { + pyo3::class::methods::PyMethodDef::fastcall_cfunction_with_keywords( + #python_name, + pyo3::class::methods::PyCFunctionFastWithKeywords(#wrapper), + #doc, + ) + }, + CallingConvention::Varargs => quote! { + pyo3::class::methods::PyMethodDef::cfunction_with_keywords( + #python_name, + pyo3::class::methods::PyCFunctionWithKeywords(#wrapper), + #doc, + ) + }, + CallingConvention::TpNew => unreachable!("tp_new cannot get a methoddef"), + } + } } #[derive(Clone, PartialEq, Debug)] diff --git a/pyo3-macros-backend/src/pyfunction.rs b/pyo3-macros-backend/src/pyfunction.rs index d0901769124..3740d8f7757 100644 --- a/pyo3-macros-backend/src/pyfunction.rs +++ b/pyo3-macros-backend/src/pyfunction.rs @@ -426,37 +426,16 @@ pub fn impl_wrap_pyfunction( deprecations: options.deprecations, }; - let doc = &spec.doc; - let python_name = spec.null_terminated_python_name(); - - let name = &func.sig.ident; - let wrapper_ident = format_ident!("__pyo3_raw_{}", name); + let wrapper_ident = format_ident!("__pyo3_raw_{}", spec.name); let wrapper = spec.get_wrapper_function(&wrapper_ident, None)?; - let (methoddef_meth, cfunc_variant) = match spec.convention { - CallingConvention::Noargs => (quote!(noargs), quote!(PyCFunction)), - CallingConvention::Fastcall => ( - quote!(fastcall_cfunction_with_keywords), - quote!(PyCFunctionFastWithKeywords), - ), - _ => ( - quote!(cfunction_with_keywords), - quote!(PyCFunctionWithKeywords), - ), - }; + let methoddef = spec.get_methoddef(wrapper_ident); let wrapped_pyfunction = quote! { #wrapper pub(crate) fn #function_wrapper_ident<'a>( args: impl Into> ) -> pyo3::PyResult<&'a pyo3::types::PyCFunction> { - pyo3::types::PyCFunction::internal_new( - pyo3::class::methods::PyMethodDef:: #methoddef_meth ( - #python_name, - pyo3::class::methods:: #cfunc_variant (#wrapper_ident), - #doc, - ), - args.into(), - ) + pyo3::types::PyCFunction::internal_new(#methoddef, args.into()) } }; Ok((function_wrapper_ident, wrapped_pyfunction)) diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index b443ddcadf3..60b17449a15 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -4,7 +4,6 @@ use std::borrow::Cow; use crate::attributes::NameAttribute; use crate::konst::ConstSpec; -use crate::method::CallingConvention; use crate::utils::ensure_not_async_fn; use crate::{deprecations::Deprecations, utils}; use crate::{ @@ -250,36 +249,14 @@ pub fn impl_py_method_def( let wrapper_ident = syn::Ident::new("__wrap", Span::call_site()); let wrapper_def = spec.get_wrapper_function(&wrapper_ident, Some(cls))?; let add_flags = flags.map(|flags| quote!(.flags(#flags))); - let doc = &spec.doc; - let python_name = spec.null_terminated_python_name(); let methoddef_type = match spec.tp { FnType::FnStatic => quote!(Static), FnType::FnClass => quote!(Class), _ => quote!(Method), }; - let (methoddef_meth, cfunc_variant) = match spec.convention { - CallingConvention::Noargs => (quote!(noargs), quote!(PyCFunction)), - CallingConvention::Fastcall => ( - quote!(fastcall_cfunction_with_keywords), - quote!(PyCFunctionFastWithKeywords), - ), - _ => ( - quote!(cfunction_with_keywords), - quote!(PyCFunctionWithKeywords), - ), - }; + let methoddef = spec.get_methoddef(quote! {{ #wrapper_def #wrapper_ident }}); Ok(quote! { - pyo3::class::PyMethodDefType:: #methoddef_type ({ - pyo3::class::PyMethodDef:: #methoddef_meth ( - #python_name, - pyo3::class::methods:: #cfunc_variant ({ - #wrapper_def - #wrapper_ident - }), - #doc - ) - #add_flags - }) + pyo3::class::PyMethodDefType::#methoddef_type(#methoddef #add_flags) }) } From 60862c308bc43f825129221d43f7e4c092074760 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Sat, 5 Jun 2021 19:41:39 +0200 Subject: [PATCH 3/3] Update changelog entry. --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a16425788c7..78c3de06ab3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,8 +49,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Reduce LLVM line counts to improve compilation times. [#1604](https://github.com/PyO3/pyo3/pull/1604) - Deprecate string-literal second argument to `#[pyfn(m, "name")]`. [#1610](https://github.com/PyO3/pyo3/pull/1610) - No longer call `PyEval_InitThreads()` in `#[pymodule]` init code. [#1630](https://github.com/PyO3/pyo3/pull/1630) -- Use `METH_FASTCALL` argument passing convention, when possible, to improve `#[pyfunction]` performance. [#1619](https://github.com/PyO3/pyo3/pull/1619) - Deprecate `#[text_signature = "..."]` attributes in favor of `#[pyo3(text_signature = "...")]`. [#1658](https://github.com/PyO3/pyo3/pull/1658) +- Use `METH_FASTCALL` argument passing convention, when possible, to improve `#[pyfunction]` and method performance. + [#1619](https://github.com/PyO3/pyo3/pull/1619), [#1660](https://github.com/PyO3/pyo3/pull/1660) ### Removed - Remove deprecated exception names `BaseException` etc. [#1426](https://github.com/PyO3/pyo3/pull/1426)