Merge #3203

3203: support ordering magic methods for `#[pyclass]` r=adamreichold a=davidhewitt

Closes #2089 

This adds `__lt__`, `__le__`, `__eq__`, `__ne__`, `__gt__`, and `__ge__` as per the Python implementations of what we call `__richcmp__`.

There's a UI test confirming that the user cannot implement split forms and `__richcmp__` simultaneously.

There's also a benchmark comparing implementing these split methods against using `__richcmp__`. I couldn't see a meaningful performance difference, so I'm tempted to deprecate `__richcmp__`, given that's not a magic method which exists in Python. Potentially we can provide options such as the opt-in `#[pyclass(eq, ord)]` to avoid boilerplate for people who don't want to implement six different methods.



Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>

Cargo.toml

@@ -123,6 +123,10 @@ harness = false
 name = "bench_call"
 harness = false
 
+[[bench]]
+name = "bench_comparisons"
+harness = false
+
 [[bench]]
 name = "bench_err"
 harness = false

benches/bench_comparisons.rs

@@ -0,0 +1,70 @@
+use criterion::{criterion_group, criterion_main, Bencher, Criterion};
+
+use pyo3::{prelude::*, pyclass::CompareOp, Python};
+
+#[pyclass]
+struct OrderedDunderMethods(i64);
+
+#[pymethods]
+impl OrderedDunderMethods {
+    fn __lt__(&self, other: &Self) -> bool {
+        self.0 < other.0
+    }
+
+    fn __le__(&self, other: &Self) -> bool {
+        self.0 <= other.0
+    }
+
+    fn __eq__(&self, other: &Self) -> bool {
+        self.0 == other.0
+    }
+
+    fn __ne__(&self, other: &Self) -> bool {
+        self.0 != other.0
+    }
+
+    fn __gt__(&self, other: &Self) -> bool {
+        self.0 > other.0
+    }
+
+    fn __ge__(&self, other: &Self) -> bool {
+        self.0 >= other.0
+    }
+}
+
+#[pyclass]
+#[derive(PartialEq, Eq, PartialOrd, Ord)]
+struct OrderedRichcmp(i64);
+
+#[pymethods]
+impl OrderedRichcmp {
+    fn __richcmp__(&self, other: &Self, op: CompareOp) -> bool {
+        op.matches(self.cmp(other))
+    }
+}
+
+fn bench_ordered_dunder_methods(b: &mut Bencher<'_>) {
+    Python::with_gil(|py| {
+        let obj1 = Py::new(py, OrderedDunderMethods(0)).unwrap().into_ref(py);
+        let obj2 = Py::new(py, OrderedDunderMethods(1)).unwrap().into_ref(py);
+
+        b.iter(|| obj2.gt(obj1).unwrap());
+    });
+}
+
+fn bench_ordered_richcmp(b: &mut Bencher<'_>) {
+    Python::with_gil(|py| {
+        let obj1 = Py::new(py, OrderedRichcmp(0)).unwrap().into_ref(py);
+        let obj2 = Py::new(py, OrderedRichcmp(1)).unwrap().into_ref(py);
+
+        b.iter(|| obj2.gt(obj1).unwrap());
+    });
+}
+
+fn criterion_benchmark(c: &mut Criterion) {
+    c.bench_function("ordered_dunder_methods", bench_ordered_dunder_methods);
+    c.bench_function("ordered_richcmp", bench_ordered_richcmp);
+}
+
+criterion_group!(benches, criterion_benchmark);
+criterion_main!(benches);

guide/src/class/protocols.md

