Add manual Traverse impl for PySlice with clear() and fix comment

youknowone · youknowone · commit 5b78ad51c401 · 2026-03-04T17:49:20.000+09:00
diff --git a/crates/vm/src/builtins/slice.rs b/crates/vm/src/builtins/slice.rs
@@ -17,14 +17,34 @@ use crate::{
 use malachite_bigint::{BigInt, ToBigInt};
 use num_traits::{One, Signed, Zero};
 
-#[pyclass(module = false, name = "slice", unhashable = true, traverse)]
+#[pyclass(module = false, name = "slice", unhashable = true, traverse = "manual")]
 #[derive(Debug)]
 pub struct PySlice {
     pub start: Option<PyObjectRef>,
     pub stop: PyObjectRef,
     pub step: Option<PyObjectRef>,
 }
 
+// SAFETY: Traverse properly visits all owned PyObjectRefs
+unsafe impl crate::object::Traverse for PySlice {
+    fn traverse(&self, traverse_fn: &mut crate::object::TraverseFn<'_>) {
+        self.start.traverse(traverse_fn);
+        self.stop.traverse(traverse_fn);
+        self.step.traverse(traverse_fn);
+    }
+
+    fn clear(&mut self, out: &mut Vec<PyObjectRef>) {
+        if let Some(start) = self.start.take() {
+            out.push(start);
+        }
+        // stop is not Option, so it will be freed when payload is dropped
+        // (via drop_in_place on freelist pop, or Box::from_raw on dealloc)
+        if let Some(step) = self.step.take() {
+            out.push(step);
+        }
+    }
+}
+
 thread_local! {
     static SLICE_FREELIST: Cell<crate::object::FreeList<PySlice>> = const { Cell::new(crate::object::FreeList::new()) };
 }
diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs
@@ -1791,7 +1791,8 @@ impl<T: PyPayload + crate::object::MaybeTraverse + core::fmt::Debug> PyRef<T> {
                     (*inner).gc_bits.store(0, Ordering::Relaxed);
                     core::ptr::drop_in_place(&mut (*inner).payload);
                     core::ptr::write(&mut (*inner).payload, payload);
-                    // typ, vtable, dict(None), weak_list, slots are preserved
+                    // typ, vtable, slots are preserved; dict is None, weak_list was
+                    // cleared by drop_slow_inner before freelist push
                 }
                 // Drop the caller's typ since the cached object already holds one
                 drop(typ);
