From 4d471077b4becf896924df899b33bd142164f24d Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Tue, 8 Feb 2022 19:39:37 +0000 Subject: [PATCH 1/4] refactor: inline pyclass_default_items trait --- pyo3-macros-backend/src/pyclass.rs | 46 ++++++++++++++++-------------- pyo3-macros-backend/src/pyimpl.rs | 34 +++++----------------- src/impl_/pyclass.rs | 3 -- 3 files changed, 31 insertions(+), 52 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 18c29957033..4a1b594ff1b 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -497,23 +497,15 @@ fn impl_enum_class( let cls = enum_.ident; let variants = enum_.variants; let pytypeinfo = impl_pytypeinfo(cls, args, None); - let pyclass_impls = PyClassImplsBuilder::new( - cls, - args, - methods_type, - unit_variants_as_items(cls, variants.iter().map(|v| v.ident)), - ) - .doc(doc) - .impl_all(); - let default_repr_impl = { + let default_repr_impl: syn::ImplItemMethod = { 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, } }); - quote! { + syn::parse_quote! { #[doc(hidden)] #[allow(non_snake_case)] #[pyo3(name = "__repr__")] @@ -533,7 +525,7 @@ fn impl_enum_class( let variant_name = variant.ident; quote! { #cls::#variant_name => #cls::#variant_name as #repr_type, } }); - quote! { + syn::parse_quote! { #[doc(hidden)] #[allow(non_snake_case)] #[pyo3(name = "__int__")] @@ -553,7 +545,7 @@ fn impl_enum_class( Ok(true.to_object(py)), } }); - quote! { + syn::parse_quote! { #[doc(hidden)] #[allow(non_snake_case)] #[pyo3(name = "__richcmp__")] @@ -584,8 +576,16 @@ fn impl_enum_class( } }; - let default_items = - gen_default_items(cls, vec![default_repr_impl, default_richcmp, default_int]); + let mut default_methods = vec![default_repr_impl, default_richcmp, default_int]; + + let pyclass_impls = PyClassImplsBuilder::new( + cls, + args, + methods_type, + enum_default_items(cls, variants.iter().map(|v| v.ident), &mut default_methods), + ) + .doc(doc) + .impl_all(); Ok(quote! { const _: () = { @@ -595,14 +595,17 @@ fn impl_enum_class( #pyclass_impls - #default_items + impl #cls { + #(#default_methods)* + } }; }) } -fn unit_variants_as_items<'a>( +fn enum_default_items<'a>( cls: &'a syn::Ident, - variant_names: impl IntoIterator, + unit_variant_names: impl IntoIterator, + default_items: &mut [syn::ImplItemMethod], ) -> TokenStream { let cls_type = syn::parse_quote!(#cls); let variant_to_attribute = |ident: &syn::Ident| ConstSpec { @@ -613,14 +616,16 @@ fn unit_variants_as_items<'a>( deprecations: Default::default(), }, }; - let py_methods = variant_names + let py_methods = unit_variant_names .into_iter() .map(|var| gen_py_const(&cls_type, &variant_to_attribute(var))); + let slots = gen_default_items(cls, default_items); + quote! { _pyo3::impl_::pyclass::PyClassItems { methods: &[#(#py_methods),*], - slots: &[] + slots: &[#(#slots),*] } } } @@ -918,9 +923,6 @@ impl<'a> PyClassImplsBuilder<'a> { let collector = PyClassImplCollector::::new(); static INTRINSIC_ITEMS: PyClassItems = #default_items; visitor(&INTRINSIC_ITEMS); - // This depends on Python implementation detail; - // an old slot entry will be overriden by newer ones. - visitor(collector.pyclass_default_items()); #pymethods_items #pyproto_items } diff --git a/pyo3-macros-backend/src/pyimpl.rs b/pyo3-macros-backend/src/pyimpl.rs index adc45a6276c..c648acb43fd 100644 --- a/pyo3-macros-backend/src/pyimpl.rs +++ b/pyo3-macros-backend/src/pyimpl.rs @@ -186,24 +186,20 @@ pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec) -> TokenStream { } } -pub fn gen_default_items(cls: &syn::Ident, method_defs: Vec) -> TokenStream { +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); - let mut method_defs: Vec<_> = method_defs - .into_iter() - .map(|token| syn::parse2::(token).unwrap()) - .collect(); - - let mut proto_impls = Vec::new(); - - for meth in &mut method_defs { + method_defs.into_iter().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); - proto_impls.push(quote!(#(#attrs)* #token_stream)) + quote!(#(#attrs)* #token_stream) } GeneratedPyMethod::SlotTraitImpl(..) => { panic!("SlotFragment methods cannot have default implementation!") @@ -212,23 +208,7 @@ pub fn gen_default_items(cls: &syn::Ident, method_defs: Vec) -> Tok panic!("Only protocol methods can have default implementation!") } } - } - - quote! { - impl #cls { - #(#method_defs)* - } - impl _pyo3::impl_::pyclass::PyClassDefaultItems<#cls> - for _pyo3::impl_::pyclass::PyClassImplCollector<#cls> { - fn pyclass_default_items(self) -> &'static _pyo3::impl_::pyclass::PyClassItems { - static ITEMS: _pyo3::impl_::pyclass::PyClassItems = _pyo3::impl_::pyclass::PyClassItems { - methods: &[], - slots: &[#(#proto_impls),*] - }; - &ITEMS - } - } - } + }) } fn impl_py_methods( diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index 8f33b964196..ff6f8b496b8 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -757,9 +757,6 @@ mod pyproto_traits { #[cfg(feature = "pyproto")] pub use pyproto_traits::*; -// items that PyO3 implements by default, but can be overidden by the users. -items_trait!(PyClassDefaultItems, pyclass_default_items); - // Protocol slots from #[pymethods] if not using inventory. #[cfg(not(feature = "multiple-pymethods"))] items_trait!(PyMethodsProtocolItems, methods_protocol_items); From 75e44585deba3fcb5be46fb687f43b0219a7b68d Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Tue, 8 Feb 2022 21:55:21 +0000 Subject: [PATCH 2/4] refactor: pass tp_alloc and tp_free via slots --- guide/src/class.md | 10 --- pyo3-macros-backend/src/pyclass.rs | 105 +++++++++++++++-------------- pyo3-macros-backend/src/pyimpl.rs | 2 +- src/impl_/pyclass.rs | 28 -------- src/pyclass.rs | 11 --- 5 files changed, 54 insertions(+), 102 deletions(-) diff --git a/guide/src/class.md b/guide/src/class.md index fdc5fc264ff..c2260d29af1 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -986,16 +986,6 @@ impl pyo3::impl_::pyclass::PyClassImpl for MyClass { let collector = PyClassImplCollector::::new(); collector.new_impl() } - fn get_alloc() -> Option { - use pyo3::impl_::pyclass::*; - let collector = PyClassImplCollector::::new(); - collector.alloc_impl() - } - fn get_free() -> Option { - use pyo3::impl_::pyclass::*; - let collector = PyClassImplCollector::::new(); - collector.free_impl() - } } # Python::with_gil(|py| { # let cls = py.get_type::(); diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 4a1b594ff1b..43081201a70 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -387,6 +387,7 @@ fn impl_class( attr, methods_type, descriptors_to_items(cls, field_options)?, + vec![], ) .doc(doc) .impl_all(); @@ -582,7 +583,8 @@ fn impl_enum_class( cls, args, methods_type, - enum_default_items(cls, variants.iter().map(|v| v.ident), &mut default_methods), + enum_default_methods(cls, variants.iter().map(|v| v.ident)), + enum_default_slots(cls, &mut default_methods), ) .doc(doc) .impl_all(); @@ -602,11 +604,10 @@ fn impl_enum_class( }) } -fn enum_default_items<'a>( +fn enum_default_methods<'a>( cls: &'a syn::Ident, unit_variant_names: impl IntoIterator, - default_items: &mut [syn::ImplItemMethod], -) -> TokenStream { +) -> Vec { let cls_type = syn::parse_quote!(#cls); let variant_to_attribute = |ident: &syn::Ident| ConstSpec { rust_ident: ident.clone(), @@ -616,18 +617,17 @@ fn enum_default_items<'a>( deprecations: Default::default(), }, }; - let py_methods = unit_variant_names + unit_variant_names .into_iter() - .map(|var| gen_py_const(&cls_type, &variant_to_attribute(var))); - - let slots = gen_default_items(cls, default_items); + .map(|var| gen_py_const(&cls_type, &variant_to_attribute(var))) + .collect() +} - quote! { - _pyo3::impl_::pyclass::PyClassItems { - methods: &[#(#py_methods),*], - slots: &[#(#slots),*] - } - } +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 { @@ -642,9 +642,9 @@ fn extract_variant_data(variant: &syn::Variant) -> syn::Result, -) -> syn::Result { +) -> syn::Result> { let ty = syn::parse_quote!(#cls); - let py_methods: Vec = field_options + field_options .into_iter() .enumerate() .flat_map(|(field_index, (field, options))| { @@ -676,14 +676,7 @@ fn descriptors_to_items( name_err.into_iter().chain(getter).chain(setter) }) - .collect::>()?; - - Ok(quote! { - _pyo3::impl_::pyclass::PyClassItems { - methods: &[#(#py_methods),*], - slots: &[] - } - }) + .collect::>() } fn impl_pytypeinfo( @@ -727,7 +720,8 @@ struct PyClassImplsBuilder<'a> { cls: &'a syn::Ident, attr: &'a PyClassArgs, methods_type: PyClassMethodsType, - default_items: TokenStream, + default_methods: Vec, + default_slots: Vec, doc: Option, } @@ -736,13 +730,15 @@ impl<'a> PyClassImplsBuilder<'a> { cls: &'a syn::Ident, attr: &'a PyClassArgs, methods_type: PyClassMethodsType, - default_items: TokenStream, + default_methods: Vec, + default_slots: Vec, ) -> Self { Self { cls, attr, methods_type, - default_items, + default_methods, + default_slots, doc: None, } } @@ -904,7 +900,9 @@ impl<'a> PyClassImplsBuilder<'a> { None }; - let default_items = &self.default_items; + let default_methods = &self.default_methods; + let default_slots = &self.default_slots; + let freelist_slots = self.freelist_slots(); quote! { impl _pyo3::impl_::pyclass::PyClassImpl for #cls { @@ -921,7 +919,10 @@ impl<'a> PyClassImplsBuilder<'a> { fn for_all_items(visitor: &mut dyn ::std::ops::FnMut(& _pyo3::impl_::pyclass::PyClassItems)) { use _pyo3::impl_::pyclass::*; let collector = PyClassImplCollector::::new(); - static INTRINSIC_ITEMS: PyClassItems = #default_items; + static INTRINSIC_ITEMS: PyClassItems = PyClassItems { + methods: &[#(#default_methods),*], + slots: &[#(#default_slots),* #(#freelist_slots),*], + }; visitor(&INTRINSIC_ITEMS); #pymethods_items #pyproto_items @@ -931,16 +932,6 @@ impl<'a> PyClassImplsBuilder<'a> { let collector = PyClassImplCollector::::new(); collector.new_impl() } - fn get_alloc() -> ::std::option::Option<_pyo3::ffi::allocfunc> { - use _pyo3::impl_::pyclass::*; - let collector = PyClassImplCollector::::new(); - collector.alloc_impl() - } - fn get_free() -> ::std::option::Option<_pyo3::ffi::freefunc> { - use _pyo3::impl_::pyclass::*; - let collector = PyClassImplCollector::::new(); - collector.free_impl() - } #dict_offset @@ -969,23 +960,33 @@ impl<'a> PyClassImplsBuilder<'a> { } } } + } + }) + } - impl _pyo3::impl_::pyclass::PyClassAllocImpl<#cls> for _pyo3::impl_::pyclass::PyClassImplCollector<#cls> { - #[inline] - fn alloc_impl(self) -> ::std::option::Option<_pyo3::ffi::allocfunc> { - ::std::option::Option::Some(_pyo3::impl_::pyclass::alloc_with_freelist::<#cls>) - } - } + fn freelist_slots(&self) -> Vec { + let cls = self.cls; - impl _pyo3::impl_::pyclass::PyClassFreeImpl<#cls> for _pyo3::impl_::pyclass::PyClassImplCollector<#cls> { - #[inline] - fn free_impl(self) -> ::std::option::Option<_pyo3::ffi::freefunc> { - ::std::option::Option::Some(_pyo3::impl_::pyclass::free_with_freelist::<#cls>) + if self.attr.freelist.is_some() { + vec![ + quote! { + _pyo3::ffi::PyType_Slot { + slot: _pyo3::ffi::Py_tp_alloc, + pfunc: _pyo3::impl_::pyclass::alloc_with_freelist::<#cls> as *mut _, } - } - } - }) + }, + quote! { + _pyo3::ffi::PyType_Slot { + slot: _pyo3::ffi::Py_tp_free, + pfunc: _pyo3::impl_::pyclass::free_with_freelist::<#cls> as *mut _, + } + }, + ] + } else { + Vec::new() + } } + /// Enforce at compile time that PyGCProtocol is implemented fn impl_gc(&self) -> TokenStream { let cls = self.cls; diff --git a/pyo3-macros-backend/src/pyimpl.rs b/pyo3-macros-backend/src/pyimpl.rs index c648acb43fd..33aaaec7741 100644 --- a/pyo3-macros-backend/src/pyimpl.rs +++ b/pyo3-macros-backend/src/pyimpl.rs @@ -194,7 +194,7 @@ pub fn gen_default_items<'a>( // all succeed. let ty: syn::Type = syn::parse_quote!(#cls); - method_defs.into_iter().map(move |meth| { + 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) => { diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index ff6f8b496b8..f293aefaea8 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -183,14 +183,6 @@ pub trait PyClassImpl: Sized { None } #[inline] - fn get_alloc() -> Option { - None - } - #[inline] - fn get_free() -> Option { - None - } - #[inline] fn dict_offset() -> Option { None } @@ -602,26 +594,6 @@ macro_rules! generate_pyclass_pow_slot { } pub use generate_pyclass_pow_slot; -pub trait PyClassAllocImpl { - fn alloc_impl(self) -> Option; -} - -impl PyClassAllocImpl for &'_ PyClassImplCollector { - fn alloc_impl(self) -> Option { - None - } -} - -pub trait PyClassFreeImpl { - fn free_impl(self) -> Option; -} - -impl PyClassFreeImpl for &'_ PyClassImplCollector { - fn free_impl(self) -> Option { - None - } -} - /// Implements a freelist. /// /// Do not implement this trait manually. Instead, use `#[pyclass(freelist = N)]` diff --git a/src/pyclass.rs b/src/pyclass.rs index f08669ad674..e070cc80dea 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -51,8 +51,6 @@ where std::mem::size_of::(), T::get_new(), tp_dealloc::, - T::get_alloc(), - T::get_free(), T::dict_offset(), T::weaklist_offset(), &T::for_all_items, @@ -75,8 +73,6 @@ unsafe fn create_type_object_impl( basicsize: usize, tp_new: Option, tp_dealloc: ffi::destructor, - tp_alloc: Option, - tp_free: Option, dict_offset: Option, weaklist_offset: Option, for_all_items: &dyn Fn(&mut dyn FnMut(&PyClassItems)), @@ -101,13 +97,6 @@ unsafe fn create_type_object_impl( ); push_slot(&mut slots, ffi::Py_tp_dealloc, tp_dealloc as _); - if let Some(alloc) = tp_alloc { - push_slot(&mut slots, ffi::Py_tp_alloc, alloc as _); - } - if let Some(free) = tp_free { - push_slot(&mut slots, ffi::Py_tp_free, free as _); - } - #[cfg(Py_3_9)] { let members = py_class_members(dict_offset, weaklist_offset); From f5b2a88a709883331ceed6c27efe0bb398dafd2b Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Tue, 8 Feb 2022 23:44:23 +0000 Subject: [PATCH 3/4] refactor: include __new__ as slot --- guide/src/class.md | 5 ----- pyo3-macros-backend/src/pyclass.rs | 5 ----- pyo3-macros-backend/src/pyimpl.rs | 6 +----- pyo3-macros-backend/src/pymethod.rs | 23 +++++++++++----------- src/impl_/pyclass.rs | 27 -------------------------- src/pyclass.rs | 30 ++++++++++++++++++++--------- tests/ui/invalid_pymethods.rs | 11 +++++++++++ tests/ui/invalid_pymethods.stderr | 11 +++++++++++ 8 files changed, 55 insertions(+), 63 deletions(-) diff --git a/guide/src/class.md b/guide/src/class.md index c2260d29af1..819b3aea782 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -981,11 +981,6 @@ impl pyo3::impl_::pyclass::PyClassImpl for MyClass { visitor(&INTRINSIC_ITEMS); visitor(collector.py_methods()); } - fn get_new() -> Option { - use pyo3::impl_::pyclass::*; - let collector = PyClassImplCollector::::new(); - collector.new_impl() - } } # Python::with_gil(|py| { # let cls = py.get_type::(); diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 43081201a70..f10ba20c2ea 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -927,11 +927,6 @@ impl<'a> PyClassImplsBuilder<'a> { #pymethods_items #pyproto_items } - fn get_new() -> ::std::option::Option<_pyo3::ffi::newfunc> { - use _pyo3::impl_::pyclass::*; - let collector = PyClassImplCollector::::new(); - collector.new_impl() - } #dict_offset diff --git a/pyo3-macros-backend/src/pyimpl.rs b/pyo3-macros-backend/src/pyimpl.rs index 33aaaec7741..e5270641c86 100644 --- a/pyo3-macros-backend/src/pyimpl.rs +++ b/pyo3-macros-backend/src/pyimpl.rs @@ -108,10 +108,6 @@ pub fn impl_methods( let attrs = get_cfg_attributes(&meth.attrs); methods.push(quote!(#(#attrs)* #token_stream)); } - GeneratedPyMethod::TraitImpl(token_stream) => { - let attrs = get_cfg_attributes(&meth.attrs); - trait_impls.push(quote!(#(#attrs)* #token_stream)); - } GeneratedPyMethod::SlotTraitImpl(method_name, token_stream) => { implemented_proto_fragments.insert(method_name); let attrs = get_cfg_attributes(&meth.attrs); @@ -204,7 +200,7 @@ pub fn gen_default_items<'a>( GeneratedPyMethod::SlotTraitImpl(..) => { panic!("SlotFragment methods cannot have default implementation!") } - GeneratedPyMethod::Method(_) | GeneratedPyMethod::TraitImpl(_) => { + GeneratedPyMethod::Method(_) => { panic!("Only protocol methods can have default implementation!") } } diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index c4c901b886d..c914b4d0b0c 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -20,7 +20,6 @@ use syn::{ext::IdentExt, spanned::Spanned, Result}; pub enum GeneratedPyMethod { Method(TokenStream), Proto(TokenStream), - TraitImpl(TokenStream), SlotTraitImpl(String, TokenStream), } @@ -128,7 +127,7 @@ pub fn gen_py_method( Some(quote!(_pyo3::ffi::METH_STATIC)), )?), // special prototypes - (_, FnType::FnNew) => GeneratedPyMethod::TraitImpl(impl_py_method_def_new(cls, spec)?), + (_, FnType::FnNew) => GeneratedPyMethod::Proto(impl_py_method_def_new(cls, spec)?), (_, FnType::Getter(self_type)) => GeneratedPyMethod::Method(impl_py_getter_def( cls, @@ -191,18 +190,18 @@ pub fn impl_py_method_def( } fn impl_py_method_def_new(cls: &syn::Type, spec: &FnSpec) -> Result { - let wrapper_ident = syn::Ident::new("__wrap", 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 _pyo3::impl_::pyclass::PyClassNewImpl<#cls> for _pyo3::impl_::pyclass::PyClassImplCollector<#cls> { - fn new_impl(self) -> ::std::option::Option<_pyo3::ffi::newfunc> { - ::std::option::Option::Some({ - #wrapper - #wrapper_ident - }) - } + Ok(quote! {{ + impl #cls { + #[doc(hidden)] + #wrapper } - }) + _pyo3::ffi::PyType_Slot { + slot: _pyo3::ffi::Py_tp_new, + pfunc: #cls::#wrapper_ident as _pyo3::ffi::newfunc as _ + } + }}) } fn impl_call_slot(cls: &syn::Type, mut spec: FnSpec) -> Result { diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index f293aefaea8..1f3174a0916 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -178,10 +178,6 @@ pub trait PyClassImpl: Sized { fn for_all_items(visitor: &mut dyn FnMut(&PyClassItems)); - #[inline] - fn get_new() -> Option { - None - } #[inline] fn dict_offset() -> Option { None @@ -194,16 +190,6 @@ pub trait PyClassImpl: Sized { // Traits describing known special methods. -pub trait PyClassNewImpl { - fn new_impl(self) -> Option; -} - -impl PyClassNewImpl for &'_ PyClassImplCollector { - fn new_impl(self) -> Option { - None - } -} - macro_rules! slot_fragment_trait { ($trait_name:ident, $($default_method:tt)*) => { #[allow(non_camel_case_types)] @@ -815,19 +801,6 @@ impl PyClassBaseType for T { type Initializer = crate::pyclass_init::PyClassInitializer; } -/// Default new implementation -pub(crate) unsafe extern "C" fn fallback_new( - _subtype: *mut ffi::PyTypeObject, - _args: *mut ffi::PyObject, - _kwds: *mut ffi::PyObject, -) -> *mut ffi::PyObject { - crate::callback_body!(py, { - Err::<(), _>(crate::exceptions::PyTypeError::new_err( - "No constructor defined", - )) - }) -} - /// Implementation of tp_dealloc for all pyclasses pub(crate) unsafe extern "C" fn tp_dealloc(obj: *mut ffi::PyObject) { crate::callback_body!(py, T::Layout::tp_dealloc(obj, py)) diff --git a/src/pyclass.rs b/src/pyclass.rs index e070cc80dea..db93239f704 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -3,8 +3,8 @@ use crate::{ callback::IntoPyCallbackOutput, ffi, impl_::pyclass::{ - assign_sequence_item_from_mapping, fallback_new, get_sequence_item_from_mapping, - tp_dealloc, PyClassDict, PyClassImpl, PyClassItems, PyClassWeakRef, + assign_sequence_item_from_mapping, get_sequence_item_from_mapping, tp_dealloc, PyClassDict, + PyClassImpl, PyClassItems, PyClassWeakRef, }, IntoPy, IntoPyPointer, PyCell, PyErr, PyMethodDefType, PyNativeType, PyObject, PyResult, PyTypeInfo, Python, @@ -49,7 +49,6 @@ where T::NAME, T::BaseType::type_object_raw(py), std::mem::size_of::(), - T::get_new(), tp_dealloc::, T::dict_offset(), T::weaklist_offset(), @@ -71,7 +70,6 @@ unsafe fn create_type_object_impl( name: &str, base_type_object: *mut ffi::PyTypeObject, basicsize: usize, - tp_new: Option, tp_dealloc: ffi::destructor, dict_offset: Option, weaklist_offset: Option, @@ -90,11 +88,6 @@ unsafe fn create_type_object_impl( push_slot(&mut slots, ffi::Py_tp_doc, doc as _); } - push_slot( - &mut slots, - ffi::Py_tp_new, - tp_new.unwrap_or(fallback_new) as _, - ); push_slot(&mut slots, ffi::Py_tp_dealloc, tp_dealloc as _); #[cfg(Py_3_9)] @@ -121,6 +114,7 @@ unsafe fn create_type_object_impl( } // protocol methods + let mut has_new = false; let mut has_getitem = false; let mut has_setitem = false; let mut has_gc_methods = false; @@ -131,6 +125,7 @@ unsafe fn create_type_object_impl( for_all_items(&mut |items| { for slot in items.slots { + has_new |= slot.slot == ffi::Py_tp_new; has_getitem |= slot.slot == ffi::Py_mp_subscript; has_setitem |= slot.slot == ffi::Py_mp_ass_subscript; has_gc_methods |= slot.slot == ffi::Py_tp_clear || slot.slot == ffi::Py_tp_traverse; @@ -171,6 +166,10 @@ unsafe fn create_type_object_impl( ); } + if !has_new { + push_slot(&mut slots, ffi::Py_tp_new, no_constructor_defined as _); + } + // Add empty sentinel at the end push_slot(&mut slots, 0, ptr::null_mut()); @@ -550,3 +549,16 @@ where } } } + +/// Default new implementation +pub(crate) unsafe extern "C" fn no_constructor_defined( + _subtype: *mut ffi::PyTypeObject, + _args: *mut ffi::PyObject, + _kwds: *mut ffi::PyObject, +) -> *mut ffi::PyObject { + crate::callback_body!(py, { + Err::<(), _>(crate::exceptions::PyTypeError::new_err( + "No constructor defined", + )) + }) +} diff --git a/tests/ui/invalid_pymethods.rs b/tests/ui/invalid_pymethods.rs index ed45bac04a1..4fc63dc701f 100644 --- a/tests/ui/invalid_pymethods.rs +++ b/tests/ui/invalid_pymethods.rs @@ -119,4 +119,15 @@ impl MyClass { fn default_arg_before_required(&self, has_default: isize, required: isize) {} } +struct TwoNew { } + +#[pymethods] +impl TwoNew { + #[new] + fn new_1() -> Self { Self { } } + + #[new] + fn new_2() -> Self { Self { } } +} + fn main() {} diff --git a/tests/ui/invalid_pymethods.stderr b/tests/ui/invalid_pymethods.stderr index bc11faba722..2a851154501 100644 --- a/tests/ui/invalid_pymethods.stderr +++ b/tests/ui/invalid_pymethods.stderr @@ -101,3 +101,14 @@ error: `pass_module` cannot be used on Python methods | 112 | #[pyo3(pass_module)] | ^^^^^^^^^^^ + +error[E0592]: duplicate definitions with name `__pymethod__new__` + --> tests/ui/invalid_pymethods.rs:124:1 + | +124 | #[pymethods] + | ^^^^^^^^^^^^ + | | + | 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) From 0c576964f8f1c8a0866ee6d3e270a85a0505eced Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Tue, 8 Feb 2022 23:50:59 +0000 Subject: [PATCH 4/4] changelog: add 2157 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4ee90572eb..585ec0dcf98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,7 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Reduce generated LLVM code size (to improve compile times) for: - internal `handle_panic` helper [#2074](https://github.com/PyO3/pyo3/pull/2074) - `#[pyfunction]` and `#[pymethods]` argument extraction [#2075](https://github.com/PyO3/pyo3/pull/2075) [#2085](https://github.com/PyO3/pyo3/pull/2085) - - `#[pyclass]` type object creation [#2076](https://github.com/PyO3/pyo3/pull/2076) [#2081](https://github.com/PyO3/pyo3/pull/2081) + - `#[pyclass]` type object creation [#2076](https://github.com/PyO3/pyo3/pull/2076) [#2081](https://github.com/PyO3/pyo3/pull/2081) [#2157](https://github.com/PyO3/pyo3/pull/2157) - `__ipow__` now supports modulo argument on Python 3.8+. [#2083](https://github.com/PyO3/pyo3/pull/2083) - `pyo3-macros-backend` is now compiled with PyO3 cfgs to enable different magic method definitions based on version. [#2083](https://github.com/PyO3/pyo3/pull/2083) - `_PyCFunctionFast` now correctly reflects the signature defined in the [Python docs](https://docs.python.org/3/c-api/structures.html#c._PyCFunctionFast). [#2126](https://github.com/PyO3/pyo3/pull/2126)