Use PyMethodsImpl instead of *ProtocolImpl::methods

Author: kngwyu

guide/src/class.md

@@ -17,76 +17,7 @@ struct MyClass {
 The above example generates implementations for [`PyTypeInfo`], [`PyTypeObject`],
 and [`PyClass`] for `MyClass`.
 
-Specifically, the following implementation is generated:
-
-```rust
-use pyo3::prelude::*;
-
-/// Class for demonstration
-struct MyClass {
-    num: i32,
-    debug: bool,
-}
-
-impl pyo3::pyclass::PyClassAlloc for MyClass {}
-
-unsafe impl pyo3::PyTypeInfo for MyClass {
-    type Type = MyClass;
-    type BaseType = PyAny;
-    type BaseLayout = pyo3::pycell::PyCellBase<PyAny>;
-    type Layout = PyCell<Self>;
-    type Initializer = PyClassInitializer<Self>;
-    type AsRefTarget = PyCell<Self>;
-
-    const NAME: &'static str = "MyClass";
-    const MODULE: Option<&'static str> = None;
-    const DESCRIPTION: &'static str = "Class for demonstration";
-    const FLAGS: usize = 0;
-
-    #[inline]
-    fn type_object() -> &'static pyo3::ffi::PyTypeObject {
-        use pyo3::type_object::LazyStaticType;
-        static TYPE_OBJECT: LazyStaticType = LazyStaticType::new();
-        TYPE_OBJECT.get_or_init::<Self>()
-    }
-}
-
-impl pyo3::pyclass::PyClass for MyClass {
-    type Dict = pyo3::pyclass_slots::PyClassDummySlot;
-    type WeakRef = pyo3::pyclass_slots::PyClassDummySlot;
-    type BaseNativeType = PyAny;
-}
-
-impl pyo3::IntoPy<PyObject> for MyClass {
-    fn into_py(self, py: pyo3::Python) -> pyo3::PyObject {
-        pyo3::IntoPy::into_py(pyo3::Py::new(py, self).unwrap(), py)
-    }
-}
-
-pub struct Pyo3MethodsInventoryForMyClass {
-    methods: &'static [pyo3::class::PyMethodDefType],
-}
-
-impl pyo3::class::methods::PyMethodsInventory for Pyo3MethodsInventoryForMyClass {
-    fn new(methods: &'static [pyo3::class::PyMethodDefType]) -> Self {
-        Self { methods }
-    }
-
-    fn get_methods(&self) -> &'static [pyo3::class::PyMethodDefType] {
-        self.methods
-    }
-}
-
-impl pyo3::class::methods::PyMethodsImpl for MyClass {
-    type Methods = Pyo3MethodsInventoryForMyClass;
-}
-
-pyo3::inventory::collect!(Pyo3MethodsInventoryForMyClass);
-# let gil = Python::acquire_gil();
-# let py = gil.python();
-# let cls = py.get_type::<MyClass>();
-# pyo3::py_run!(py, cls, "assert cls.__name__ == 'MyClass'")
-```
+If you curious what `#[pyclass]` generates, see [How methods are implemented](#how-methods-are-implemented) section.
 
 ## Adding the class to a module
 
@@ -944,7 +875,77 @@ pyclass dependent on whether there is an impl block, we'd need to implement the
 `#[pyclass]` and override the implementation in `#[pymethods]`, which is to the best of my knowledge
 only possible with the specialization feature, which can't be used on stable.
 
-To escape this we use [inventory](https://github.com/dtolnay/inventory), which allows us to collect `impl`s from arbitrary source code by exploiting some binary trick. See [inventory: how it works](https://github.com/dtolnay/inventory#how-it-works) and `pyo3_derive_backend::py_class::impl_inventory` for more details.
+To escape this we use [inventory](https://github.com/dtolnay/inventory),
+which allows us to collect `impl`s from arbitrary source code by exploiting some binary trick.
+See [inventory: how it works](https://github.com/dtolnay/inventory#how-it-works) and `pyo3_derive_backend::py_class` for more details.
+
+Specifically, the following implementation is generated:
+
+```rust
+use pyo3::prelude::*;
+
+/// Class for demonstration
+struct MyClass {
+    num: i32,
+    debug: bool,
+}
+
+impl pyo3::pyclass::PyClassAlloc for MyClass {}
+
+unsafe impl pyo3::PyTypeInfo for MyClass {
+    type Type = MyClass;
+    type BaseType = PyAny;
+    type BaseLayout = pyo3::pycell::PyCellBase<PyAny>;
+    type Layout = PyCell<Self>;
+    type Initializer = PyClassInitializer<Self>;
+    type AsRefTarget = PyCell<Self>;
+
+    const NAME: &'static str = "MyClass";
+    const MODULE: Option<&'static str> = None;
+    const DESCRIPTION: &'static str = "Class for demonstration";
+    const FLAGS: usize = 0;
+
+    #[inline]
+    fn type_object() -> &'static pyo3::ffi::PyTypeObject {
+        use pyo3::type_object::LazyStaticType;
+        static TYPE_OBJECT: LazyStaticType = LazyStaticType::new();
+        TYPE_OBJECT.get_or_init::<Self>()
+    }
+}
+
+impl pyo3::pyclass::PyClass for MyClass {
+    type Dict = pyo3::pyclass_slots::PyClassDummySlot;
+    type WeakRef = pyo3::pyclass_slots::PyClassDummySlot;
+    type BaseNativeType = PyAny;
+}
+
+impl pyo3::IntoPy<PyObject> for MyClass {
+    fn into_py(self, py: pyo3::Python) -> pyo3::PyObject {
+        pyo3::IntoPy::into_py(pyo3::Py::new(py, self).unwrap(), py)
+    }
+}
+
+pub struct Pyo3MethodsInventoryForMyClass {
+    methods: &'static [pyo3::class::PyMethodDefType],
+}
+impl pyo3::class::methods::PyMethodsInventory for Pyo3MethodsInventoryForMyClass {
+    fn new(methods: &'static [pyo3::class::PyMethodDefType]) -> Self {
+        Self { methods }
+    }
+    fn get(&self) -> &'static [pyo3::class::PyMethodDefType] {
+        self.methods
+    }
+}
+impl pyo3::class::methods::PyMethodsImpl for MyClass {
+    type Methods = Pyo3MethodsInventoryForMyClass;
+}
+pyo3::inventory::collect!(Pyo3MethodsInventoryForMyClass);
+# let gil = Python::acquire_gil();
+# let py = gil.python();
+# let cls = py.get_type::<MyClass>();
+# pyo3::py_run!(py, cls, "assert cls.__name__ == 'MyClass'")
+```
+
 
 [`GILGuard`]: https://docs.rs/pyo3/latest/pyo3/struct.GILGuard.html
 [`PyGCProtocol`]: https://docs.rs/pyo3/latest/pyo3/class/gc/trait.PyGCProtocol.html

pyo3-derive-backend/src/pyclass.rs

@@ -208,11 +208,9 @@ fn parse_descriptors(item: &mut syn::Field) -> syn::Result<Vec<FnType>> {
     Ok(descs)
 }
 
-/// The orphan rule disallows using a generic inventory struct, so we create the whole boilerplate
-/// once per class
-fn impl_inventory(cls: &syn::Ident) -> TokenStream {
-    // Try to build a unique type that gives a hint about it's function when
-    // it comes up in error messages
+/// To allow multiple #[pymethods]/#[pyproto] block, we define inventory types.
+fn impl_methods_inventory(cls: &syn::Ident) -> TokenStream {
+    // Try to build a unique type for better error messages
     let name = format!("Pyo3MethodsInventoryFor{}", cls);
     let inventory_cls = syn::Ident::new(&name, Span::call_site());
 
@@ -221,15 +219,11 @@ fn impl_inventory(cls: &syn::Ident) -> TokenStream {
         pub struct #inventory_cls {
             methods: &'static [pyo3::class::PyMethodDefType],
         }
-
         impl pyo3::class::methods::PyMethodsInventory for #inventory_cls {
             fn new(methods: &'static [pyo3::class::PyMethodDefType]) -> Self {
-                Self {
-                    methods
-                }
+                Self { methods }
             }
-
-            fn get_methods(&self) -> &'static [pyo3::class::PyMethodDefType] {
+            fn get(&self) -> &'static [pyo3::class::PyMethodDefType] {
                 self.methods
             }
         }
@@ -345,7 +339,7 @@ fn impl_class(
         quote! {}
     };
 
-    let inventory_impl = impl_inventory(&cls);
+    let impl_inventory = impl_methods_inventory(&cls);
 
     let base = &attr.base;
     let flags = &attr.flags;
@@ -418,7 +412,7 @@ fn impl_class(
 
         #into_pyobject
 
-        #inventory_impl
+        #impl_inventory
 
         #extra
 

pyo3-derive-backend/src/pyproto.rs

@@ -36,7 +36,7 @@ pub fn build_py_proto(ast: &mut syn::ItemImpl) -> syn::Result<TokenStream> {
             ));
         };
 
-        let tokens = impl_proto_impl(&ast.self_ty, &mut ast.items, proto);
+        let tokens = impl_proto_impl(&ast.self_ty, &mut ast.items, proto)?;
 
         // attach lifetime
         let mut seg = path.segments.pop().unwrap().into_value();
@@ -57,53 +57,54 @@ fn impl_proto_impl(
     ty: &syn::Type,
     impls: &mut Vec<syn::ImplItem>,
     proto: &defs::Proto,
-) -> TokenStream {
-    let mut tokens = TokenStream::new();
+) -> syn::Result<TokenStream> {
+    let mut trait_impls = TokenStream::new();
     let mut py_methods = Vec::new();
 
     for iimpl in impls.iter_mut() {
         if let syn::ImplItem::Method(ref mut met) = iimpl {
             if let Some(m) = proto.get_proto(&met.sig.ident) {
-                impl_method_proto(ty, &mut met.sig, m).to_tokens(&mut tokens);
+                impl_method_proto(ty, &mut met.sig, m).to_tokens(&mut trait_impls);
             }
             if let Some(m) = proto.get_method(&met.sig.ident) {
                 let name = &met.sig.ident;
-                let proto: syn::Path = syn::parse_str(m.proto).unwrap();
-
-                let fn_spec = match FnSpec::parse(&met.sig, &mut met.attrs, false) {
-                    Ok(fn_spec) => fn_spec,
-                    Err(err) => return err.to_compile_error(),
-                };
-                let meth = pymethod::impl_proto_wrap(ty, &fn_spec);
+                let fn_spec = FnSpec::parse(&met.sig, &mut met.attrs, false)?;
+                let method = pymethod::impl_proto_wrap(ty, &fn_spec);
                 let coexist = if m.can_coexist {
+                    // We need METH_COEXIST here to prevent __add__  from overriding __radd__
                     quote!(pyo3::ffi::METH_COEXIST)
                 } else {
                     quote!(0)
                 };
+                // TODO(kngwyu): doc
                 py_methods.push(quote! {
-                    impl #proto for #ty
-                    {
-                        #[inline]
-                        fn #name() -> Option<pyo3::class::methods::PyMethodDef> {
-                            #meth
-
-                            Some(pyo3::class::PyMethodDef {
-                                ml_name: stringify!(#name),
-                                ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap),
-                                // We need METH_COEXIST here to prevent __add__  from overriding __radd__
-                                ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS | #coexist,
-                                ml_doc: ""
-                            })
+                    pyo3::class::PyMethodDefType::Method({
+                        #method
+                        pyo3::class::PyMethodDef {
+                            ml_name: stringify!(#name),
+                            ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap),
+                            ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS | #coexist,
+                            ml_doc: ""
                         }
-                    }
+                    })
                 });
             }
         }
     }
 
-    quote! {
-        #tokens
-
-        #(#py_methods)*
+    if py_methods.is_empty() {
+        return Ok(quote! { #trait_impls });
     }
+    let inventory_submission = quote! {
+        pyo3::inventory::submit! {
+            #![crate = pyo3] {
+                type ProtoInventory = <#ty as pyo3::class::methods::PyMethodsImpl>::Methods;
+                <ProtoInventory as pyo3::class::methods::PyMethodsInventory>::new(&[#(#py_methods),*])
+            }
+        }
+    };
+    Ok(quote! {
+        #trait_impls
+        #inventory_submission
+    })
 }

src/class/basic.rs

@@ -9,7 +9,6 @@
 //! [typeobj docs](https://docs.python.org/3/c-api/typeobj.html)
 
 use crate::callback::HashCallbackOutput;
-use crate::class::methods::PyMethodDef;
 use crate::{
     exceptions, ffi, FromPyObject, IntoPy, PyAny, PyCell, PyClass, PyErr, PyObject, PyResult,
 };
@@ -145,15 +144,11 @@ pub trait PyObjectRichcmpProtocol<'p>: PyObjectProtocol<'p> {
 
 #[doc(hidden)]
 pub trait PyObjectProtocolImpl {
-    fn methods() -> Vec<PyMethodDef>;
     fn tp_as_object(_type_object: &mut ffi::PyTypeObject);
     fn nb_bool_fn() -> Option<ffi::inquiry>;
 }
 
 impl<T> PyObjectProtocolImpl for T {
-    default fn methods() -> Vec<PyMethodDef> {
-        Vec::new()
-    }
     default fn tp_as_object(_type_object: &mut ffi::PyTypeObject) {}
     default fn nb_bool_fn() -> Option<ffi::inquiry> {
         None
@@ -164,20 +159,6 @@ impl<'p, T> PyObjectProtocolImpl for T
 where
     T: PyObjectProtocol<'p>,
 {
-    fn methods() -> Vec<PyMethodDef> {
-        let mut methods = Vec::new();
-
-        if let Some(def) = <Self as FormatProtocolImpl>::__format__() {
-            methods.push(def)
-        }
-        if let Some(def) = <Self as BytesProtocolImpl>::__bytes__() {
-            methods.push(def)
-        }
-        if let Some(def) = <Self as UnicodeProtocolImpl>::__unicode__() {
-            methods.push(def)
-        }
-        methods
-    }
     fn tp_as_object(type_object: &mut ffi::PyTypeObject) {
         type_object.tp_str = Self::tp_str();
         type_object.tp_repr = Self::tp_repr();
@@ -373,45 +354,6 @@ where
     }
 }
 
-#[doc(hidden)]
-pub trait FormatProtocolImpl {
-    fn __format__() -> Option<PyMethodDef>;
-}
-impl<'p, T> FormatProtocolImpl for T
-where
-    T: PyObjectProtocol<'p>,
-{
-    default fn __format__() -> Option<PyMethodDef> {
-        None
-    }
-}
-
-#[doc(hidden)]
-pub trait BytesProtocolImpl {
-    fn __bytes__() -> Option<PyMethodDef>;
-}
-impl<'p, T> BytesProtocolImpl for T
-where
-    T: PyObjectProtocol<'p>,
-{
-    default fn __bytes__() -> Option<PyMethodDef> {
-        None
-    }
-}
-
-#[doc(hidden)]
-pub trait UnicodeProtocolImpl {
-    fn __unicode__() -> Option<PyMethodDef>;
-}
-impl<'p, T> UnicodeProtocolImpl for T
-where
-    T: PyObjectProtocol<'p>,
-{
-    default fn __unicode__() -> Option<PyMethodDef> {
-        None
-    }
-}
-
 trait HashProtocolImpl {
     fn tp_hash() -> Option<ffi::hashfunc>;
 }