pymethods: support protocols with multiple-pymethods feature

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,25 +457,28 @@ 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::<<Self as ::pyo3::class::impl_::HasMethodsInventory>::Methods>() {
-                    visitor(::pyo3::class::impl_::PyMethodsInventory::get(inventory));
+                    visitor(::pyo3::class::impl_::PyMethodsInventory::methods(inventory));
                 }
             },
         ),
     };
 
     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::<<Self as ::pyo3::class::impl_::HasMethodsInventory>::Methods>() {
+                    visitor(::pyo3::class::impl_::PyMethodsInventory::slots(inventory));
+                }
+            }
         }
-        PyClassMethodsType::Inventory => None,
     };
 
     let base = &attr.base;

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>) -> TokenStream {
     }
 }
 
-fn impl_protos(
+fn add_shared_proto_slots(
     ty: &syn::Type,
-    mut proto_impls: Vec<TokenStream>,
+    proto_impls: &mut Vec<TokenStream>,
     mut implemented_proto_fragments: HashSet<String>,
-) -> 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>) -> 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>) -> TokenStream {
-    if methods.is_empty() {
-        return TokenStream::default();
-    }
-
+fn submit_methods_inventory(
+    ty: &syn::Type,
+    methods: Vec<TokenStream>,
+    proto_impls: Vec<TokenStream>,
+) -> TokenStream {
     quote! {
         ::pyo3::inventory::submit! {
             #![crate = ::pyo3] {
                 type Inventory = <#ty as ::pyo3::class::impl_::HasMethodsInventory>::Methods;
-                <Inventory as ::pyo3::class::impl_::PyMethodsInventory>::new(::std::vec![#(#methods),*])
+                <Inventory as ::pyo3::class::impl_::PyMethodsInventory>::new(::std::vec![#(#methods),*], ::std::vec![#(#proto_impls),*])
             }
         }
     }

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<PyMethodDefType>) -> Self;
+    fn new(methods: Vec<PyMethodDefType>, slots: Vec<ffi::PyType_Slot>) -> 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);
 

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;

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;

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::*};

tests/test_dunder.rs → 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 {

tests/ui/abi3_nativetype_inheritance.stderr

@@ -4,9 +4,9 @@ error[E0277]: the trait bound `PyDict: PyClass` is not satisfied
 5   | #[pyclass(extends=PyDict)]
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `PyClass` is not implemented for `PyDict`
     |
-   ::: src/class/impl_.rs:775:1
+   ::: src/class/impl_.rs:766:1
     |
-775 | pub trait PyClassBaseType: Sized {
+766 | pub trait PyClassBaseType: Sized {
     | -------------------------------- required by this bound in `PyClassBaseType`
     |
     = note: required because of the requirements on the impl of `PyClassBaseType` for `PyDict`
@@ -18,9 +18,9 @@ error[E0277]: the trait bound `PyDict: PyClass` is not satisfied
 5   | #[pyclass(extends=PyDict)]
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `PyClass` is not implemented for `PyDict`
     |
-   ::: src/class/impl_.rs:762:47
+   ::: src/class/impl_.rs:753:47
     |
-762 | pub struct ThreadCheckerInherited<T: Send, U: PyClassBaseType>(PhantomData<T>, U::ThreadChecker);
+753 | pub struct ThreadCheckerInherited<T: Send, U: PyClassBaseType>(PhantomData<T>, U::ThreadChecker);
     |                                               --------------- required by this bound in `ThreadCheckerInherited`
     |
     = note: required because of the requirements on the impl of `PyClassBaseType` for `PyDict`

tests/ui/deprecations.stderr

@@ -1,21 +1,15 @@
-error: use of deprecated constant `pyo3::impl_::deprecations::CALL_ATTRIBUTE`: use `fn __call__` instead of `#[call]`
-  --> tests/ui/deprecations.rs:26:7
+error: use of deprecated constant `pyo3::impl_::deprecations::NAME_ATTRIBUTE`: use `#[pyo3(name = "...")]` instead of `#[name = "..."]`
+  --> tests/ui/deprecations.rs:14:5
    |
-26 |     #[call]
-   |       ^^^^
+14 |     #[name = "num"]
+   |     ^
    |
 note: the lint level is defined here
   --> tests/ui/deprecations.rs:1:9
    |
 1  | #![deny(deprecated)]
    |         ^^^^^^^^^^
 
-error: use of deprecated constant `pyo3::impl_::deprecations::NAME_ATTRIBUTE`: use `#[pyo3(name = "...")]` instead of `#[name = "..."]`
-  --> tests/ui/deprecations.rs:14:5
-   |
-14 |     #[name = "num"]
-   |     ^
-
 error: use of deprecated constant `pyo3::impl_::deprecations::NAME_ATTRIBUTE`: use `#[pyo3(name = "...")]` instead of `#[name = "..."]`
   --> tests/ui/deprecations.rs:17:5
    |
@@ -40,6 +34,12 @@ error: use of deprecated constant `pyo3::impl_::deprecations::TEXT_SIGNATURE_ATT
 23 |     #[text_signature = "()"]
    |     ^
 
+error: use of deprecated constant `pyo3::impl_::deprecations::CALL_ATTRIBUTE`: use `fn __call__` instead of `#[call]`
+  --> tests/ui/deprecations.rs:26:7
+   |
+26 |     #[call]
+   |       ^^^^
+
 error: use of deprecated constant `pyo3::impl_::deprecations::NAME_ATTRIBUTE`: use `#[pyo3(name = "...")]` instead of `#[name = "..."]`
   --> tests/ui/deprecations.rs:31:1
    |

tests/ui/pyclass_send.stderr

@@ -4,9 +4,9 @@ error[E0277]: `Rc<i32>` cannot be sent between threads safely
 4   | #[pyclass]
     | ^^^^^^^^^^ `Rc<i32>` cannot be sent between threads safely
     |
-   ::: src/class/impl_.rs:706:33
+   ::: src/class/impl_.rs:710:33
     |
-706 | pub struct ThreadCheckerStub<T: Send>(PhantomData<T>);
+710 | pub struct ThreadCheckerStub<T: Send>(PhantomData<T>);
     |                                 ---- required by this bound in `ThreadCheckerStub`
     |
     = help: within `NotThreadSafe`, the trait `Send` is not implemented for `Rc<i32>`