@@ -58,11 +58,28 @@ given signatures should be interpreted as follows:
     ```
     </details>
 
+  - `__lt__(<self>, object) -> object`
+  - `__le__(<self>, object) -> object`
+  - `__eq__(<self>, object) -> object`
+  - `__ne__(<self>, object) -> object`
+  - `__gt__(<self>, object) -> object`
+  - `__ge__(<self>, object) -> object`
+
+    The implementations of Python's "rich comparison" operators `<`, `<=`, `==`, `!=`, `>` and `>=` respectively.
+
+    _Note that implementing any of these methods will cause Python not to generate a default `__hash__` implementation, so consider also implementing `__hash__`._
+    <details>
+    <summary>Return type</summary>
+    The return type will normally be `bool` or `PyResult<bool>`, however any Python object can be returned.
+    </details>
+
   - `__richcmp__(<self>, object, pyo3::basic::CompareOp) -> object`
 
-    Overloads Python comparison operations (`==`, `!=`, `<`, `<=`, `>`, and `>=`).
+    Implements Python comparison operations (`==`, `!=`, `<`, `<=`, `>`, and `>=`) in a single method.
     The `CompareOp` argument indicates the comparison operation being performed.
 
+    _This method cannot be implemented in combination with any of `__lt__`, `__le__`, `__eq__`, `__ne__`, `__gt__`, or `__ge__`._
+
     _Note that implementing `__richcmp__` will cause Python not to generate a default `__hash__` implementation, so consider implementing `__hash__` when implementing `__richcmp__`._
     <details>
     <summary>Return type</summary>

newsfragments/3203.added.md

@@ -0,0 +1 @@
+Support `__lt__`, `__le__`, `__eq__`, `__ne__`, `__gt__` and `__ge__` in `#[pymethods]`

pyo3-macros-backend/src/pyimpl.rs

