macros-backend: support noargs for pyproto py_methods

CHANGELOG.md

@@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Fix FFI definition `PyIndex_Check` missing with the `abi3` feature. [#1436](https://github.com/PyO3/pyo3/pull/1436)
 - Fix incorrect `TypeError` raised when keyword-only argument passed along with a positional argument in `*args`. [#1440](https://github.com/PyO3/pyo3/pull/1440)
 - Fix inability to use a named lifetime for `&PyTuple` of `*args` in `#[pyfunction]`. [#1440](https://github.com/PyO3/pyo3/pull/1440)
+- Fix inability to add `#[text_signature]` to some `#[pyproto]` methods. [#1483](https://github.com/PyO3/pyo3/pull/1483)
 
 ## [0.13.2] - 2021-02-12
 ### Packaging

pyo3-macros-backend/src/module.rs

@@ -208,7 +208,6 @@ pub fn add_fn_to_module(
                 pyo3::class::methods::PyMethodDef::cfunction_with_keywords(
                     name,
                     pyo3::class::methods::PyCFunctionWithKeywords(#wrapper_ident),
-                    0,
                     #doc,
                 ),
                 args.into(),

pyo3-macros-backend/src/pymethod.rs

@@ -27,7 +27,9 @@ pub fn gen_py_method(
     let spec = FnSpec::parse(sig, &mut *meth_attrs, true)?;
 
     Ok(match &spec.tp {
-        FnType::Fn(self_ty) => GeneratedPyMethod::Method(impl_py_method_def(cls, &spec, self_ty)?),
+        FnType::Fn(self_ty) => {
+            GeneratedPyMethod::Method(impl_py_method_def(cls, &spec, self_ty, None)?)
+        }
         FnType::FnNew => GeneratedPyMethod::New(impl_py_method_def_new(cls, &spec)?),
         FnType::FnCall(self_ty) => {
             GeneratedPyMethod::Call(impl_py_method_def_call(cls, &spec, self_ty)?)
@@ -83,72 +85,24 @@ pub fn gen_py_const(
     Ok(None)
 }
 
-/// Generate function wrapper (PyCFunction, PyCFunctionWithKeywords)
+/// Generate function wrapper for PyCFunctionWithKeywords
-pub fn impl_wrap(
+pub fn impl_wrap_cfunction_with_keywords(
     cls: &syn::Type,
     spec: &FnSpec<'_>,
     self_ty: &SelfType,
-    noargs: bool,
 ) -> Result<TokenStream> {
     let body = impl_call(cls, &spec);
     let slf = self_ty.receiver(cls);
     let python_name = &spec.python_name;
-    if spec.args.is_empty() && noargs {
+    let body = impl_arg_params(&spec, Some(cls), body)?;
-        Ok(quote! {
-            unsafe extern "C" fn __wrap(
-                _slf: *mut pyo3::ffi::PyObject,
-                _args: *mut pyo3::ffi::PyObject,
-            ) -> *mut pyo3::ffi::PyObject
-            {
-                const _LOCATION: &'static str = concat!(
-                    stringify!(#cls), ".", stringify!(#python_name), "()");
-                pyo3::callback::handle_panic(|_py| {
-                    #slf
-                    #body
-                })
-            }
-        })
-    } else {
-        let body = impl_arg_params(&spec, Some(cls), body)?;
-        Ok(quote! {
-            unsafe extern "C" fn __wrap(
-                _slf: *mut pyo3::ffi::PyObject,
-                _args: *mut pyo3::ffi::PyObject,
-                _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject
-            {
-                const _LOCATION: &'static str = concat!(
-                    stringify!(#cls), ".", stringify!(#python_name), "()");
-                pyo3::callback::handle_panic(|_py| {
-                    #slf
-                    let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
-                    let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);
-
-                    #body
-                })
-            }
-        })
-    }
-}
-
-/// Generate function wrapper for protocol method (PyCFunction, PyCFunctionWithKeywords)
-pub fn impl_proto_wrap(
-    cls: &syn::Type,
-    spec: &FnSpec<'_>,
-    self_ty: &SelfType,
-) -> Result<TokenStream> {
-    let python_name = &spec.python_name;
-    let cb = impl_call(cls, &spec);
-    let body = impl_arg_params(&spec, Some(cls), cb)?;
-    let slf = self_ty.receiver(cls);
-
     Ok(quote! {
-        #[allow(unused_mut)]
         unsafe extern "C" fn __wrap(
             _slf: *mut pyo3::ffi::PyObject,
             _args: *mut pyo3::ffi::PyObject,
             _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject
         {
-            const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
+            const _LOCATION: &'static str = concat!(
+                stringify!(#cls), ".", stringify!(#python_name), "()");
             pyo3::callback::handle_panic(|_py| {
                 #slf
                 let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
@@ -160,6 +114,28 @@ pub fn impl_proto_wrap(
     })
 }
 
+/// Generate function wrapper PyCFunction
+pub fn impl_wrap_noargs(cls: &syn::Type, spec: &FnSpec<'_>, self_ty: &SelfType) -> TokenStream {
+    let body = impl_call(cls, &spec);
+    let slf = self_ty.receiver(cls);
+    let python_name = &spec.python_name;
+    assert!(spec.args.is_empty());
+    quote! {
+        unsafe extern "C" fn __wrap(
+            _slf: *mut pyo3::ffi::PyObject,
+            _args: *mut pyo3::ffi::PyObject,
+        ) -> *mut pyo3::ffi::PyObject
+        {
+            const _LOCATION: &'static str = concat!(
+                stringify!(#cls), ".", stringify!(#python_name), "()");
+            pyo3::callback::handle_panic(|_py| {
+                #slf
+                #body
+            })
+        }
+    }
+}
+
 /// Generate class method wrapper (PyCFunction, PyCFunctionWithKeywords)
 pub fn impl_wrap_new(cls: &syn::Type, spec: &FnSpec<'_>) -> Result<TokenStream> {
     let name = &spec.name;
@@ -591,23 +567,28 @@ pub fn impl_py_method_def(
     cls: &syn::Type,
     spec: &FnSpec,
     self_ty: &SelfType,
+    flags: Option<TokenStream>,
 ) -> Result<TokenStream> {
-    let wrapper = impl_wrap(cls, &spec, self_ty, true)?;
+    let add_flags = flags.map(|flags| quote!(.flags(#flags)));
     let python_name = &spec.python_name;
     let doc = &spec.doc;
     if spec.args.is_empty() {
+        let wrapper = impl_wrap_noargs(cls, spec, self_ty);
         Ok(quote! {
             pyo3::class::PyMethodDefType::Method({
                 #wrapper
 
-                pyo3::class::PyMethodDef::cfunction(
+                pyo3::class::PyMethodDef::noargs(
                     concat!(stringify!(#python_name), "\0"),
                     pyo3::class::methods::PyCFunction(__wrap),
                     #doc
                 )
+                #add_flags
+
             })
         })
     } else {
+        let wrapper = impl_wrap_cfunction_with_keywords(cls, &spec, self_ty)?;
         Ok(quote! {
             pyo3::class::PyMethodDefType::Method({
                 #wrapper
@@ -615,9 +596,9 @@ pub fn impl_py_method_def(
                 pyo3::class::PyMethodDef::cfunction_with_keywords(
                     concat!(stringify!(#python_name), "\0"),
                     pyo3::class::methods::PyCFunctionWithKeywords(__wrap),
-                    0,
                     #doc
                 )
+                #add_flags
             })
         })
     }
@@ -647,9 +628,8 @@ pub fn impl_py_method_def_class(cls: &syn::Type, spec: &FnSpec) -> Result<TokenS
             pyo3::class::PyMethodDef::cfunction_with_keywords(
                 concat!(stringify!(#python_name), "\0"),
                 pyo3::class::methods::PyCFunctionWithKeywords(__wrap),
-                pyo3::ffi::METH_CLASS,
                 #doc
-            )
+            ).flags(pyo3::ffi::METH_CLASS)
         })
     })
 }
@@ -665,9 +645,8 @@ pub fn impl_py_method_def_static(cls: &syn::Type, spec: &FnSpec) -> Result<Token
             pyo3::class::PyMethodDef::cfunction_with_keywords(
                 concat!(stringify!(#python_name), "\0"),
                 pyo3::class::methods::PyCFunctionWithKeywords(__wrap),
-                pyo3::ffi::METH_STATIC,
                 #doc
-            )
+            ).flags(pyo3::ffi::METH_STATIC)
         })
     })
 }
@@ -706,7 +685,7 @@ pub fn impl_py_method_def_call(
     spec: &FnSpec,
     self_ty: &SelfType,
 ) -> Result<TokenStream> {
-    let wrapper = impl_wrap(cls, &spec, self_ty, false)?;
+    let wrapper = impl_wrap_cfunction_with_keywords(cls, &spec, self_ty)?;
     Ok(quote! {
         impl pyo3::class::impl_::PyClassCallImpl<#cls> for pyo3::class::impl_::PyClassImplCollector<#cls> {
             fn call_impl(self) -> Option<pyo3::ffi::PyCFunctionWithKeywords> {

pyo3-macros-backend/src/pyproto.rs

@@ -64,33 +64,22 @@ fn impl_proto_impl(
             if let Some(m) = proto.get_method(&met.sig.ident) {
                 let fn_spec = FnSpec::parse(&mut met.sig, &mut met.attrs, false)?;
 
+                let flags = if m.can_coexist {
+                    // We need METH_COEXIST here to prevent __add__  from overriding __radd__
+                    Some(quote!(pyo3::ffi::METH_COEXIST))
+                } else {
+                    None
+                };
+
                 let method = if let FnType::Fn(self_ty) = &fn_spec.tp {
-                    pymethod::impl_proto_wrap(ty, &fn_spec, &self_ty)?
+                    pymethod::impl_py_method_def(ty, &fn_spec, &self_ty, flags)?
                 } else {
                     bail_spanned!(
                         met.sig.span() => "expected method with receiver for #[pyproto] method"
                     );
                 };
 
-                let coexist = if m.can_coexist {
+                py_methods.push(method);
-                    // We need METH_COEXIST here to prevent __add__  from overriding __radd__
-                    quote!(pyo3::ffi::METH_COEXIST)
-                } else {
-                    quote!(0)
-                };
-                let name = &met.sig.ident;
-                // TODO(kngwyu): Set ml_doc
-                py_methods.push(quote! {
-                    pyo3::class::PyMethodDef::cfunction_with_keywords(
-                        concat!(stringify!(#name), "\0"),
-                        {
-                            #method
-                            pyo3::class::methods::PyCFunctionWithKeywords(__wrap)
-                        },
-                        #coexist,
-                        "\0"
-                    )
-                });
             }
         }
     }
@@ -120,7 +109,7 @@ fn impl_normal_methods(
         {
             fn #methods_trait_methods(self) -> &'static [pyo3::class::methods::PyMethodDefType] {
                 static METHODS: &[pyo3::class::methods::PyMethodDefType] =
-                    &[#(pyo3::class::methods::PyMethodDefType::Method(#py_methods)),*];
+                    &[#(#py_methods),*];
                 METHODS
             }
         }

src/class/methods.rs

@@ -82,7 +82,7 @@ unsafe impl Sync for PySetterDef {}
 
 impl PyMethodDef {
     /// Define a function with no `*args` and `**kwargs`.
-    pub const fn cfunction(name: &'static str, cfunction: PyCFunction, doc: &'static str) -> Self {
+    pub const fn noargs(name: &'static str, cfunction: PyCFunction, doc: &'static str) -> Self {
         Self {
             ml_name: name,
             ml_meth: PyMethodType::PyCFunction(cfunction),
@@ -95,17 +95,21 @@ impl PyMethodDef {
     pub const fn cfunction_with_keywords(
         name: &'static str,
         cfunction: PyCFunctionWithKeywords,
-        flags: c_int,
         doc: &'static str,
     ) -> Self {
         Self {
             ml_name: name,
             ml_meth: PyMethodType::PyCFunctionWithKeywords(cfunction),
-            ml_flags: flags | ffi::METH_VARARGS | ffi::METH_KEYWORDS,
+            ml_flags: ffi::METH_VARARGS | ffi::METH_KEYWORDS,
             ml_doc: doc,
         }
     }
 
+    pub const fn flags(mut self, flags: c_int) -> Self {
+        self.ml_flags |= flags;
+        self
+    }
+
     /// Convert `PyMethodDef` to Python method definition struct `ffi::PyMethodDef`
     pub(crate) fn as_method_def(&self) -> Result<ffi::PyMethodDef, NulByteInString> {
         let meth = match self.ml_meth {

src/types/function.rs

@@ -23,12 +23,7 @@ impl PyCFunction {
         py_or_module: PyFunctionArguments<'a>,
     ) -> PyResult<&'a Self> {
         Self::internal_new(
-            PyMethodDef::cfunction_with_keywords(
+            PyMethodDef::cfunction_with_keywords(name, methods::PyCFunctionWithKeywords(fun), doc),
-                name,
-                methods::PyCFunctionWithKeywords(fun),
-                0,
-                doc,
-            ),
             py_or_module,
         )
     }
@@ -41,7 +36,7 @@ impl PyCFunction {
         py_or_module: PyFunctionArguments<'a>,
     ) -> PyResult<&'a Self> {
         Self::internal_new(
-            PyMethodDef::cfunction(name, methods::PyCFunction(fun), doc),
+            PyMethodDef::noargs(name, methods::PyCFunction(fun), doc),
             py_or_module,
         )
     }

tests/test_dunder.rs

@@ -109,6 +109,10 @@ fn string_methods() {
     py_assert!(py, obj, "repr(obj) == 'repr'");
     py_assert!(py, obj, "'{0:x}'.format(obj) == 'format(x)'");
     py_assert!(py, obj, "bytes(obj) == b'bytes'");
+
+    // Test that `__bytes__` takes no arguments (should be METH_NOARGS)
+    py_assert!(py, obj, "obj.__bytes__() == b'bytes'");
+    py_expect_exception!(py, obj, "obj.__bytes__('unexpected argument')", PyTypeError);
 }
 
 #[pyclass]