From a306365db8211e22c90667b9dee9507a4ead5df8 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Tue, 24 May 2022 19:36:24 +0100 Subject: [PATCH] pymethods: prevent methods sharing the same name --- CHANGELOG.md | 1 + pyo3-macros-backend/src/params.rs | 11 +- pyo3-macros-backend/src/pyclass.rs | 79 +++++--- pyo3-macros-backend/src/pyimpl.rs | 40 +--- pyo3-macros-backend/src/pymethod.rs | 280 +++++++++++++++++----------- pyo3-macros-backend/src/utils.rs | 15 -- src/pycell.rs | 4 +- tests/ui/invalid_pymethods.rs | 12 ++ tests/ui/invalid_pymethods.stderr | 17 +- 9 files changed, 261 insertions(+), 198 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8a0cc9f..57271756 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `impl IntoPy for [T; N]` now requires `T: IntoPy` rather than `T: ToPyObject`. [#2326](https://github.com/PyO3/pyo3/pull/2326) - Correct `wrap_pymodule` to match normal namespacing rules: it no longer "sees through" glob imports of `use submodule::*` when `submodule::submodule` is a `#[pymodule]`. [#2363](https://github.com/PyO3/pyo3/pull/2363) - Allow `#[classattr]` methods to be fallible. [#2385](https://github.com/PyO3/pyo3/pull/2385) +- Prevent multiple `#[pymethods]` with the same name for a single `#[pyclass]`. [#2399](https://github.com/PyO3/pyo3/pull/2399) ### Fixed diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs index 219c1e19..04a3981b 100644 --- a/pyo3-macros-backend/src/params.rs +++ b/pyo3-macros-backend/src/params.rs @@ -4,7 +4,7 @@ use crate::{ attributes::FromPyWithAttribute, method::{FnArg, FnSpec}, pyfunction::Argument, - utils::{remove_lifetime, replace_self, unwrap_ty_group}, + utils::{remove_lifetime, unwrap_ty_group}, }; use proc_macro2::{Span, TokenStream}; use quote::{quote, quote_spanned}; @@ -67,7 +67,7 @@ pub fn impl_arg_params( // is (*args, **kwds). let mut arg_convert = vec![]; for (i, arg) in spec.args.iter().enumerate() { - arg_convert.push(impl_arg_param(arg, spec, i, None, &mut 0, py, &args_array)?); + arg_convert.push(impl_arg_param(arg, spec, i, &mut 0, py, &args_array)?); } return Ok(quote! { let _args = #py.from_borrowed_ptr::<_pyo3::types::PyTuple>(_args); @@ -118,7 +118,6 @@ pub fn impl_arg_params( arg, spec, idx, - self_, &mut option_pos, py, &args_array, @@ -189,7 +188,6 @@ 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, @@ -290,10 +288,7 @@ fn impl_arg_param( }; return if let syn::Type::Reference(tref) = unwrap_ty_group(arg.optional.unwrap_or(ty)) { - let mut tref = remove_lifetime(tref); - if let Some(cls) = self_ { - replace_self(&mut tref.elem, cls); - } + let tref = remove_lifetime(tref); let mut_ = tref.mutability; let (target_ty, borrow_tmp) = if arg.optional.is_some() { // Get Option<&T> from Option> diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index d421acf5..cb7cbab7 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -6,9 +6,13 @@ use crate::attributes::{ }; use crate::deprecations::{Deprecation, Deprecations}; use crate::konst::{ConstAttributes, ConstSpec}; -use crate::pyimpl::{gen_default_items, gen_py_const, PyClassMethodsType}; -use crate::pymethod::{impl_py_getter_def, impl_py_setter_def, PropertyType}; +use crate::method::FnSpec; +use crate::pyimpl::{gen_py_const, PyClassMethodsType}; +use crate::pymethod::{ + impl_py_getter_def, impl_py_setter_def, PropertyType, SlotDef, __INT__, __REPR__, __RICHCMP__, +}; use crate::utils::{self, get_pyo3_crate, PythonDoc}; +use crate::PyFunctionOptions; use proc_macro2::{Span, TokenStream}; use quote::quote; use syn::ext::IdentExt; @@ -418,49 +422,48 @@ fn impl_enum_class( krate: syn::Path, ) -> TokenStream { let cls = enum_.ident; + let ty: syn::Type = syn::parse_quote!(#cls); let variants = enum_.variants; let pytypeinfo = impl_pytypeinfo(cls, args, None); - let default_repr_impl: syn::ImplItemMethod = { + let (default_repr, default_repr_slot) = { let variants_repr = variants.iter().map(|variant| { let variant_name = variant.ident; // Assuming all variants are unit variants because they are the only type we support. let repr = format!("{}.{}", cls, variant_name); quote! { #cls::#variant_name => #repr, } }); - syn::parse_quote! { - #[doc(hidden)] - #[allow(non_snake_case)] - #[pyo3(name = "__repr__")] + let mut repr_impl: syn::ImplItemMethod = syn::parse_quote! { fn __pyo3__repr__(&self) -> &'static str { match self { #(#variants_repr)* } } - } + }; + let repr_slot = generate_default_protocol_slot(&ty, &mut repr_impl, &__REPR__).unwrap(); + (repr_impl, repr_slot) }; let repr_type = &enum_.repr_type; - let default_int = { + let (default_int, default_int_slot) = { // This implementation allows us to convert &T to #repr_type without implementing `Copy` let variants_to_int = variants.iter().map(|variant| { let variant_name = variant.ident; quote! { #cls::#variant_name => #cls::#variant_name as #repr_type, } }); - syn::parse_quote! { - #[doc(hidden)] - #[allow(non_snake_case)] - #[pyo3(name = "__int__")] + let mut int_impl: syn::ImplItemMethod = syn::parse_quote! { fn __pyo3__int__(&self) -> #repr_type { match self { #(#variants_to_int)* } } - } + }; + let int_slot = generate_default_protocol_slot(&ty, &mut int_impl, &__INT__).unwrap(); + (int_impl, int_slot) }; - let default_richcmp = { + let (default_richcmp, default_richcmp_slot) = { let variants_eq = variants.iter().map(|variant| { let variant_name = variant.ident; quote! { @@ -468,10 +471,7 @@ fn impl_enum_class( Ok(true.to_object(py)), } }); - syn::parse_quote! { - #[doc(hidden)] - #[allow(non_snake_case)] - #[pyo3(name = "__richcmp__")] + let mut richcmp_impl: syn::ImplItemMethod = syn::parse_quote! { fn __pyo3__richcmp__( &self, py: _pyo3::Python, @@ -496,17 +496,20 @@ fn impl_enum_class( _ => Ok(py.NotImplemented()), } } - } + }; + let richcmp_slot = + generate_default_protocol_slot(&ty, &mut richcmp_impl, &__RICHCMP__).unwrap(); + (richcmp_impl, richcmp_slot) }; - let mut default_methods = vec![default_repr_impl, default_richcmp, default_int]; + let default_slots = vec![default_repr_slot, default_int_slot, default_richcmp_slot]; let pyclass_impls = PyClassImplsBuilder::new( cls, args, methods_type, enum_default_methods(cls, variants.iter().map(|v| v.ident)), - enum_default_slots(cls, &mut default_methods), + default_slots, ) .doc(doc) .impl_all(); @@ -519,13 +522,36 @@ fn impl_enum_class( #pyclass_impls + #[doc(hidden)] + #[allow(non_snake_case)] impl #cls { - #(#default_methods)* + #default_repr + #default_int + #default_richcmp } }; } } +fn generate_default_protocol_slot( + cls: &syn::Type, + method: &mut syn::ImplItemMethod, + slot: &SlotDef, +) -> syn::Result { + let spec = FnSpec::parse( + &mut method.sig, + &mut Vec::new(), + PyFunctionOptions::default(), + ) + .unwrap(); + let name = spec.name.to_string(); + slot.generate_type_slot( + &syn::parse_quote!(#cls), + &spec, + &format!("__default_{}__", name), + ) +} + fn enum_default_methods<'a>( cls: &'a syn::Ident, unit_variant_names: impl IntoIterator, @@ -548,13 +574,6 @@ fn enum_default_methods<'a>( .collect() } -fn enum_default_slots( - cls: &syn::Ident, - default_items: &mut [syn::ImplItemMethod], -) -> Vec { - gen_default_items(cls, default_items).collect() -} - fn extract_variant_data(variant: &syn::Variant) -> syn::Result> { use syn::Fields; let ident = match variant.fields { diff --git a/pyo3-macros-backend/src/pyimpl.rs b/pyo3-macros-backend/src/pyimpl.rs index 3338c76c..1d796970 100644 --- a/pyo3-macros-backend/src/pyimpl.rs +++ b/pyo3-macros-backend/src/pyimpl.rs @@ -11,7 +11,7 @@ use crate::{ }; use proc_macro2::TokenStream; use pymethod::GeneratedPyMethod; -use quote::quote; +use quote::{format_ident, quote}; use syn::{ parse::{Parse, ParseStream}, spanned::Spanned, @@ -164,6 +164,7 @@ pub fn impl_methods( pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec) -> TokenStream { let member = &spec.rust_ident; + let wrapper_ident = format_ident!("__pymethod_{}__", member); let deprecations = &spec.attributes.deprecations; let python_name = &spec.null_terminated_python_name(); quote! { @@ -171,42 +172,21 @@ pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec) -> TokenStream { _pyo3::class::PyClassAttributeDef::new( #python_name, _pyo3::impl_::pymethods::PyClassAttributeFactory({ - fn __wrap(py: _pyo3::Python<'_>) -> _pyo3::PyResult<_pyo3::PyObject> { - #deprecations - ::std::result::Result::Ok(_pyo3::IntoPy::into_py(#cls::#member, py)) + impl #cls { + #[doc(hidden)] + #[allow(non_snake_case)] + fn #wrapper_ident(py: _pyo3::Python<'_>) -> _pyo3::PyResult<_pyo3::PyObject> { + #deprecations + ::std::result::Result::Ok(_pyo3::IntoPy::into_py(#cls::#member, py)) + } } - __wrap + #cls::#wrapper_ident }) ) }) } } -pub fn gen_default_items<'a>( - cls: &syn::Ident, - method_defs: &'a mut [syn::ImplItemMethod], -) -> impl Iterator + 'a { - // This function uses a lot of `unwrap()`; since method_defs are provided by us, they should - // all succeed. - let ty: syn::Type = syn::parse_quote!(#cls); - - method_defs.iter_mut().map(move |meth| { - let options = PyFunctionOptions::from_attrs(&mut meth.attrs).unwrap(); - match pymethod::gen_py_method(&ty, &mut meth.sig, &mut meth.attrs, options).unwrap() { - GeneratedPyMethod::Proto(token_stream) => { - let attrs = get_cfg_attributes(&meth.attrs); - quote!(#(#attrs)* #token_stream) - } - GeneratedPyMethod::SlotTraitImpl(..) => { - panic!("SlotFragment methods cannot have default implementation!") - } - GeneratedPyMethod::Method(_) => { - panic!("Only protocol methods can have default implementation!") - } - } - }) -} - fn impl_py_methods( ty: &syn::Type, methods: Vec, diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 20bb476c..768c9163 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -4,9 +4,7 @@ use std::borrow::Cow; use crate::attributes::NameAttribute; use crate::method::{CallingConvention, ExtractErrorMode}; -use crate::utils::{ - ensure_not_async_fn, remove_lifetime, replace_self, unwrap_ty_group, PythonDoc, -}; +use crate::utils::{ensure_not_async_fn, remove_lifetime, unwrap_ty_group, PythonDoc}; use crate::{deprecations::Deprecations, utils}; use crate::{ method::{FnArg, FnSpec, FnType, SelfType}, @@ -265,7 +263,7 @@ pub fn impl_py_method_def( spec: &FnSpec<'_>, flags: Option, ) -> Result { - let wrapper_ident = syn::Ident::new("__wrap", Span::call_site()); + let wrapper_ident = format_ident!("__pymethod_{}__", spec.python_name); let wrapper_def = spec.get_wrapper_function(&wrapper_ident, Some(cls))?; let add_flags = flags.map(|flags| quote!(.flags(#flags))); let methoddef_type = match spec.tp { @@ -273,18 +271,22 @@ pub fn impl_py_method_def( FnType::FnClass => quote!(Class), _ => quote!(Method), }; - let methoddef = spec.get_methoddef(quote! {{ #wrapper_def #wrapper_ident }}); + let methoddef = spec.get_methoddef(quote! {{ + impl #cls { #[doc(hidden)]#[allow(non_snake_case)] #wrapper_def } + #cls::#wrapper_ident + }}); Ok(quote! { _pyo3::class::PyMethodDefType::#methoddef_type(#methoddef #add_flags) }) } fn impl_py_method_def_new(cls: &syn::Type, spec: &FnSpec<'_>) -> Result { - let wrapper_ident = syn::Ident::new("__pymethod__new__", Span::call_site()); + let wrapper_ident = syn::Ident::new("__pymethod___new____", Span::call_site()); let wrapper = spec.get_wrapper_function(&wrapper_ident, Some(cls))?; Ok(quote! {{ impl #cls { #[doc(hidden)] + #[allow(non_snake_case)] #wrapper } _pyo3::ffi::PyType_Slot { @@ -299,13 +301,17 @@ fn impl_call_slot(cls: &syn::Type, mut spec: FnSpec<'_>) -> Result // Probably indicates there's a refactoring opportunity somewhere. spec.convention = CallingConvention::Varargs; - let wrapper_ident = syn::Ident::new("__wrap", Span::call_site()); + let wrapper_ident = syn::Ident::new("__pymethod___call____", Span::call_site()); let wrapper = spec.get_wrapper_function(&wrapper_ident, Some(cls))?; Ok(quote! {{ - #wrapper + impl #cls { + #[doc(hidden)] + #[allow(non_snake_case)] + #wrapper + } _pyo3::ffi::PyType_Slot { slot: _pyo3::ffi::Py_tp_call, - pfunc: __wrap as _pyo3::ffi::ternaryfunc as _ + pfunc: #cls::#wrapper_ident as _pyo3::ffi::ternaryfunc as _ } }}) } @@ -313,35 +319,38 @@ fn impl_call_slot(cls: &syn::Type, mut spec: FnSpec<'_>) -> Result fn impl_traverse_slot(cls: &syn::Type, spec: FnSpec<'_>) -> TokenStream { let ident = spec.name; quote! {{ - pub unsafe extern "C" fn __wrap_( - slf: *mut _pyo3::ffi::PyObject, - visit: _pyo3::ffi::visitproc, - arg: *mut ::std::os::raw::c_void, - ) -> ::std::os::raw::c_int - { - let pool = _pyo3::GILPool::new(); - let py = pool.python(); - _pyo3::callback::abort_on_traverse_panic(::std::panic::catch_unwind(move || { - let slf = py.from_borrowed_ptr::<_pyo3::PyCell<#cls>>(slf); + impl #cls { + pub unsafe extern "C" fn __pymethod_traverse__( + slf: *mut _pyo3::ffi::PyObject, + visit: _pyo3::ffi::visitproc, + arg: *mut ::std::os::raw::c_void, + ) -> ::std::os::raw::c_int + { + let pool = _pyo3::GILPool::new(); + let py = pool.python(); + _pyo3::callback::abort_on_traverse_panic(::std::panic::catch_unwind(move || { + let slf = py.from_borrowed_ptr::<_pyo3::PyCell<#cls>>(slf); - let visit = _pyo3::class::gc::PyVisit::from_raw(visit, arg, py); - let borrow = slf.try_borrow(); - if let ::std::result::Result::Ok(borrow) = borrow { - _pyo3::impl_::pymethods::unwrap_traverse_result(borrow.#ident(visit)) - } else { - 0 - } - })) + let visit = _pyo3::class::gc::PyVisit::from_raw(visit, arg, py); + let borrow = slf.try_borrow(); + if let ::std::result::Result::Ok(borrow) = borrow { + _pyo3::impl_::pymethods::unwrap_traverse_result(borrow.#ident(visit)) + } else { + 0 + } + })) + } } _pyo3::ffi::PyType_Slot { slot: _pyo3::ffi::Py_tp_traverse, - pfunc: __wrap_ as _pyo3::ffi::traverseproc as _ + pfunc: #cls::__pymethod_traverse__ as _pyo3::ffi::traverseproc as _ } }} } fn impl_py_class_attribute(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream { let name = &spec.name; + let wrapper_ident = format_ident!("__pymethod_{}__", name); let deprecations = &spec.deprecations; let python_name = spec.null_terminated_python_name(); quote! { @@ -349,16 +358,20 @@ fn impl_py_class_attribute(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream { _pyo3::class::PyClassAttributeDef::new( #python_name, _pyo3::impl_::pymethods::PyClassAttributeFactory({ - fn __wrap(py: _pyo3::Python<'_>) -> _pyo3::PyResult<_pyo3::PyObject> { - #deprecations - let mut ret = #cls::#name(); - if false { - use _pyo3::impl_::ghost::IntoPyResult; - ret.assert_into_py_result(); + impl #cls { + #[doc(hidden)] + #[allow(non_snake_case)] + fn #wrapper_ident(py: _pyo3::Python<'_>) -> _pyo3::PyResult<_pyo3::PyObject> { + #deprecations + let mut ret = #cls::#name(); + if false { + use _pyo3::impl_::ghost::IntoPyResult; + ret.assert_into_py_result(); + } + _pyo3::callback::convert(py, ret) } - _pyo3::callback::convert(py, ret) } - __wrap + #cls::#wrapper_ident }) ) }) @@ -418,32 +431,53 @@ pub fn impl_py_setter_def(cls: &syn::Type, property_type: PropertyType<'_>) -> R self_type.receiver(cls, ExtractErrorMode::Raise) } }; + + let wrapper_ident = match property_type { + PropertyType::Descriptor { + field: syn::Field { + ident: Some(ident), .. + }, + .. + } => { + format_ident!("__pymethod_set_{}__", ident) + } + PropertyType::Descriptor { field_index, .. } => { + format_ident!("__pymethod_set_field_{}__", field_index) + } + PropertyType::Function { spec, .. } => { + format_ident!("__pymethod_set_{}__", spec.name) + } + }; Ok(quote! { _pyo3::class::PyMethodDefType::Setter({ #deprecations _pyo3::class::PySetterDef::new( #python_name, _pyo3::impl_::pymethods::PySetter({ - 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 { - let gil = _pyo3::GILPool::new(); - let _py = gil.python(); - _pyo3::callback::panic_result_into_callback_output(_py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> { - #slf - let _value = _py - .from_borrowed_ptr_or_opt(_value) - .ok_or_else(|| { - _pyo3::exceptions::PyAttributeError::new_err("can't delete attribute") - })?; - let _val = _pyo3::FromPyObject::extract(_value)?; + impl #cls { + #[doc(hidden)] + #[allow(non_snake_case)] + unsafe extern "C" fn #wrapper_ident( + _slf: *mut _pyo3::ffi::PyObject, + _value: *mut _pyo3::ffi::PyObject, + _: *mut ::std::os::raw::c_void + ) -> ::std::os::raw::c_int { + let gil = _pyo3::GILPool::new(); + let _py = gil.python(); + _pyo3::callback::panic_result_into_callback_output(_py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> { + #slf + let _value = _py + .from_borrowed_ptr_or_opt(_value) + .ok_or_else(|| { + _pyo3::exceptions::PyAttributeError::new_err("can't delete attribute") + })?; + let _val = _pyo3::FromPyObject::extract(_value)?; - _pyo3::callback::convert(_py, #setter_impl) - })) + _pyo3::callback::convert(_py, #setter_impl) + })) + } } - __wrap + #cls::#wrapper_ident }), #doc ) @@ -516,25 +550,46 @@ pub fn impl_py_getter_def(cls: &syn::Type, property_type: PropertyType<'_>) -> R } }; + let wrapper_ident = match property_type { + PropertyType::Descriptor { + field: syn::Field { + ident: Some(ident), .. + }, + .. + } => { + format_ident!("__pymethod_get_{}__", ident) + } + PropertyType::Descriptor { field_index, .. } => { + format_ident!("__pymethod_get_field_{}__", field_index) + } + PropertyType::Function { spec, .. } => { + format_ident!("__pymethod_get_{}__", spec.name) + } + }; + Ok(quote! { _pyo3::class::PyMethodDefType::Getter({ #deprecations _pyo3::class::PyGetterDef::new( #python_name, _pyo3::impl_::pymethods::PyGetter({ - unsafe extern "C" fn __wrap( - _slf: *mut _pyo3::ffi::PyObject, - _: *mut ::std::os::raw::c_void - ) -> *mut _pyo3::ffi::PyObject { - let gil = _pyo3::GILPool::new(); - let _py = gil.python(); - _pyo3::callback::panic_result_into_callback_output(_py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> { - #slf - let item = #getter_impl; - #conversion - })) + impl #cls { + #[doc(hidden)] + #[allow(non_snake_case)] + unsafe extern "C" fn #wrapper_ident( + _slf: *mut _pyo3::ffi::PyObject, + _: *mut ::std::os::raw::c_void + ) -> *mut _pyo3::ffi::PyObject { + let gil = _pyo3::GILPool::new(); + let _py = gil.python(); + _pyo3::callback::panic_result_into_callback_output(_py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> { + #slf + let item = #getter_impl; + #conversion + })) + } } - __wrap + #cls::#wrapper_ident }), #doc ) @@ -599,13 +654,13 @@ impl PropertyType<'_> { } const __STR__: SlotDef = SlotDef::new("Py_tp_str", "reprfunc"); -const __REPR__: SlotDef = SlotDef::new("Py_tp_repr", "reprfunc"); +pub const __REPR__: SlotDef = SlotDef::new("Py_tp_repr", "reprfunc"); const __HASH__: SlotDef = SlotDef::new("Py_tp_hash", "hashfunc") .ret_ty(Ty::PyHashT) .return_conversion(TokenGenerator( || quote! { _pyo3::callback::HashCallbackOutput }, )); -const __RICHCMP__: SlotDef = SlotDef::new("Py_tp_richcompare", "richcmpfunc") +pub const __RICHCMP__: SlotDef = SlotDef::new("Py_tp_richcompare", "richcmpfunc") .extract_error_mode(ExtractErrorMode::NotImplemented) .arguments(&[Ty::Object, Ty::CompareOp]); const __GET__: SlotDef = SlotDef::new("Py_tp_descr_get", "descrgetfunc") @@ -636,7 +691,7 @@ const __NEG__: SlotDef = SlotDef::new("Py_nb_negative", "unaryfunc"); const __ABS__: SlotDef = SlotDef::new("Py_nb_absolute", "unaryfunc"); const __INVERT__: SlotDef = SlotDef::new("Py_nb_invert", "unaryfunc"); const __INDEX__: SlotDef = SlotDef::new("Py_nb_index", "unaryfunc"); -const __INT__: SlotDef = SlotDef::new("Py_nb_int", "unaryfunc"); +pub const __INT__: SlotDef = SlotDef::new("Py_nb_int", "unaryfunc"); const __FLOAT__: SlotDef = SlotDef::new("Py_nb_float", "unaryfunc"); const __BOOL__: SlotDef = SlotDef::new("Py_nb_bool", "inquiry").ret_ty(Ty::Int); @@ -734,7 +789,6 @@ impl Ty { fn extract( self, - cls: &syn::Type, py: &syn::Ident, ident: &syn::Ident, arg: &FnArg<'_>, @@ -749,7 +803,7 @@ impl Ty { #py.from_borrowed_ptr::<_pyo3::PyAny>(#ident).extract() }, ); - extract_object(cls, arg.ty, ident, extract) + extract_object(arg.ty, ident, extract) } Ty::MaybeNullObject => { let extract = handle_error( @@ -765,7 +819,7 @@ impl Ty { ).extract() }, ); - extract_object(cls, arg.ty, ident, extract) + extract_object(arg.ty, ident, extract) } Ty::NonNullObject => { let extract = handle_error( @@ -775,7 +829,7 @@ impl Ty { #py.from_borrowed_ptr::<_pyo3::PyAny>(#ident.as_ptr()).extract() }, ); - extract_object(cls, arg.ty, ident, extract) + extract_object(arg.ty, ident, extract) } Ty::IPowModulo => { let extract = handle_error( @@ -785,7 +839,7 @@ impl Ty { #ident.extract(#py) }, ); - extract_object(cls, arg.ty, ident, extract) + extract_object(arg.ty, ident, extract) } Ty::CompareOp => { let extract = handle_error( @@ -835,15 +889,9 @@ fn handle_error( } } -fn extract_object( - cls: &syn::Type, - target: &syn::Type, - ident: &syn::Ident, - extract: TokenStream, -) -> TokenStream { +fn extract_object(target: &syn::Type, ident: &syn::Ident, extract: TokenStream) -> TokenStream { if let syn::Type::Reference(tref) = unwrap_ty_group(target) { - let mut tref = remove_lifetime(tref); - replace_self(&mut tref.elem, cls); + let tref = remove_lifetime(tref); let mut_ = tref.mutability; quote! { let #mut_ #ident: <#tref as _pyo3::derive_utils::ExtractExt<'_>>::Target = #extract; @@ -878,7 +926,7 @@ impl ReturnMode { } } -struct SlotDef { +pub struct SlotDef { slot: StaticIdent, func_ty: StaticIdent, arguments: &'static [Ty], @@ -933,7 +981,7 @@ impl SlotDef { self } - fn generate_type_slot( + pub fn generate_type_slot( &self, cls: &syn::Type, spec: &FnSpec<'_>, @@ -955,7 +1003,12 @@ impl SlotDef { ); } let py = syn::Ident::new("_py", Span::call_site()); - let method_arguments = generate_method_arguments(arguments); + let arg_types: &Vec<_> = &arguments.iter().map(|arg| arg.ffi_type()).collect(); + let arg_idents: &Vec<_> = &(0..arguments.len()) + .into_iter() + .map(|i| format_ident!("arg{}", i)) + .collect(); + let wrapper_ident = format_ident!("__pymethod_{}__", method_name); let ret_ty = ret_ty.ffi_type(); let body = generate_method_body( cls, @@ -966,32 +1019,26 @@ impl SlotDef { return_mode.as_ref(), )?; Ok(quote!({ - unsafe extern "C" fn __wrap(_raw_slf: *mut _pyo3::ffi::PyObject, #(#method_arguments),*) -> #ret_ty { - let _slf = _raw_slf; - let gil = _pyo3::GILPool::new(); - let #py = gil.python(); - _pyo3::callback::panic_result_into_callback_output(#py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> { - #body - })) + impl #cls { + #[doc(hidden)] + #[allow(non_snake_case)] + unsafe extern "C" fn #wrapper_ident(_raw_slf: *mut _pyo3::ffi::PyObject, #(#arg_idents: #arg_types),*) -> #ret_ty { + let _slf = _raw_slf; + let gil = _pyo3::GILPool::new(); + let #py = gil.python(); + _pyo3::callback::panic_result_into_callback_output(#py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> { + #body + })) + } } _pyo3::ffi::PyType_Slot { slot: _pyo3::ffi::#slot, - pfunc: __wrap as _pyo3::ffi::#func_ty as _ + pfunc: #cls::#wrapper_ident as _pyo3::ffi::#func_ty as _ } })) } } -fn generate_method_arguments(arguments: &[Ty]) -> impl Iterator + '_ { - arguments.iter().enumerate().map(|(i, arg)| { - let ident = syn::Ident::new(&format!("arg{}", i), Span::call_site()); - let ffi_type = arg.ffi_type(); - quote! { - #ident: #ffi_type - } - }) -} - fn generate_method_body( cls: &syn::Type, spec: &FnSpec<'_>, @@ -1003,7 +1050,7 @@ fn generate_method_body( let self_conversion = spec.tp.self_conversion(Some(cls), extract_error_mode); let rust_name = spec.name; let (arg_idents, arg_count, conversions) = - extract_proto_arguments(cls, py, &spec.args, arguments, extract_error_mode)?; + extract_proto_arguments(py, &spec.args, arguments, extract_error_mode)?; if arg_count != arguments.len() { bail_spanned!(spec.name.span() => format!("Expected {} arguments, got {}", arguments.len(), arg_count)); } @@ -1056,8 +1103,13 @@ impl SlotFragmentDef { } = self; let fragment_trait = format_ident!("PyClass{}SlotFragment", fragment); let method = syn::Ident::new(fragment, Span::call_site()); + let wrapper_ident = format_ident!("__pymethod_{}__", fragment); let py = syn::Ident::new("_py", Span::call_site()); - let method_arguments = generate_method_arguments(arguments); + let arg_types: &Vec<_> = &arguments.iter().map(|arg| arg.ffi_type()).collect(); + let arg_idents: &Vec<_> = &(0..arguments.len()) + .into_iter() + .map(|i| format_ident!("arg{}", i)) + .collect(); let body = generate_method_body(cls, spec, &py, arguments, *extract_error_mode, None)?; let ret_ty = ret_ty.ffi_type(); Ok(quote! { @@ -1068,10 +1120,19 @@ impl SlotFragmentDef { self, #py: _pyo3::Python, _raw_slf: *mut _pyo3::ffi::PyObject, - #(#method_arguments),* + #(#arg_idents: #arg_types),* ) -> _pyo3::PyResult<#ret_ty> { - let _slf = _raw_slf; - #body + impl #cls { + unsafe fn #wrapper_ident( + #py: _pyo3::Python, + _raw_slf: *mut _pyo3::ffi::PyObject, + #(#arg_idents: #arg_types),* + ) -> _pyo3::PyResult<#ret_ty> { + let _slf = _raw_slf; + #body + } + } + #cls::#wrapper_ident(#py, _raw_slf, #(#arg_idents),*) } } }) @@ -1134,7 +1195,6 @@ const __RPOW__: SlotFragmentDef = SlotFragmentDef::new("__rpow__", &[Ty::Object, .ret_ty(Ty::Object); fn extract_proto_arguments( - cls: &syn::Type, py: &syn::Ident, method_args: &[FnArg<'_>], proto_args: &[Ty], @@ -1152,7 +1212,7 @@ fn extract_proto_arguments( let ident = syn::Ident::new(&format!("arg{}", non_python_args), Span::call_site()); let conversions = proto_args.get(non_python_args) .ok_or_else(|| err_spanned!(arg.ty.span() => format!("Expected at most {} non-python arguments", proto_args.len())))? - .extract(cls, py, &ident, arg, extract_error_mode); + .extract(py, &ident, arg, extract_error_mode); non_python_args += 1; args_conversions.push(conversions); arg_idents.push(ident); diff --git a/pyo3-macros-backend/src/utils.rs b/pyo3-macros-backend/src/utils.rs index 471c40d2..88bbffd8 100644 --- a/pyo3-macros-backend/src/utils.rs +++ b/pyo3-macros-backend/src/utils.rs @@ -169,21 +169,6 @@ pub(crate) fn remove_lifetime(tref: &syn::TypeReference) -> syn::TypeReference { tref } -/// Replace `Self` keyword in type with `cls` -pub(crate) fn replace_self(ty: &mut syn::Type, cls: &syn::Type) { - match ty { - syn::Type::Reference(tref) => replace_self(&mut tref.elem, cls), - syn::Type::Path(tpath) => { - if let Some(ident) = tpath.path.get_ident() { - if ident == "Self" { - *ty = cls.to_owned(); - } - } - } - _ => {} - } -} - /// Extract the path to the pyo3 crate, or use the default (`::pyo3`). pub(crate) fn get_pyo3_crate(attr: &Option) -> syn::Path { attr.as_ref() diff --git a/src/pycell.rs b/src/pycell.rs index 17fee01d..928e7c3d 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -56,7 +56,7 @@ //! # } //! # //! // This function is exported to Python. -//! unsafe extern "C" fn __wrap( +//! unsafe extern "C" fn __pymethod_increment__( //! _slf: *mut ::pyo3::ffi::PyObject, //! _args: *mut ::pyo3::ffi::PyObject, //! ) -> *mut ::pyo3::ffi::PyObject { @@ -75,7 +75,7 @@ //! }), //! ) //! } -//! ``` +//! ``` //! //! # When to use PyCell //! ## Using pyclasses from Rust diff --git a/tests/ui/invalid_pymethods.rs b/tests/ui/invalid_pymethods.rs index cad832b1..de78a1af 100644 --- a/tests/ui/invalid_pymethods.rs +++ b/tests/ui/invalid_pymethods.rs @@ -135,4 +135,16 @@ impl TwoNew { fn new_2() -> Self { Self { } } } +struct DuplicateMethod { } + +#[pymethods] +impl DuplicateMethod { + #[pyo3(name = "func")] + fn func_a(&self) { } + + #[pyo3(name = "func")] + fn func_b(&self) { } +} + + fn main() {} diff --git a/tests/ui/invalid_pymethods.stderr b/tests/ui/invalid_pymethods.stderr index 06306bca..57c98123 100644 --- a/tests/ui/invalid_pymethods.stderr +++ b/tests/ui/invalid_pymethods.stderr @@ -109,13 +109,24 @@ error: Python objects are shared, so 'self' cannot be moved out of the Python in 124 | fn method_self_by_value(self){} | ^^^^ -error[E0592]: duplicate definitions with name `__pymethod__new__` +error[E0592]: duplicate definitions with name `__pymethod___new____` --> tests/ui/invalid_pymethods.rs:129:1 | 129 | #[pymethods] | ^^^^^^^^^^^^ | | - | duplicate definitions for `__pymethod__new__` - | other definition for `__pymethod__new__` + | duplicate definitions for `__pymethod___new____` + | other definition for `__pymethod___new____` + | + = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0592]: duplicate definitions with name `__pymethod_func__` + --> tests/ui/invalid_pymethods.rs:140:1 + | +140 | #[pymethods] + | ^^^^^^^^^^^^ + | | + | duplicate definitions for `__pymethod_func__` + | other definition for `__pymethod_func__` | = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)