From 52cd9cbc9074a1dc56862f1c3869a9a6a373c911 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Fri, 14 Jul 2023 13:18:57 +0100 Subject: [PATCH] macros: change self_arg to be an expression --- pyo3-macros-backend/src/method.rs | 78 ++++++++----------- pyo3-macros-backend/src/pymethod.rs | 114 +++++++++++++--------------- 2 files changed, 84 insertions(+), 108 deletions(-) diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 2f233a5a..593a10d4 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -8,7 +8,7 @@ use quote::ToTokens; use quote::{quote, quote_spanned}; use syn::ext::IdentExt; use syn::spanned::Spanned; -use syn::{Result, Token}; +use syn::Result; #[derive(Clone, Debug)] pub struct FnArg<'a> { @@ -100,38 +100,31 @@ pub enum FnType { } impl FnType { - pub fn self_conversion( - &self, - cls: Option<&syn::Type>, - error_mode: ExtractErrorMode, - ) -> TokenStream { + pub fn self_arg(&self, cls: Option<&syn::Type>, error_mode: ExtractErrorMode) -> TokenStream { match self { - FnType::Getter(st) | FnType::Setter(st) | FnType::Fn(st) => st.receiver( - cls.expect("no class given for Fn with a \"self\" receiver"), - error_mode, - ), + FnType::Getter(st) | FnType::Setter(st) | FnType::Fn(st) => { + let mut receiver = st.receiver( + cls.expect("no class given for Fn with a \"self\" receiver"), + error_mode, + ); + syn::Token![,](Span::call_site()).to_tokens(&mut receiver); + receiver + } FnType::FnNew | FnType::FnStatic | FnType::ClassAttribute => { quote!() } FnType::FnClass | FnType::FnNewClass => { quote! { - let _slf = _pyo3::types::PyType::from_type_ptr(_py, _slf as *mut _pyo3::ffi::PyTypeObject); + _pyo3::types::PyType::from_type_ptr(_py, _slf as *mut _pyo3::ffi::PyTypeObject), } } FnType::FnModule => { quote! { - let _slf = _py.from_borrowed_ptr::<_pyo3::types::PyModule>(_slf); + _py.from_borrowed_ptr::<_pyo3::types::PyModule>(_slf), } } } } - - pub fn self_arg(&self) -> TokenStream { - match self { - FnType::FnNew | FnType::FnStatic | FnType::ClassAttribute => quote!(), - _ => quote!(_slf,), - } - } } #[derive(Clone, Debug)] @@ -166,40 +159,34 @@ impl SelfType { let _slf = syn::Ident::new("_slf", Span::call_site()); match self { SelfType::Receiver { span, mutable } => { - let (method, mutability) = if *mutable { - ( - quote_spanned! { *span => extract_pyclass_ref_mut }, - Some(Token![mut](*span)), - ) + let method = if *mutable { + syn::Ident::new("extract_pyclass_ref_mut", *span) } else { - (quote_spanned! { *span => extract_pyclass_ref }, None) + syn::Ident::new("extract_pyclass_ref", *span) }; - let extract = error_mode.handle_error( + error_mode.handle_error( &py, quote_spanned! { *span => - _pyo3::impl_::extract_argument::#method( + _pyo3::impl_::extract_argument::#method::<#cls>( #py.from_borrowed_ptr::<_pyo3::PyAny>(#_slf), - &mut holder, + &mut { _pyo3::impl_::extract_argument::FunctionArgumentHolder::INIT }, ) }, - ); - quote_spanned! { *span => - let mut holder = _pyo3::impl_::extract_argument::FunctionArgumentHolder::INIT; - let #_slf: &#mutability #cls = #extract; - } + ) } SelfType::TryFromPyCell(span) => { - let cell = error_mode.handle_error( + error_mode.handle_error( &py, - quote!{ - _py.from_borrowed_ptr::<_pyo3::PyAny>(_slf).downcast::<_pyo3::PyCell<#cls>>() + quote_spanned! { *span => + #py.from_borrowed_ptr::<_pyo3::PyAny>(#_slf).downcast::<_pyo3::PyCell<#cls>>() + .map_err(::std::convert::Into::<_pyo3::PyErr>::into) + .and_then( + #[allow(clippy::useless_conversion)] // In case _slf is PyCell + |cell| ::std::convert::TryFrom::try_from(cell).map_err(::std::convert::Into::into) + ) + } - ); - quote_spanned! { *span => - let _cell = #cell; - #[allow(clippy::useless_conversion)] // In case _slf is PyCell - let #_slf = ::std::convert::TryFrom::try_from(_cell)?; - } + ) } } } @@ -421,8 +408,7 @@ impl<'a> FnSpec<'a> { ident: &proc_macro2::Ident, cls: Option<&syn::Type>, ) -> Result { - let self_conversion = self.tp.self_conversion(cls, ExtractErrorMode::Raise); - let self_arg = self.tp.self_arg(); + let self_arg = self.tp.self_arg(cls, ExtractErrorMode::Raise); let py = syn::Ident::new("_py", Span::call_site()); let func_name = &self.name; @@ -448,13 +434,13 @@ impl<'a> FnSpec<'a> { } else { rust_call(vec![]) }; + quote! { unsafe fn #ident<'py>( #py: _pyo3::Python<'py>, _slf: *mut _pyo3::ffi::PyObject, ) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> { let function = #rust_name; // Shadow the function name to avoid #3017 - #self_conversion #call } } @@ -471,7 +457,6 @@ impl<'a> FnSpec<'a> { _kwnames: *mut _pyo3::ffi::PyObject ) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> { let function = #rust_name; // Shadow the function name to avoid #3017 - #self_conversion #arg_convert #call } @@ -488,7 +473,6 @@ impl<'a> FnSpec<'a> { _kwargs: *mut _pyo3::ffi::PyObject ) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> { let function = #rust_name; // Shadow the function name to avoid #3017 - #self_conversion #arg_convert #call } diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 0e3861ea..c077073c 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -475,8 +475,13 @@ fn impl_py_class_attribute(cls: &syn::Type, spec: &FnSpec<'_>) -> syn::Result) -> syn::Result { +fn impl_call_setter( + cls: &syn::Type, + spec: &FnSpec<'_>, + self_type: &SelfType, +) -> syn::Result { let (py_arg, args) = split_off_python_arg(&spec.signature.arguments); + let slf = self_type.receiver(cls, ExtractErrorMode::Raise); if args.is_empty() { bail_spanned!(spec.name.span() => "setter function expected to have one argument"); @@ -489,9 +494,9 @@ fn impl_call_setter(cls: &syn::Type, spec: &FnSpec<'_>) -> syn::Result { - // named struct field - quote!({ _slf.#ident = _val; }) - } - PropertyType::Descriptor { field_index, .. } => { - // tuple struct field - let index = syn::Index::from(field_index); - quote!({ _slf.#index = _val; }) - } - PropertyType::Function { spec, .. } => impl_call_setter(cls, spec)?, - }; - - let slf = match property_type { - PropertyType::Descriptor { .. } => SelfType::Receiver { - mutable: true, - span: Span::call_site(), - } - .receiver(cls, ExtractErrorMode::Raise), - PropertyType::Function { self_type, .. } => { - self_type.receiver(cls, ExtractErrorMode::Raise) + let slf = SelfType::Receiver { + mutable: true, + span: Span::call_site(), + } + .receiver(cls, ExtractErrorMode::Raise); + if let Some(ident) = &field.ident { + // named struct field + quote!({ #slf.#ident = _val; }) + } else { + // tuple struct field + let index = syn::Index::from(field_index); + quote!({ #slf.#index = _val; }) + } } + PropertyType::Function { + spec, self_type, .. + } => impl_call_setter(cls, spec, self_type)?, }; let wrapper_ident = match property_type { @@ -568,7 +567,6 @@ pub fn impl_py_setter_def( _slf: *mut _pyo3::ffi::PyObject, _value: *mut _pyo3::ffi::PyObject, ) -> _pyo3::PyResult<::std::os::raw::c_int> { - #slf let _value = _py .from_borrowed_ptr_or_opt(_value) .ok_or_else(|| { @@ -597,8 +595,13 @@ pub fn impl_py_setter_def( }) } -fn impl_call_getter(cls: &syn::Type, spec: &FnSpec<'_>) -> syn::Result { +fn impl_call_getter( + cls: &syn::Type, + spec: &FnSpec<'_>, + self_type: &SelfType, +) -> syn::Result { let (py_arg, args) = split_off_python_arg(&spec.signature.arguments); + let slf = self_type.receiver(cls, ExtractErrorMode::Raise); ensure_spanned!( args.is_empty(), args[0].ty.span() => "getter function can only have one argument (of type pyo3::Python)" @@ -606,9 +609,9 @@ fn impl_call_getter(cls: &syn::Type, spec: &FnSpec<'_>) -> syn::Result Result { let python_name = property_type.null_terminated_python_name()?; let doc = property_type.doc(); + let getter_impl = match property_type { PropertyType::Descriptor { - field: syn::Field { - ident: Some(ident), .. - }, - .. + field_index, field, .. } => { - // named struct field - quote!(::std::clone::Clone::clone(&(_slf.#ident))) - } - PropertyType::Descriptor { field_index, .. } => { - // tuple struct field - let index = syn::Index::from(field_index); - quote!(::std::clone::Clone::clone(&(_slf.#index))) - } - PropertyType::Function { spec, .. } => impl_call_getter(cls, spec)?, - }; - - let slf = match property_type { - PropertyType::Descriptor { .. } => SelfType::Receiver { - mutable: false, - span: Span::call_site(), - } - .receiver(cls, ExtractErrorMode::Raise), - PropertyType::Function { self_type, .. } => { - self_type.receiver(cls, ExtractErrorMode::Raise) + let slf = SelfType::Receiver { + mutable: false, + span: Span::call_site(), + } + .receiver(cls, ExtractErrorMode::Raise); + if let Some(ident) = &field.ident { + // named struct field + quote!(::std::clone::Clone::clone(&(#slf.#ident))) + } else { + // tuple struct field + let index = syn::Index::from(field_index); + quote!(::std::clone::Clone::clone(&(#slf.#index))) + } } + PropertyType::Function { + spec, self_type, .. + } => impl_call_getter(cls, spec, self_type)?, }; let conversion = match property_type { @@ -699,7 +697,6 @@ pub fn impl_py_getter_def( _py: _pyo3::Python<'_>, _slf: *mut _pyo3::ffi::PyObject ) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> { - #slf let item = #getter_impl; #conversion } @@ -1151,19 +1148,14 @@ fn generate_method_body( extract_error_mode: ExtractErrorMode, return_mode: Option<&ReturnMode>, ) -> Result { - let self_conversion = spec.tp.self_conversion(Some(cls), extract_error_mode); - let self_arg = spec.tp.self_arg(); + let self_arg = spec.tp.self_arg(Some(cls), extract_error_mode); let rust_name = spec.name; let args = extract_proto_arguments(py, spec, arguments, extract_error_mode)?; let call = quote! { _pyo3::callback::convert(#py, #cls::#rust_name(#self_arg #(#args),*)) }; - let body = if let Some(return_mode) = return_mode { + Ok(if let Some(return_mode) = return_mode { return_mode.return_call_output(py, call) } else { call - }; - Ok(quote! { - #self_conversion - #body }) }