@@ -233,41 +233,50 @@ fn add_shared_proto_slots(
     mut implemented_proto_fragments: HashSet<String>,
 ) {
     macro_rules! try_add_shared_slot {
-        ($first:literal, $second:literal, $slot:ident) => {{
+        ($slot:ident, $($fragments:literal),*) => {{
-            let first_implemented = implemented_proto_fragments.remove($first);
+            let mut implemented = false;
-            let second_implemented = implemented_proto_fragments.remove($second);
+            $(implemented |= implemented_proto_fragments.remove($fragments));*;
-            if first_implemented || second_implemented {
+            if implemented {
                 proto_impls.push(quote! { _pyo3::impl_::pyclass::$slot!(#ty) })
             }
         }};
     }
 
     try_add_shared_slot!(
+        generate_pyclass_getattro_slot,
         "__getattribute__",
-        "__getattr__",
+        "__getattr__"
-        generate_pyclass_getattro_slot
     );
-    try_add_shared_slot!("__setattr__", "__delattr__", generate_pyclass_setattr_slot);
+    try_add_shared_slot!(generate_pyclass_setattr_slot, "__setattr__", "__delattr__");
-    try_add_shared_slot!("__set__", "__delete__", generate_pyclass_setdescr_slot);
+    try_add_shared_slot!(generate_pyclass_setdescr_slot, "__set__", "__delete__");
-    try_add_shared_slot!("__setitem__", "__delitem__", generate_pyclass_setitem_slot);
+    try_add_shared_slot!(generate_pyclass_setitem_slot, "__setitem__", "__delitem__");
-    try_add_shared_slot!("__add__", "__radd__", generate_pyclass_add_slot);
+    try_add_shared_slot!(generate_pyclass_add_slot, "__add__", "__radd__");
-    try_add_shared_slot!("__sub__", "__rsub__", generate_pyclass_sub_slot);
+    try_add_shared_slot!(generate_pyclass_sub_slot, "__sub__", "__rsub__");
-    try_add_shared_slot!("__mul__", "__rmul__", generate_pyclass_mul_slot);
+    try_add_shared_slot!(generate_pyclass_mul_slot, "__mul__", "__rmul__");
-    try_add_shared_slot!("__mod__", "__rmod__", generate_pyclass_mod_slot);
+    try_add_shared_slot!(generate_pyclass_mod_slot, "__mod__", "__rmod__");
-    try_add_shared_slot!("__divmod__", "__rdivmod__", generate_pyclass_divmod_slot);
+    try_add_shared_slot!(generate_pyclass_divmod_slot, "__divmod__", "__rdivmod__");
-    try_add_shared_slot!("__lshift__", "__rlshift__", generate_pyclass_lshift_slot);
+    try_add_shared_slot!(generate_pyclass_lshift_slot, "__lshift__", "__rlshift__");
-    try_add_shared_slot!("__rshift__", "__rrshift__", generate_pyclass_rshift_slot);
+    try_add_shared_slot!(generate_pyclass_rshift_slot, "__rshift__", "__rrshift__");
-    try_add_shared_slot!("__and__", "__rand__", generate_pyclass_and_slot);
+    try_add_shared_slot!(generate_pyclass_and_slot, "__and__", "__rand__");
-    try_add_shared_slot!("__or__", "__ror__", generate_pyclass_or_slot);
+    try_add_shared_slot!(generate_pyclass_or_slot, "__or__", "__ror__");
-    try_add_shared_slot!("__xor__", "__rxor__", generate_pyclass_xor_slot);
+    try_add_shared_slot!(generate_pyclass_xor_slot, "__xor__", "__rxor__");
-    try_add_shared_slot!("__matmul__", "__rmatmul__", generate_pyclass_matmul_slot);
+    try_add_shared_slot!(generate_pyclass_matmul_slot, "__matmul__", "__rmatmul__");
-    try_add_shared_slot!("__truediv__", "__rtruediv__", generate_pyclass_truediv_slot);
+    try_add_shared_slot!(generate_pyclass_truediv_slot, "__truediv__", "__rtruediv__");
     try_add_shared_slot!(
+        generate_pyclass_floordiv_slot,
         "__floordiv__",
-        "__rfloordiv__",
+        "__rfloordiv__"
-        generate_pyclass_floordiv_slot
+    );
+    try_add_shared_slot!(generate_pyclass_pow_slot, "__pow__", "__rpow__");
+    try_add_shared_slot!(
+        generate_pyclass_richcompare_slot,
+        "__lt__",
+        "__le__",
+        "__eq__",
+        "__ne__",
+        "__gt__",
+        "__ge__"
     );
-    try_add_shared_slot!("__pow__", "__rpow__", generate_pyclass_pow_slot);
 
     // if this assertion trips, a slot fragment has been implemented which has not been added in the
     // list above

pyo3-macros-backend/src/pymethod.rs

@@ -133,6 +133,12 @@ impl PyMethodKind {
             "__ror__" => PyMethodKind::Proto(PyMethodProtoKind::SlotFragment(&__ROR__)),
             "__pow__" => PyMethodKind::Proto(PyMethodProtoKind::SlotFragment(&__POW__)),
             "__rpow__" => PyMethodKind::Proto(PyMethodProtoKind::SlotFragment(&__RPOW__)),
+            "__lt__" => PyMethodKind::Proto(PyMethodProtoKind::SlotFragment(&__LT__)),
+            "__le__" => PyMethodKind::Proto(PyMethodProtoKind::SlotFragment(&__LE__)),
+            "__eq__" => PyMethodKind::Proto(PyMethodProtoKind::SlotFragment(&__EQ__)),
+            "__ne__" => PyMethodKind::Proto(PyMethodProtoKind::SlotFragment(&__NE__)),
+            "__gt__" => PyMethodKind::Proto(PyMethodProtoKind::SlotFragment(&__GT__)),
+            "__ge__" => PyMethodKind::Proto(PyMethodProtoKind::SlotFragment(&__GE__)),
             // Some tricky protocols which don't fit the pattern of the rest
             "__call__" => PyMethodKind::Proto(PyMethodProtoKind::Call),
             "__traverse__" => PyMethodKind::Proto(PyMethodProtoKind::Traverse),
@@ -1298,6 +1304,25 @@ const __RPOW__: SlotFragmentDef = SlotFragmentDef::new("__rpow__", &[Ty::Object,
     .extract_error_mode(ExtractErrorMode::NotImplemented)
     .ret_ty(Ty::Object);
 
+const __LT__: SlotFragmentDef = SlotFragmentDef::new("__lt__", &[Ty::Object])
+    .extract_error_mode(ExtractErrorMode::NotImplemented)
+    .ret_ty(Ty::Object);
+const __LE__: SlotFragmentDef = SlotFragmentDef::new("__le__", &[Ty::Object])
+    .extract_error_mode(ExtractErrorMode::NotImplemented)
+    .ret_ty(Ty::Object);
+const __EQ__: SlotFragmentDef = SlotFragmentDef::new("__eq__", &[Ty::Object])
+    .extract_error_mode(ExtractErrorMode::NotImplemented)
+    .ret_ty(Ty::Object);
+const __NE__: SlotFragmentDef = SlotFragmentDef::new("__ne__", &[Ty::Object])
+    .extract_error_mode(ExtractErrorMode::NotImplemented)
+    .ret_ty(Ty::Object);
+const __GT__: SlotFragmentDef = SlotFragmentDef::new("__gt__", &[Ty::Object])
+    .extract_error_mode(ExtractErrorMode::NotImplemented)
+    .ret_ty(Ty::Object);
+const __GE__: SlotFragmentDef = SlotFragmentDef::new("__ge__", &[Ty::Object])
+    .extract_error_mode(ExtractErrorMode::NotImplemented)
+    .ret_ty(Ty::Object);
+
 fn extract_proto_arguments(
     py: &syn::Ident,
     spec: &FnSpec<'_>,

pytests/requirements-dev.txt

@@ -2,3 +2,4 @@ hypothesis>=3.55
 pytest>=6.0
 pytest-benchmark>=3.4
 psutil>=5.6
+typing_extensions>=4.0.0

pytests/src/comparisons.rs

@@ -0,0 +1,111 @@
+use pyo3::prelude::*;
+use pyo3::{types::PyModule, Python};
+
+#[pyclass]
+struct Eq(i64);
+
+#[pymethods]
+impl Eq {
+    #[new]
+    fn new(value: i64) -> Self {
+        Self(value)
+    }
+
+    fn __eq__(&self, other: &Self) -> bool {
+        self.0 == other.0
+    }
+
+    fn __ne__(&self, other: &Self) -> bool {
+        self.0 != other.0
+    }
+}
+
+#[pyclass]
+struct EqDefaultNe(i64);
+
+#[pymethods]
+impl EqDefaultNe {
+    #[new]
+    fn new(value: i64) -> Self {
+        Self(value)
+    }
+
+    fn __eq__(&self, other: &Self) -> bool {
+        self.0 == other.0
+    }
+}
+
+#[pyclass]
+struct Ordered(i64);
+
+#[pymethods]
+impl Ordered {
+    #[new]
+    fn new(value: i64) -> Self {
+        Self(value)
+    }
+
+    fn __lt__(&self, other: &Self) -> bool {
+        self.0 < other.0
+    }
+
+    fn __le__(&self, other: &Self) -> bool {
+        self.0 <= other.0
+    }
+
+    fn __eq__(&self, other: &Self) -> bool {
+        self.0 == other.0
+    }
+
+    fn __ne__(&self, other: &Self) -> bool {
+        self.0 != other.0
+    }
+
+    fn __gt__(&self, other: &Self) -> bool {
+        self.0 > other.0
+    }
+
+    fn __ge__(&self, other: &Self) -> bool {
+        self.0 >= other.0
+    }
+}
+
+#[pyclass]
+struct OrderedDefaultNe(i64);
+
+#[pymethods]
+impl OrderedDefaultNe {
+    #[new]
+    fn new(value: i64) -> Self {
+        Self(value)
+    }
+
+    fn __lt__(&self, other: &Self) -> bool {
+        self.0 < other.0
+    }
+
+    fn __le__(&self, other: &Self) -> bool {
+        self.0 <= other.0
+    }
+
+    fn __eq__(&self, other: &Self) -> bool {
+        self.0 == other.0
+    }
+
+    fn __gt__(&self, other: &Self) -> bool {
+        self.0 > other.0
+    }
+
+    fn __ge__(&self, other: &Self) -> bool {
+        self.0 >= other.0
+    }
+}
+
+#[pymodule]
+pub fn comparisons(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
+    m.add_class::<Eq>()?;
+    m.add_class::<EqDefaultNe>()?;
+    m.add_class::<Ordered>()?;
+    m.add_class::<OrderedDefaultNe>()?;
+    Ok(())
+}

pytests/src/lib.rs

@@ -3,6 +3,7 @@ use pyo3::types::PyDict;
 use pyo3::wrap_pymodule;
 
 pub mod buf_and_str;
+pub mod comparisons;
 pub mod datetime;
 pub mod deprecated_pyfunctions;
 pub mod dict_iter;
@@ -19,6 +20,7 @@ pub mod subclassing;
 fn pyo3_pytests(py: Python<'_>, m: &PyModule) -> PyResult<()> {
     #[cfg(not(Py_LIMITED_API))]
     m.add_wrapped(wrap_pymodule!(buf_and_str::buf_and_str))?;
+    m.add_wrapped(wrap_pymodule!(comparisons::comparisons))?;
     #[cfg(not(Py_LIMITED_API))]
     m.add_wrapped(wrap_pymodule!(datetime::datetime))?;
     m.add_wrapped(wrap_pymodule!(
@@ -40,6 +42,7 @@ fn pyo3_pytests(py: Python<'_>, m: &PyModule) -> PyResult<()> {
     let sys = PyModule::import(py, "sys")?;
     let sys_modules: &PyDict = sys.getattr("modules")?.downcast()?;
     sys_modules.set_item("pyo3_pytests.buf_and_str", m.getattr("buf_and_str")?)?;
+    sys_modules.set_item("pyo3_pytests.comparisons", m.getattr("comparisons")?)?;
     sys_modules.set_item("pyo3_pytests.datetime", m.getattr("datetime")?)?;
     sys_modules.set_item(
         "pyo3_pytests.deprecated_pyfunctions",

pytests/tests/test_comparisons.py

@@ -0,0 +1,171 @@
+from typing import Type, Union
+
+import pytest
+from pyo3_pytests.comparisons import Eq, EqDefaultNe, Ordered, OrderedDefaultNe
+from typing_extensions import Self
+
+
+class PyEq:
+    def __init__(self, x: int) -> None:
+        self.x = x
+
+    def __eq__(self, other: Self) -> bool:
+        return self.x == other.x
+
+    def __ne__(self, other: Self) -> bool:
+        return self.x != other.x
+
+
+@pytest.mark.parametrize("ty", (Eq, PyEq), ids=("rust", "python"))
+def test_eq(ty: Type[Union[Eq, PyEq]]):
+    a = ty(0)
+    b = ty(0)
+    c = ty(1)
+
+    assert a == b
+    assert a != c
+
+    assert b == a
+    assert b != c
+
+    with pytest.raises(TypeError):
+        assert a <= b
+
+    with pytest.raises(TypeError):
+        assert a >= b
+
+    with pytest.raises(TypeError):
+        assert a < c
+
+    with pytest.raises(TypeError):
+        assert c > a
+
+
+class PyEqDefaultNe:
+    def __init__(self, x: int) -> None:
+        self.x = x
+
+    def __eq__(self, other: Self) -> bool:
+        return self.x == other.x
+
+
+@pytest.mark.parametrize("ty", (Eq, PyEq), ids=("rust", "python"))
+def test_eq_default_ne(ty: Type[Union[EqDefaultNe, PyEqDefaultNe]]):
+    a = ty(0)
+    b = ty(0)
+    c = ty(1)
+
+    assert a == b
+    assert a != c
+
+    assert b == a
+    assert b != c
+
+    with pytest.raises(TypeError):
+        assert a <= b
+
+    with pytest.raises(TypeError):
+        assert a >= b
+
+    with pytest.raises(TypeError):
+        assert a < c
+
+    with pytest.raises(TypeError):
+        assert c > a
+
+
+class PyOrdered:
+    def __init__(self, x: int) -> None:
+        self.x = x
+
+    def __lt__(self, other: Self) -> bool:
+        return self.x < other.x
+
+    def __le__(self, other: Self) -> bool:
+        return self.x <= other.x
+
+    def __eq__(self, other: Self) -> bool:
+        return self.x == other.x
+
+    def __ne__(self, other: Self) -> bool:
+        return self.x != other.x
+
+    def __gt__(self, other: Self) -> bool:
+        return self.x >= other.x
+
+    def __ge__(self, other: Self) -> bool:
+        return self.x >= other.x
+
+
+@pytest.mark.parametrize("ty", (Ordered, PyOrdered), ids=("rust", "python"))
+def test_ordered(ty: Type[Union[Ordered, PyOrdered]]):
+    a = ty(0)
+    b = ty(0)
+    c = ty(1)
+
+    assert a == b
+    assert a <= b
+    assert a >= b
+    assert a != c
+    assert a <= c
+
+    assert b == a
+    assert b <= a
+    assert b >= a
+    assert b != c
+    assert b <= c
+
+    assert c != a
+    assert c != b
+    assert c > a
+    assert c >= a
+    assert c > b
+    assert c >= b
+
+
+class PyOrderedDefaultNe:
+    def __init__(self, x: int) -> None:
+        self.x = x
+
+    def __lt__(self, other: Self) -> bool:
+        return self.x < other.x
+
+    def __le__(self, other: Self) -> bool:
+        return self.x <= other.x
+
+    def __eq__(self, other: Self) -> bool:
+        return self.x == other.x
+
+    def __gt__(self, other: Self) -> bool:
+        return self.x >= other.x
+
+    def __ge__(self, other: Self) -> bool:
+        return self.x >= other.x
+
+
+@pytest.mark.parametrize(
+    "ty", (OrderedDefaultNe, PyOrderedDefaultNe), ids=("rust", "python")
+)
+def test_ordered_default_ne(ty: Type[Union[OrderedDefaultNe, PyOrderedDefaultNe]]):
+    a = ty(0)
+    b = ty(0)
+    c = ty(1)
+
+    assert a == b
+    assert a <= b
+    assert a >= b
+    assert a != c
+    assert a <= c
+
+    assert b == a
+    assert b <= a
+    assert b >= a
+    assert b != c
+    assert b <= c
+
+    assert c != a
+    assert c != b
+    assert c > a
+    assert c >= a
+    assert c > b
+    assert c >= b

src/impl_/pyclass.rs

@@ -757,6 +757,130 @@ macro_rules! generate_pyclass_pow_slot {
 }
 pub use generate_pyclass_pow_slot;
 
+slot_fragment_trait! {
+    PyClass__lt__SlotFragment,
+
+    /// # Safety: _slf and _other must be valid non-null Python objects
+    #[inline]
+    unsafe fn __lt__(
+        self,
+        _py: Python<'_>,
+        _slf: *mut ffi::PyObject,
+        _other: *mut ffi::PyObject,
+    ) -> PyResult<*mut ffi::PyObject> {
+        Ok(ffi::_Py_NewRef(ffi::Py_NotImplemented()))
+    }
+}
+
+slot_fragment_trait! {
+    PyClass__le__SlotFragment,
+
+    /// # Safety: _slf and _other must be valid non-null Python objects
+    #[inline]
+    unsafe fn __le__(
+        self,
+        _py: Python<'_>,
+        _slf: *mut ffi::PyObject,
+        _other: *mut ffi::PyObject,
+    ) -> PyResult<*mut ffi::PyObject> {
+        Ok(ffi::_Py_NewRef(ffi::Py_NotImplemented()))
+    }
+}
+
+slot_fragment_trait! {
+    PyClass__eq__SlotFragment,
+
+    /// # Safety: _slf and _other must be valid non-null Python objects
+    #[inline]
+    unsafe fn __eq__(
+        self,
+        _py: Python<'_>,
+        _slf: *mut ffi::PyObject,
+        _other: *mut ffi::PyObject,
+    ) -> PyResult<*mut ffi::PyObject> {
+        Ok(ffi::_Py_NewRef(ffi::Py_NotImplemented()))
+    }
+}
+
+slot_fragment_trait! {
+    PyClass__ne__SlotFragment,
+
+    /// # Safety: _slf and _other must be valid non-null Python objects
+    #[inline]
+    unsafe fn __ne__(
+        self,
+        _py: Python<'_>,
+        _slf: *mut ffi::PyObject,
+        _other: *mut ffi::PyObject,
+    ) -> PyResult<*mut ffi::PyObject> {
+        Ok(ffi::_Py_NewRef(ffi::Py_NotImplemented()))
+    }
+}
+
+slot_fragment_trait! {
+    PyClass__gt__SlotFragment,
+
+    /// # Safety: _slf and _other must be valid non-null Python objects
+    #[inline]
+    unsafe fn __gt__(
+        self,
+        _py: Python<'_>,
+        _slf: *mut ffi::PyObject,
+        _other: *mut ffi::PyObject,
+    ) -> PyResult<*mut ffi::PyObject> {
+        Ok(ffi::_Py_NewRef(ffi::Py_NotImplemented()))
+    }
+}
+
+slot_fragment_trait! {
+    PyClass__ge__SlotFragment,
+
+    /// # Safety: _slf and _other must be valid non-null Python objects
+    #[inline]
+    unsafe fn __ge__(
+        self,
+        _py: Python<'_>,
+        _slf: *mut ffi::PyObject,
+        _other: *mut ffi::PyObject,
+    ) -> PyResult<*mut ffi::PyObject> {
+        Ok(ffi::_Py_NewRef(ffi::Py_NotImplemented()))
+    }
+}
+
+#[doc(hidden)]
+#[macro_export]
+macro_rules! generate_pyclass_richcompare_slot {
+    ($cls:ty) => {{
+        impl $cls {
+            #[allow(non_snake_case)]
+            unsafe extern "C" fn __pymethod___richcmp____(
+                slf: *mut $crate::ffi::PyObject,
+                other: *mut $crate::ffi::PyObject,
+                op: ::std::os::raw::c_int,
+            ) -> *mut $crate::ffi::PyObject {
+                $crate::impl_::trampoline::richcmpfunc(slf, other, op, |py, slf, other, op| {
+                    use $crate::class::basic::CompareOp;
+                    use $crate::impl_::pyclass::*;
+                    let collector = PyClassImplCollector::<$cls>::new();
+                    match CompareOp::from_raw(op).expect("invalid compareop") {
+                        CompareOp::Lt => collector.__lt__(py, slf, other),
+                        CompareOp::Le => collector.__le__(py, slf, other),
+                        CompareOp::Eq => collector.__eq__(py, slf, other),
+                        CompareOp::Ne => collector.__ne__(py, slf, other),
+                        CompareOp::Gt => collector.__gt__(py, slf, other),
+                        CompareOp::Ge => collector.__ge__(py, slf, other),
+                    }
+                })
+            }
+        }
+        $crate::ffi::PyType_Slot {
+            slot: $crate::ffi::Py_tp_richcompare,
+            pfunc: <$cls>::__pymethod___richcmp____ as $crate::ffi::richcmpfunc as _,
+        }
+    }};
+}
+pub use generate_pyclass_richcompare_slot;
+
 /// Implements a freelist.
 ///
 /// Do not implement this trait manually. Instead, use `#[pyclass(freelist = N)]`

src/test_hygiene/pymethods.rs

@@ -398,13 +398,6 @@ impl Dummy {
 
     // Dunder methods invented for protocols
 
-    fn __richcmp__(
-        &self,
-        other: &Self,
-        op: crate::class::basic::CompareOp,
-    ) -> crate::PyResult<bool> {
-        ::std::result::Result::Ok(false)
-    }
     // PyGcProtocol
     // Buffer protocol?
 }
@@ -797,13 +790,6 @@ impl Dummy {
 
     // Dunder methods invented for protocols
 
-    fn __richcmp__(
-        &self,
-        other: &Self,
-        op: crate::class::basic::CompareOp,
-    ) -> crate::PyResult<bool> {
-        ::std::result::Result::Ok(false)
-    }
     // PyGcProtocol
     // Buffer protocol?
 }

tests/ui/invalid_proto_pymethods.rs

@@ -4,6 +4,7 @@
 //! so that the function names can describe the edge case to be rejected.
 
 use pyo3::prelude::*;
+use pyo3::pyclass::CompareOp;
 
 #[pyclass]
 struct MyClass {}
@@ -48,4 +49,18 @@ impl MyClass {
     }
 }
 
+#[pyclass]
+struct EqAndRichcmp;
+
+#[pymethods]
+impl EqAndRichcmp {
+    fn __eq__(&self, other: &Self) -> bool {
+        true
+    }
+
+    fn __richcmp__(&self, other: &Self, op: CompareOp) -> bool {
+        true
+    }
+}
+
 fn main() {}

tests/ui/invalid_proto_pymethods.stderr

@@ -1,23 +1,34 @@
 error: Expected 1 arguments, got 0
-  --> tests/ui/invalid_proto_pymethods.rs:18:8
+  --> tests/ui/invalid_proto_pymethods.rs:19:8
    |
-18 |     fn truediv_expects_one_argument(&self) -> PyResult<()> {
+19 |     fn truediv_expects_one_argument(&self) -> PyResult<()> {
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: Expected 1 arguments, got 0
-  --> tests/ui/invalid_proto_pymethods.rs:26:8
+  --> tests/ui/invalid_proto_pymethods.rs:27:8
    |
-26 |     fn truediv_expects_one_argument_py(&self, _py: Python<'_>) -> PyResult<()> {
+27 |     fn truediv_expects_one_argument_py(&self, _py: Python<'_>) -> PyResult<()> {
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: `signature` cannot be used with magic method `__bool__`
-  --> tests/ui/invalid_proto_pymethods.rs:37:31
+  --> tests/ui/invalid_proto_pymethods.rs:38:31
    |
-37 |     #[pyo3(name = "__bool__", signature = ())]
+38 |     #[pyo3(name = "__bool__", signature = ())]
    |                               ^^^^^^^^^
 
 error: `text_signature` cannot be used with magic method `__bool__`
-  --> tests/ui/invalid_proto_pymethods.rs:45:31
+  --> tests/ui/invalid_proto_pymethods.rs:46:31
    |
-45 |     #[pyo3(name = "__bool__", text_signature = "")]
+46 |     #[pyo3(name = "__bool__", text_signature = "")]
    |                               ^^^^^^^^^^^^^^
+
+error[E0592]: duplicate definitions with name `__pymethod___richcmp____`
+  --> tests/ui/invalid_proto_pymethods.rs:55:1
+   |
+55 | #[pymethods]
+   | ^^^^^^^^^^^^
+   | |
+   | duplicate definitions for `__pymethod___richcmp____`
+   | other definition for `__pymethod___richcmp____`
+   |
+   = note: this error originates in the macro `_pyo3::impl_::pyclass::generate_pyclass_richcompare_slot` which comes from the expansion of the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)

tests/ui/pyclass_send.stderr

@@ -1,18 +1,18 @@
 error[E0277]: `Rc<i32>` cannot be sent between threads safely
-   --> tests/ui/pyclass_send.rs:4:1
+ --> tests/ui/pyclass_send.rs:4:1
-    |
+  |
-4   | #[pyclass]
+4 | #[pyclass]
-    | ^^^^^^^^^^ `Rc<i32>` cannot be sent between threads safely
+  | ^^^^^^^^^^ `Rc<i32>` cannot be sent between threads safely
-    |
+  |
-    = help: within `NotThreadSafe`, the trait `Send` is not implemented for `Rc<i32>`
+  = help: within `NotThreadSafe`, the trait `Send` is not implemented for `Rc<i32>`
 note: required because it appears within the type `NotThreadSafe`
-   --> tests/ui/pyclass_send.rs:5:8
+ --> tests/ui/pyclass_send.rs:5:8
-    |
+  |
-5   | struct NotThreadSafe {
+5 | struct NotThreadSafe {
-    |        ^^^^^^^^^^^^^
+  |        ^^^^^^^^^^^^^
 note: required by a bound in `ThreadCheckerStub`
-   --> src/impl_/pyclass.rs
+ --> src/impl_/pyclass.rs
-    |
+  |
-    | pub struct ThreadCheckerStub<T: Send>(PhantomData<T>);
+  | pub struct ThreadCheckerStub<T: Send>(PhantomData<T>);
-    |                                 ^^^^ required by this bound in `ThreadCheckerStub`
+  |                                 ^^^^ 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)
+  = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info)