From 0e0e6f8bf52e79374513d54c8361daf7155af7d4 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Mon, 18 Oct 2021 08:30:59 +0100 Subject: [PATCH] pymethods: support protocols with `multiple-pymethods` feature --- pyo3-macros-backend/src/pyclass.rs | 30 +++++++---- pyo3-macros-backend/src/pyimpl.rs | 59 +++++++++++---------- src/class/impl_.rs | 8 ++- tests/test_arithmetics.rs | 2 - tests/test_hygiene.rs | 2 - tests/test_proto_methods.rs | 2 - tests/{test_dunder.rs => test_pyproto.rs} | 24 --------- tests/ui/abi3_nativetype_inheritance.stderr | 18 +++---- tests/ui/pyclass_send.stderr | 4 +- 9 files changed, 67 insertions(+), 82 deletions(-) rename tests/{test_dunder.rs => test_pyproto.rs} (96%) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index d747040dc66..a75b2584c9d 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -346,14 +346,21 @@ fn impl_methods_inventory(cls: &syn::Ident) -> TokenStream { #[doc(hidden)] pub struct #inventory_cls { methods: ::std::vec::Vec<::pyo3::class::PyMethodDefType>, + slots: ::std::vec::Vec<::pyo3::ffi::PyType_Slot>, } impl ::pyo3::class::impl_::PyMethodsInventory for #inventory_cls { - fn new(methods: ::std::vec::Vec<::pyo3::class::PyMethodDefType>) -> Self { - Self { methods } + fn new( + methods: ::std::vec::Vec<::pyo3::class::PyMethodDefType>, + slots: ::std::vec::Vec<::pyo3::ffi::PyType_Slot>, + ) -> Self { + Self { methods, slots } } - fn get(&'static self) -> &'static [::pyo3::class::PyMethodDefType] { + fn methods(&'static self) -> &'static [::pyo3::class::PyMethodDefType] { &self.methods } + fn slots(&'static self) -> &'static [::pyo3::ffi::PyType_Slot] { + &self.slots + } } impl ::pyo3::class::impl_::HasMethodsInventory for #cls { @@ -450,15 +457,12 @@ fn impl_class( }; let (impl_inventory, for_each_py_method) = match methods_type { - PyClassMethodsType::Specialization => ( - ::std::option::Option::None, - quote! { visitor(collector.py_methods()); }, - ), + PyClassMethodsType::Specialization => (None, quote! { visitor(collector.py_methods()); }), PyClassMethodsType::Inventory => ( Some(impl_methods_inventory(cls)), quote! { for inventory in ::pyo3::inventory::iter::<::Methods>() { - visitor(::pyo3::class::impl_::PyMethodsInventory::get(inventory)); + visitor(::pyo3::class::impl_::PyMethodsInventory::methods(inventory)); } }, ), @@ -466,9 +470,15 @@ fn impl_class( let methods_protos = match methods_type { PyClassMethodsType::Specialization => { - Some(quote! { visitor(collector.methods_protocol_slots()); }) + quote! { visitor(collector.methods_protocol_slots()); } + } + PyClassMethodsType::Inventory => { + quote! { + for inventory in ::pyo3::inventory::iter::<::Methods>() { + visitor(::pyo3::class::impl_::PyMethodsInventory::slots(inventory)); + } + } } - PyClassMethodsType::Inventory => None, }; let base = &attr.base; diff --git a/pyo3-macros-backend/src/pyimpl.rs b/pyo3-macros-backend/src/pyimpl.rs index 62477289c00..885c2540dfd 100644 --- a/pyo3-macros-backend/src/pyimpl.rs +++ b/pyo3-macros-backend/src/pyimpl.rs @@ -85,32 +85,29 @@ pub fn impl_methods( } } - let methods_registration = match methods_type { - PyClassMethodsType::Specialization => impl_py_methods(ty, methods), - PyClassMethodsType::Inventory => submit_methods_inventory(ty, methods), - }; + add_shared_proto_slots(ty, &mut proto_impls, implemented_proto_fragments); - let protos_registration = match methods_type { + Ok(match methods_type { PyClassMethodsType::Specialization => { - Some(impl_protos(ty, proto_impls, implemented_proto_fragments)) + let methods_registration = impl_py_methods(ty, methods); + let protos_registration = impl_protos(ty, proto_impls); + + quote! { + #(#trait_impls)* + + #protos_registration + + #methods_registration + } } PyClassMethodsType::Inventory => { - if proto_impls.is_empty() { - None - } else { - panic!( - "cannot implement protos in #[pymethods] using `multiple-pymethods` feature" - ); + let inventory = submit_methods_inventory(ty, methods, proto_impls); + quote! { + #(#trait_impls)* + + #inventory } } - }; - - Ok(quote! { - #(#trait_impls)* - - #protos_registration - - #methods_registration }) } @@ -147,11 +144,11 @@ fn impl_py_methods(ty: &syn::Type, methods: Vec) -> TokenStream { } } -fn impl_protos( +fn add_shared_proto_slots( ty: &syn::Type, - mut proto_impls: Vec, + proto_impls: &mut Vec, mut implemented_proto_fragments: HashSet, -) -> TokenStream { +) { macro_rules! try_add_shared_slot { ($first:literal, $second:literal, $slot:ident) => {{ let first_implemented = implemented_proto_fragments.remove($first); @@ -184,6 +181,10 @@ fn impl_protos( ); try_add_shared_slot!("__pow__", "__rpow__", generate_pyclass_pow_slot); + assert!(implemented_proto_fragments.is_empty()); +} + +fn impl_protos(ty: &syn::Type, proto_impls: Vec) -> TokenStream { quote! { impl ::pyo3::class::impl_::PyMethodsProtocolSlots<#ty> for ::pyo3::class::impl_::PyClassImplCollector<#ty> @@ -195,16 +196,16 @@ fn impl_protos( } } -fn submit_methods_inventory(ty: &syn::Type, methods: Vec) -> TokenStream { - if methods.is_empty() { - return TokenStream::default(); - } - +fn submit_methods_inventory( + ty: &syn::Type, + methods: Vec, + proto_impls: Vec, +) -> TokenStream { quote! { ::pyo3::inventory::submit! { #![crate = ::pyo3] { type Inventory = <#ty as ::pyo3::class::impl_::HasMethodsInventory>::Methods; - ::new(::std::vec![#(#methods),*]) + ::new(::std::vec![#(#methods),*], ::std::vec![#(#proto_impls),*]) } } } diff --git a/src/class/impl_.rs b/src/class/impl_.rs index 0102723997c..e9fb6492034 100644 --- a/src/class/impl_.rs +++ b/src/class/impl_.rs @@ -612,10 +612,13 @@ macro_rules! methods_trait { #[cfg(all(feature = "macros", feature = "multiple-pymethods"))] pub trait PyMethodsInventory: inventory::Collect { /// Create a new instance - fn new(methods: Vec) -> Self; + fn new(methods: Vec, slots: Vec) -> Self; /// Returns the methods for a single `#[pymethods] impl` block - fn get(&'static self) -> &'static [PyMethodDefType]; + fn methods(&'static self) -> &'static [PyMethodDefType]; + + /// Returns the slots for a single `#[pymethods] impl` block + fn slots(&'static self) -> &'static [ffi::PyType_Slot]; } /// Implemented for `#[pyclass]` in our proc macro code. @@ -663,6 +666,7 @@ slots_trait!(PyAsyncProtocolSlots, async_protocol_slots); slots_trait!(PySequenceProtocolSlots, sequence_protocol_slots); slots_trait!(PyBufferProtocolSlots, buffer_protocol_slots); +// Protocol slots from #[pymethods] if not using inventory. #[cfg(not(feature = "multiple-pymethods"))] slots_trait!(PyMethodsProtocolSlots, methods_protocol_slots); diff --git a/tests/test_arithmetics.rs b/tests/test_arithmetics.rs index 64649e08229..1faf8492950 100644 --- a/tests/test_arithmetics.rs +++ b/tests/test_arithmetics.rs @@ -1,5 +1,3 @@ -#![cfg(not(feature = "multiple-pymethods"))] - use pyo3::class::basic::CompareOp; use pyo3::prelude::*; use pyo3::py_run; diff --git a/tests/test_hygiene.rs b/tests/test_hygiene.rs index b1d405b4b3a..47ffa6212eb 100644 --- a/tests/test_hygiene.rs +++ b/tests/test_hygiene.rs @@ -2,8 +2,6 @@ mod hygiene { mod misc; mod pyclass; mod pyfunction; - // cannot implement protos in #[pymethods] using `multiple-pymethods` feature - #[cfg(not(feature = "multiple-pymethods"))] mod pymethods; mod pymodule; mod pyproto; diff --git a/tests/test_proto_methods.rs b/tests/test_proto_methods.rs index f0cada77e25..f60154fe974 100644 --- a/tests/test_proto_methods.rs +++ b/tests/test_proto_methods.rs @@ -1,5 +1,3 @@ -#![cfg(not(feature = "multiple-pymethods"))] - use pyo3::exceptions::PyValueError; use pyo3::types::{PySlice, PyType}; use pyo3::{exceptions::PyAttributeError, prelude::*}; diff --git a/tests/test_dunder.rs b/tests/test_pyproto.rs similarity index 96% rename from tests/test_dunder.rs rename to tests/test_pyproto.rs index 52c190f40d6..da2dc0dee32 100644 --- a/tests/test_dunder.rs +++ b/tests/test_pyproto.rs @@ -210,30 +210,6 @@ fn sequence() { py_expect_exception!(py, c, "c['abc']", PyTypeError); } -#[pyclass] -struct Callable {} - -#[pymethods] -impl Callable { - #[__call__] - fn __call__(&self, arg: i32) -> i32 { - arg * 6 - } -} - -#[test] -fn callable() { - let gil = Python::acquire_gil(); - let py = gil.python(); - - let c = Py::new(py, Callable {}).unwrap(); - py_assert!(py, c, "callable(c)"); - py_assert!(py, c, "c(7) == 42"); - - let nc = Py::new(py, Comparisons { val: 0 }).unwrap(); - py_assert!(py, nc, "not callable(nc)"); -} - #[pyclass] #[derive(Debug)] struct SetItem { diff --git a/tests/ui/abi3_nativetype_inheritance.stderr b/tests/ui/abi3_nativetype_inheritance.stderr index 883a5b9af47..a90d40442f5 100644 --- a/tests/ui/abi3_nativetype_inheritance.stderr +++ b/tests/ui/abi3_nativetype_inheritance.stderr @@ -6,15 +6,15 @@ error[E0277]: the trait bound `PyDict: PyClass` is not satisfied | = note: required because of the requirements on the impl of `PyClassBaseType` for `PyDict` note: required by a bound in `PyClassBaseType` - --> src/class/impl_.rs:762:1 + --> src/class/impl_.rs:766:1 | -762 | / pub trait PyClassBaseType: Sized { -763 | | type Dict; -764 | | type WeakRef; -765 | | type LayoutAsBase: PyCellLayout; +766 | / pub trait PyClassBaseType: Sized { +767 | | type Dict; +768 | | type WeakRef; +769 | | type LayoutAsBase: PyCellLayout; ... | -768 | | type Initializer: PyObjectInit; -769 | | } +772 | | type Initializer: PyObjectInit; +773 | | } | |_^ required by this bound in `PyClassBaseType` = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) @@ -26,8 +26,8 @@ error[E0277]: the trait bound `PyDict: PyClass` is not satisfied | = note: required because of the requirements on the impl of `PyClassBaseType` for `PyDict` note: required by a bound in `ThreadCheckerInherited` - --> src/class/impl_.rs:749:47 + --> src/class/impl_.rs:753:47 | -749 | pub struct ThreadCheckerInherited(PhantomData, U::ThreadChecker); +753 | pub struct ThreadCheckerInherited(PhantomData, U::ThreadChecker); | ^^^^^^^^^^^^^^^ required by this bound in `ThreadCheckerInherited` = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/ui/pyclass_send.stderr b/tests/ui/pyclass_send.stderr index 1c671d0e5ba..56217f1e916 100644 --- a/tests/ui/pyclass_send.stderr +++ b/tests/ui/pyclass_send.stderr @@ -11,8 +11,8 @@ note: required because it appears within the type `NotThreadSafe` 5 | struct NotThreadSafe { | ^^^^^^^^^^^^^ note: required by a bound in `ThreadCheckerStub` - --> src/class/impl_.rs:706:33 + --> src/class/impl_.rs:710:33 | -706 | pub struct ThreadCheckerStub(PhantomData); +710 | pub struct ThreadCheckerStub(PhantomData); | ^^^^ required by this bound in `ThreadCheckerStub` = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info)