RichCompare contains pymethod. No AtomicCell for slots (#6562)

* slots: add comparison pymethods to Comparable trait

* No AtomicCell

crates/stdlib/src/contextvars.rs

@@ -264,7 +264,7 @@ mod _contextvars {
                         Err(vm.new_key_error(needle.to_owned().into()))
                     }
                 }),
-                ass_subscript: AtomicCell::new(None),
+                ass_subscript: None,
             };
             &AS_MAPPING
         }

crates/stdlib/src/sqlite.rs

@@ -20,7 +20,6 @@ pub(crate) fn make_module(vm: &VirtualMachine) -> PyRef<PyModule> {
 
 #[pymodule]
 mod _sqlite {
-    use crossbeam_utils::atomic::AtomicCell;
     use libsqlite3_sys::{
         SQLITE_BLOB, SQLITE_DETERMINISTIC, SQLITE_FLOAT, SQLITE_INTEGER, SQLITE_NULL,
         SQLITE_OPEN_CREATE, SQLITE_OPEN_READWRITE, SQLITE_OPEN_URI, SQLITE_TEXT, SQLITE_TRACE_STMT,
@@ -2549,19 +2548,19 @@ mod _sqlite {
     impl AsSequence for Blob {
         fn as_sequence() -> &'static PySequenceMethods {
             static AS_SEQUENCE: PySequenceMethods = PySequenceMethods {
-                length: AtomicCell::new(None),
-                concat: AtomicCell::new(None),
-                repeat: AtomicCell::new(None),
-                item: AtomicCell::new(None),
-                ass_item: AtomicCell::new(None),
+                length: None,
+                concat: None,
+                repeat: None,
+                item: None,
+                ass_item: None,
                 contains: atomic_func!(|seq, _needle, vm| {
                     Err(vm.new_type_error(format!(
                         "argument of type '{}' is not iterable",
                         seq.obj.class().name(),
                     )))
                 }),
-                inplace_concat: AtomicCell::new(None),
-                inplace_repeat: AtomicCell::new(None),
+                inplace_concat: None,
+                inplace_repeat: None,
             };
             &AS_SEQUENCE
         }

crates/vm/src/protocol/mapping.rs

@@ -35,13 +35,13 @@ impl PyMappingSlots {
 
     /// Copy from static PyMappingMethods
     pub fn copy_from(&self, methods: &PyMappingMethods) {
-        if let Some(f) = methods.length.load() {
+        if let Some(f) = methods.length {
             self.length.store(Some(f));
         }
-        if let Some(f) = methods.subscript.load() {
+        if let Some(f) = methods.subscript {
             self.subscript.store(Some(f));
         }
-        if let Some(f) = methods.ass_subscript.load() {
+        if let Some(f) = methods.ass_subscript {
             self.ass_subscript.store(Some(f));
         }
     }
@@ -50,11 +50,10 @@ impl PyMappingSlots {
 #[allow(clippy::type_complexity)]
 #[derive(Default)]
 pub struct PyMappingMethods {
-    pub length: AtomicCell<Option<fn(PyMapping<'_>, &VirtualMachine) -> PyResult<usize>>>,
-    pub subscript: AtomicCell<Option<fn(PyMapping<'_>, &PyObject, &VirtualMachine) -> PyResult>>,
-    pub ass_subscript: AtomicCell<
+    pub length: Option<fn(PyMapping<'_>, &VirtualMachine) -> PyResult<usize>>,
+    pub subscript: Option<fn(PyMapping<'_>, &PyObject, &VirtualMachine) -> PyResult>,
+    pub ass_subscript:
         Option<fn(PyMapping<'_>, &PyObject, Option<PyObjectRef>, &VirtualMachine) -> PyResult<()>>,
-    >,
 }
 
 impl std::fmt::Debug for PyMappingMethods {
@@ -64,11 +63,10 @@ impl std::fmt::Debug for PyMappingMethods {
 }
 
 impl PyMappingMethods {
-    #[allow(clippy::declare_interior_mutable_const)]
     pub const NOT_IMPLEMENTED: Self = Self {
-        length: AtomicCell::new(None),
-        subscript: AtomicCell::new(None),
-        ass_subscript: AtomicCell::new(None),
+        length: None,
+        subscript: None,
+        ass_subscript: None,
     };
 }
 

crates/vm/src/protocol/sequence.rs

@@ -42,28 +42,28 @@ impl PySequenceSlots {
 
     /// Copy from static PySequenceMethods
     pub fn copy_from(&self, methods: &PySequenceMethods) {
-        if let Some(f) = methods.length.load() {
+        if let Some(f) = methods.length {
             self.length.store(Some(f));
         }
-        if let Some(f) = methods.concat.load() {
+        if let Some(f) = methods.concat {
             self.concat.store(Some(f));
         }
-        if let Some(f) = methods.repeat.load() {
+        if let Some(f) = methods.repeat {
             self.repeat.store(Some(f));
         }
-        if let Some(f) = methods.item.load() {
+        if let Some(f) = methods.item {
             self.item.store(Some(f));
         }
-        if let Some(f) = methods.ass_item.load() {
+        if let Some(f) = methods.ass_item {
             self.ass_item.store(Some(f));
         }
-        if let Some(f) = methods.contains.load() {
+        if let Some(f) = methods.contains {
             self.contains.store(Some(f));
         }
-        if let Some(f) = methods.inplace_concat.load() {
+        if let Some(f) = methods.inplace_concat {
             self.inplace_concat.store(Some(f));
         }
-        if let Some(f) = methods.inplace_repeat.load() {
+        if let Some(f) = methods.inplace_repeat {
             self.inplace_repeat.store(Some(f));
         }
     }
@@ -72,18 +72,15 @@ impl PySequenceSlots {
 #[allow(clippy::type_complexity)]
 #[derive(Default)]
 pub struct PySequenceMethods {
-    pub length: AtomicCell<Option<fn(PySequence<'_>, &VirtualMachine) -> PyResult<usize>>>,
-    pub concat: AtomicCell<Option<fn(PySequence<'_>, &PyObject, &VirtualMachine) -> PyResult>>,
-    pub repeat: AtomicCell<Option<fn(PySequence<'_>, isize, &VirtualMachine) -> PyResult>>,
-    pub item: AtomicCell<Option<fn(PySequence<'_>, isize, &VirtualMachine) -> PyResult>>,
-    pub ass_item: AtomicCell<
+    pub length: Option<fn(PySequence<'_>, &VirtualMachine) -> PyResult<usize>>,
+    pub concat: Option<fn(PySequence<'_>, &PyObject, &VirtualMachine) -> PyResult>,
+    pub repeat: Option<fn(PySequence<'_>, isize, &VirtualMachine) -> PyResult>,
+    pub item: Option<fn(PySequence<'_>, isize, &VirtualMachine) -> PyResult>,
+    pub ass_item:
         Option<fn(PySequence<'_>, isize, Option<PyObjectRef>, &VirtualMachine) -> PyResult<()>>,
-    >,
-    pub contains:
-        AtomicCell<Option<fn(PySequence<'_>, &PyObject, &VirtualMachine) -> PyResult<bool>>>,
-    pub inplace_concat:
-        AtomicCell<Option<fn(PySequence<'_>, &PyObject, &VirtualMachine) -> PyResult>>,
-    pub inplace_repeat: AtomicCell<Option<fn(PySequence<'_>, isize, &VirtualMachine) -> PyResult>>,
+    pub contains: Option<fn(PySequence<'_>, &PyObject, &VirtualMachine) -> PyResult<bool>>,
+    pub inplace_concat: Option<fn(PySequence<'_>, &PyObject, &VirtualMachine) -> PyResult>,
+    pub inplace_repeat: Option<fn(PySequence<'_>, isize, &VirtualMachine) -> PyResult>,
 }
 
 impl std::fmt::Debug for PySequenceMethods {
@@ -93,16 +90,15 @@ impl std::fmt::Debug for PySequenceMethods {
 }
 
 impl PySequenceMethods {
-    #[allow(clippy::declare_interior_mutable_const)]
     pub const NOT_IMPLEMENTED: Self = Self {
-        length: AtomicCell::new(None),
-        concat: AtomicCell::new(None),
-        repeat: AtomicCell::new(None),
-        item: AtomicCell::new(None),
-        ass_item: AtomicCell::new(None),
-        contains: AtomicCell::new(None),
-        inplace_concat: AtomicCell::new(None),
-        inplace_repeat: AtomicCell::new(None),
+        length: None,
+        concat: None,
+        repeat: None,
+        item: None,
+        ass_item: None,
+        contains: None,
+        inplace_concat: None,
+        inplace_repeat: None,
     };
 }
 

crates/vm/src/types/slot.rs

@@ -113,7 +113,7 @@ impl<T: Any + 'static> std::ops::DerefMut for TypeDataRefMut<'_, T> {
 #[macro_export]
 macro_rules! atomic_func {
     ($x:expr) => {
-        crossbeam_utils::atomic::AtomicCell::new(Some($x))
+        Some($x)
     };
 }
 
@@ -385,6 +385,59 @@ fn setitem_wrapper<K: ToPyObject>(
     .map(drop)
 }
 
+#[inline(never)]
+fn mapping_setitem_wrapper(
+    mapping: PyMapping<'_>,
+    key: &PyObject,
+    value: Option<PyObjectRef>,
+    vm: &VirtualMachine,
+) -> PyResult<()> {
+    setitem_wrapper(mapping.obj, key, value, vm)
+}
+
+#[inline(never)]
+fn mapping_getitem_wrapper(
+    mapping: PyMapping<'_>,
+    key: &PyObject,
+    vm: &VirtualMachine,
+) -> PyResult {
+    getitem_wrapper(mapping.obj, key, vm)
+}
+
+#[inline(never)]
+fn mapping_len_wrapper(mapping: PyMapping<'_>, vm: &VirtualMachine) -> PyResult<usize> {
+    len_wrapper(mapping.obj, vm)
+}
+
+#[inline(never)]
+fn sequence_len_wrapper(seq: PySequence<'_>, vm: &VirtualMachine) -> PyResult<usize> {
+    len_wrapper(seq.obj, vm)
+}
+
+#[inline(never)]
+fn sequence_getitem_wrapper(seq: PySequence<'_>, i: isize, vm: &VirtualMachine) -> PyResult {
+    getitem_wrapper(seq.obj, i, vm)
+}
+
+#[inline(never)]
+fn sequence_setitem_wrapper(
+    seq: PySequence<'_>,
+    i: isize,
+    value: Option<PyObjectRef>,
+    vm: &VirtualMachine,
+) -> PyResult<()> {
+    setitem_wrapper(seq.obj, i, value, vm)
+}
+
+#[inline(never)]
+fn sequence_contains_wrapper(
+    seq: PySequence<'_>,
+    needle: &PyObject,
+    vm: &VirtualMachine,
+) -> PyResult<bool> {
+    contains_wrapper(seq.obj, needle, vm)
+}
+
 fn repr_wrapper(zelf: &PyObject, vm: &VirtualMachine) -> PyResult<PyRef<PyStr>> {
     let ret = vm.call_special_method(zelf, identifier!(vm, __repr__), ())?;
     ret.downcast::<PyStr>().map_err(|obj| {
@@ -1139,12 +1192,7 @@ impl PyType {
 
             // === Sequence slots ===
             SlotAccessor::SqLength => {
-                update_sub_slot!(
-                    as_sequence,
-                    length,
-                    |seq, vm| len_wrapper(seq.obj, vm),
-                    SeqLength
-                )
+                update_sub_slot!(as_sequence, length, sequence_len_wrapper, SeqLength)
             }
             SlotAccessor::SqConcat | SlotAccessor::SqInplaceConcat => {
                 // Sequence concat uses sq_concat slot - no generic wrapper needed
@@ -1161,52 +1209,32 @@ impl PyType {
                 }
             }
             SlotAccessor::SqItem => {
-                update_sub_slot!(
-                    as_sequence,
-                    item,
-                    |seq, i, vm| getitem_wrapper(seq.obj, i, vm),
-                    SeqItem
-                )
+                update_sub_slot!(as_sequence, item, sequence_getitem_wrapper, SeqItem)
             }
             SlotAccessor::SqAssItem => {
-                update_sub_slot!(
-                    as_sequence,
-                    ass_item,
-                    |seq, i, value, vm| setitem_wrapper(seq.obj, i, value, vm),
-                    SeqAssItem
-                )
+                update_sub_slot!(as_sequence, ass_item, sequence_setitem_wrapper, SeqAssItem)
             }
             SlotAccessor::SqContains => {
                 update_sub_slot!(
                     as_sequence,
                     contains,
-                    |seq, needle, vm| contains_wrapper(seq.obj, needle, vm),
+                    sequence_contains_wrapper,
                     SeqContains
                 )
             }
 
             // === Mapping slots ===
             SlotAccessor::MpLength => {
-                update_sub_slot!(
-                    as_mapping,
-                    length,
-                    |mapping, vm| len_wrapper(mapping.obj, vm),
-                    MapLength
-                )
+                update_sub_slot!(as_mapping, length, mapping_len_wrapper, MapLength)
             }
             SlotAccessor::MpSubscript => {
-                update_sub_slot!(
-                    as_mapping,
-                    subscript,
-                    |mapping, key, vm| getitem_wrapper(mapping.obj, key, vm),
-                    MapSubscript
-                )
+                update_sub_slot!(as_mapping, subscript, mapping_getitem_wrapper, MapSubscript)
             }
             SlotAccessor::MpAssSubscript => {
                 update_sub_slot!(
                     as_mapping,
                     ass_subscript,
-                    |mapping, key, value, vm| setitem_wrapper(mapping.obj, key, value, vm),
+                    mapping_setitem_wrapper,
                     MapAssSubscript
                 )
             }
@@ -1484,8 +1512,6 @@ pub trait Hashable: PyPayload {
         Self::hash(zelf, vm)
     }
 
-    // __hash__ is now exposed via SlotFunc::Hash wrapper in extend_class()
-
     fn hash(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyHash>;
 }
 
@@ -1535,8 +1561,60 @@ pub trait Comparable: PyPayload {
         vm: &VirtualMachine,
     ) -> PyResult<PyComparisonValue>;
 
-    // Comparison methods are exposed as wrapper_descriptor via slot_richcompare
-    // No #[pymethod] - add_operators creates wrapper from SLOT_DEFS
+    #[inline]
+    #[pymethod]
+    fn __eq__(
+        zelf: &Py<Self>,
+        other: PyObjectRef,
+        vm: &VirtualMachine,
+    ) -> PyResult<PyComparisonValue> {
+        Self::cmp(zelf, &other, PyComparisonOp::Eq, vm)
+    }
+    #[inline]
+    #[pymethod]
+    fn __ne__(
+        zelf: &Py<Self>,
+        other: PyObjectRef,
+        vm: &VirtualMachine,
+    ) -> PyResult<PyComparisonValue> {
+        Self::cmp(zelf, &other, PyComparisonOp::Ne, vm)
+    }
+    #[inline]
+    #[pymethod]
+    fn __lt__(
+        zelf: &Py<Self>,
+        other: PyObjectRef,
+        vm: &VirtualMachine,
+    ) -> PyResult<PyComparisonValue> {
+        Self::cmp(zelf, &other, PyComparisonOp::Lt, vm)
+    }
+    #[inline]
+    #[pymethod]
+    fn __le__(
+        zelf: &Py<Self>,
+        other: PyObjectRef,
+        vm: &VirtualMachine,
+    ) -> PyResult<PyComparisonValue> {
+        Self::cmp(zelf, &other, PyComparisonOp::Le, vm)
+    }
+    #[inline]
+    #[pymethod]
+    fn __ge__(
+        zelf: &Py<Self>,
+        other: PyObjectRef,
+        vm: &VirtualMachine,
+    ) -> PyResult<PyComparisonValue> {
+        Self::cmp(zelf, &other, PyComparisonOp::Ge, vm)
+    }
+    #[inline]
+    #[pymethod]
+    fn __gt__(
+        zelf: &Py<Self>,
+        other: PyObjectRef,
+        vm: &VirtualMachine,
+    ) -> PyResult<PyComparisonValue> {
+        Self::cmp(zelf, &other, PyComparisonOp::Gt, vm)
+    }
 }
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
@@ -1779,8 +1857,6 @@ pub trait Iterable: PyPayload {
         Self::iter(zelf, vm)
     }
 
-    // __iter__ is exposed via SlotFunc::Iter wrapper in extend_class()
-
     fn iter(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult;
 
     fn extend_slots(_slots: &mut PyTypeSlots) {}
@@ -1798,8 +1874,6 @@ pub trait IterNext: PyPayload + Iterable {
     }
 
     fn next(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyIterReturn>;
-
-    // __next__ is exposed via SlotFunc::IterNext wrapper in extend_class()
 }
 
 pub trait SelfIter: PyPayload {}
@@ -1814,8 +1888,6 @@ where
         unreachable!("slot must be overridden for {}", repr.as_str());
     }
 
-    // __iter__ is exposed via SlotFunc::Iter wrapper in extend_class()
-
     #[cold]
     fn iter(_zelf: PyRef<Self>, _vm: &VirtualMachine) -> PyResult {
         unreachable!("slot_iter is implemented");