From ac34a42ca927bac2a54cf4fdf64223a300eda98c Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 27 Sep 2026 09:38:36 +0900 Subject: [PATCH 1/4] Move PyList Python methods and operators to Py The #[pymethod]s, #[pyclassmethod] __class_getitem__ and the operators (__add__, which replaces concat(), __iadd__, __mul__, __imul__, __getitem__, __setitem__, __delitem__, __contains__, inplace_concat) are defined in a #[pyclass] impl of Py, registered through with(Py, ...). The #[pyclass] impl of PyList keeps the class attributes and is empty. PyList keeps the helpers that work on the payload (_getitem, _setitem, _delitem, repeat, irepeat and sort, which also sorts PyList values that have no object yet); Py reaches them through the payload field. The mapping and sequence slots call the Py operators. PyList::__len__() is removed; its callers read borrow_vec().len(). GenericAlias.__dir__ appends to the PyList returned by vm.dir() through borrow_vec_mut() and checks membership with mut_contains(). Assisted-by: Claude Code:claude-opus-5-5 --- crates/capi/src/listobject.rs | 2 +- crates/stdlib/src/_heapq.rs | 22 +-- crates/vm/src/builtins/genericalias.rs | 5 +- crates/vm/src/builtins/list.rs | 244 ++++++++++++------------- crates/vm/src/builtins/object.rs | 2 +- 5 files changed, 137 insertions(+), 138 deletions(-) diff --git a/crates/capi/src/listobject.rs b/crates/capi/src/listobject.rs index 208f2026bc4..ec31e184b4b 100644 --- a/crates/capi/src/listobject.rs +++ b/crates/capi/src/listobject.rs @@ -25,7 +25,7 @@ pub extern "C" fn PyList_New(size: isize) -> *mut PyObject { pub unsafe extern "C" fn PyList_Size(obj: *mut PyObject) -> isize { with_vm(|vm| { let list = unsafe { obj.assume_borrowed_and_cast::(vm) }?; - Ok(list.__len__()) + Ok(list.borrow_vec().len()) }) } diff --git a/crates/stdlib/src/_heapq.rs b/crates/stdlib/src/_heapq.rs index f1066e97608..a3dbf08959d 100644 --- a/crates/stdlib/src/_heapq.rs +++ b/crates/stdlib/src/_heapq.rs @@ -17,7 +17,7 @@ mod _heapq { mut pos: usize, vm: &VirtualMachine, ) -> PyResult<()> { - let size = heap.__len__(); + let size = heap.borrow_vec().len(); let newitem = match heap.borrow_vec().get(pos) { Some(v) => v.clone(), @@ -35,7 +35,7 @@ mod _heapq { let cmp = newitem.rich_compare_bool(&parent, PyComparisonOp::Lt, vm)?; - if size != heap.__len__() { + if size != heap.borrow_vec().len() { return Err(vm.new_runtime_error("list changed size during iteration")); } @@ -56,7 +56,7 @@ mod _heapq { /// [CPython's siftup](https://github.com/python/cpython/blob/v3.14.5/Modules/_heapqmodule.c#L70-L118) fn siftup(heap: &Py, mut pos: usize, vm: &VirtualMachine) -> PyResult<()> { - let endpos = heap.__len__(); + let endpos = heap.borrow_vec().len(); let startpos = pos; if pos >= endpos { @@ -78,7 +78,7 @@ mod _heapq { let cmp = a.rich_compare_bool(&b, PyComparisonOp::Lt, vm)?; - if endpos != heap.__len__() { + if endpos != heap.borrow_vec().len() { return Err(vm.new_runtime_error("list changed size during iteration")); } @@ -119,7 +119,7 @@ mod _heapq { vec.push(item); } - let size = heap.__len__(); + let size = heap.borrow_vec().len(); siftdown_func(heap, 0, size - 1, vm) } @@ -251,7 +251,7 @@ mod _heapq { where F: Fn(&Py, usize, &VirtualMachine) -> PyResult<()>, { - let m = heap.__len__() >> 1; // index of first childless node + let m = heap.borrow_vec().len() >> 1; // index of first childless node let leftmost = keep_top_bit(m + 1) - 1; // leftmost node in row of m let mhalf = m >> 1; // parent of first childless node @@ -291,7 +291,7 @@ mod _heapq { where F: Fn(&Py, usize, &VirtualMachine) -> PyResult<()>, { - let n = heap.__len__(); + let n = heap.borrow_vec().len(); if n > 2500 { return cache_friendly_heapify(heap, siftup_func, vm); @@ -316,7 +316,7 @@ mod _heapq { mut pos: usize, vm: &VirtualMachine, ) -> PyResult<()> { - let size = heap.__len__(); + let size = heap.borrow_vec().len(); let newitem = match heap.borrow_vec().get(pos) { Some(v) => v.clone(), @@ -334,7 +334,7 @@ mod _heapq { let cmp = parent.rich_compare_bool(&newitem, PyComparisonOp::Lt, vm)?; - if size != heap.__len__() { + if size != heap.borrow_vec().len() { return Err(vm.new_runtime_error("list changed size during iteration")); } @@ -355,7 +355,7 @@ mod _heapq { /// [CPython's siftup_max](https://github.com/python/cpython/blob/v3.14.5/Modules/_heapqmodule.c#L451-L499) fn siftup_max(heap: &Py, mut pos: usize, vm: &VirtualMachine) -> PyResult<()> { - let endpos = heap.__len__(); + let endpos = heap.borrow_vec().len(); let startpos = pos; if pos >= endpos { @@ -377,7 +377,7 @@ mod _heapq { let cmp = a.rich_compare_bool(&b, PyComparisonOp::Lt, vm)?; - if endpos != heap.__len__() { + if endpos != heap.borrow_vec().len() { return Err(vm.new_runtime_error("list changed size during iteration")); } diff --git a/crates/vm/src/builtins/genericalias.rs b/crates/vm/src/builtins/genericalias.rs index 46de0bec698..efd8e8c27b8 100644 --- a/crates/vm/src/builtins/genericalias.rs +++ b/crates/vm/src/builtins/genericalias.rs @@ -11,6 +11,7 @@ use crate::{ convert::ToPyObject, function::{FuncArgs, PyComparisonValue}, protocol::{PyMappingMethods, PyNumberMethods}, + sequence::MutObjectSequenceOp, types::{ AsMapping, AsNumber, Callable, Comparable, Constructor, GetAttr, Hashable, IterNext, Iterable, PyComparisonOp, Representable, @@ -244,8 +245,8 @@ impl PyGenericAlias { let dir = vm.dir(Some(self.origin.clone()))?; for exc in &ATTR_EXCEPTIONS { let exc_obj = (*exc).to_pyobject(vm); - if !dir.__contains__(&exc_obj, vm)? { - dir.append((*exc).to_pyobject(vm)); + if !dir.mut_contains(vm, &exc_obj)? { + dir.borrow_vec_mut().push((*exc).to_pyobject(vm)); } } Ok(dir) diff --git a/crates/vm/src/builtins/list.rs b/crates/vm/src/builtins/list.rs index 2a20e2e4658..e8ad1b220e5 100644 --- a/crates/vm/src/builtins/list.rs +++ b/crates/vm/src/builtins/list.rs @@ -171,6 +171,73 @@ impl PyList { zelf.borrow_vec_mut().imul(vm, n)?; Ok(zelf) } + + fn _getitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { + match SequenceIndex::try_from_borrowed_object(vm, needle, "list")? { + SequenceIndex::Int(i) => { + let vec = self.borrow_vec(); + let pos = vec + .wrap_index(i) + .ok_or_else(|| vm.new_index_error("list index out of range"))?; + Ok(vec.do_get(pos)) + } + SequenceIndex::Slice(slice) => self + .borrow_vec() + .getitem_by_slice(vm, slice) + .map(|x| vm.ctx.new_list(x).into()), + } + } + + fn _setitem(&self, needle: &PyObject, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + match SequenceIndex::try_from_borrowed_object(vm, needle, "list")? { + SequenceIndex::Int(index) => self + .borrow_vec_mut() + .setitem_by_index(vm, index, value) + .map_err(|e| { + if e.class().is(vm.ctx.exceptions.index_error) { + vm.new_index_error("list assignment index out of range") + } else { + e + } + }), + SequenceIndex::Slice(slice) => { + let sec = extract_cloned(&value, Ok, vm)?; + self.borrow_vec_mut().setitem_by_slice(vm, slice, &sec) + } + } + } + + pub(crate) fn sort(&self, options: SortOptions, vm: &VirtualMachine) -> PyResult<()> { + // replace list contents with [] for duration of sort. + // this prevents keyfunc from messing with the list and makes it easy to + // check if it tries to append elements to it. + let (mut elements, version_before) = { + let mut guard = self.elements.write(); + let version_before = self.mutation_counter.load(Ordering::Relaxed); + (core::mem::take(&mut *guard), version_before) + }; + let res = do_sort(vm, &mut elements, options.key.as_deref(), options.reverse); + let mutated = { + let mut guard = self.elements.write(); + let mutated = self.mutation_counter.load(Ordering::Relaxed) != version_before; + core::mem::swap(&mut *guard, &mut elements); + mutated + }; + res?; + + if mutated { + return Err(vm.new_value_error("list modified during sort")); + } + + Ok(()) + } + + fn _delitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult<()> { + match SequenceIndex::try_from_borrowed_object(vm, needle, "list")? { + SequenceIndex::Int(i) => self.borrow_vec_mut().delitem_by_index(vm, i), + SequenceIndex::Slice(slice) => self.borrow_vec_mut().delitem_by_slice(vm, slice), + } + } } #[derive(FromArgs, Default, Traverse)] @@ -192,6 +259,7 @@ struct PopArgs { #[pyclass( with( + Py, Constructor, Initializer, AsMapping, @@ -202,7 +270,10 @@ struct PopArgs { ), flags(BASETYPE, SEQUENCE, _MATCH_SELF) )] -impl PyList { +impl PyList {} + +#[pyclass] +impl Py { #[pymethod] pub(crate) fn append(&self, object: PyObjectRef) { self.borrow_vec_mut().push(object); @@ -227,37 +298,25 @@ impl PyList { elements.insert(index, object); } - fn concat(&self, other: &PyObject, vm: &VirtualMachine) -> PyResult> { - let other = other.downcast_ref::().ok_or_else(|| { + fn __add__(&self, other: &PyObject, vm: &VirtualMachine) -> PyResult { + let other = other.downcast_ref::().ok_or_else(|| { vm.new_type_error(format!( "Cannot add {} and {}", - Self::class(&vm.ctx).name(), + PyList::class(&vm.ctx).name(), other.class().name() )) })?; let mut elements = self.borrow_vec().to_vec(); elements.extend(other.borrow_vec().iter().cloned()); - Ok(Self::from(elements).into_ref(&vm.ctx)) + Ok(PyList::from(elements).into_ref(&vm.ctx)) } - fn __add__(&self, other: &PyObject, vm: &VirtualMachine) -> PyResult> { - self.concat(other, vm) - } - - fn inplace_concat( - zelf: &Py, - other: &PyObject, - vm: &VirtualMachine, - ) -> PyResult { - zelf.extend(other.to_owned(), vm)?; - Ok(zelf.to_owned().into()) + fn inplace_concat(&self, other: &PyObject, vm: &VirtualMachine) -> PyResult { + self.extend(other.to_owned(), vm)?; + Ok(self.to_owned().into()) } - fn __iadd__( - zelf: PyRef, - other: PyObjectRef, - vm: &VirtualMachine, - ) -> PyResult> { + fn __iadd__(zelf: PyListRef, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { zelf.extend(other, vm)?; Ok(zelf) } @@ -268,18 +327,14 @@ impl PyList { } #[pymethod] - fn copy(&self, vm: &VirtualMachine) -> PyRef { - Self::from(self.borrow_vec().to_vec()).into_ref(&vm.ctx) - } - - pub fn __len__(&self) -> usize { - self.borrow_vec().len() + fn copy(&self, vm: &VirtualMachine) -> PyListRef { + PyList::from(self.borrow_vec().to_vec()).into_ref(&vm.ctx) } #[pymethod] fn __sizeof__(&self) -> usize { - core::mem::size_of::() - + self.elements.read().capacity() * core::mem::size_of::() + core::mem::size_of::() + + self.payload.elements.read().capacity() * core::mem::size_of::() } #[pymethod] @@ -288,51 +343,16 @@ impl PyList { } #[pymethod] - fn __reversed__(zelf: PyRef) -> PyListReverseIterator { - let position = zelf.__len__().saturating_sub(1); + fn __reversed__(zelf: PyListRef) -> PyListReverseIterator { + let position = zelf.borrow_vec().len().saturating_sub(1); PyListReverseIterator { internal: PyMutex::new(PositionIterInternal::new(zelf, position)), } } - fn _getitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { - match SequenceIndex::try_from_borrowed_object(vm, needle, "list")? { - SequenceIndex::Int(i) => { - let vec = self.borrow_vec(); - let pos = vec - .wrap_index(i) - .ok_or_else(|| vm.new_index_error("list index out of range"))?; - Ok(vec.do_get(pos)) - } - SequenceIndex::Slice(slice) => self - .borrow_vec() - .getitem_by_slice(vm, slice) - .map(|x| vm.ctx.new_list(x).into()), - } - } - #[pymethod(coexist)] fn __getitem__(&self, index: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self._getitem(&index, vm) - } - - fn _setitem(&self, needle: &PyObject, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - match SequenceIndex::try_from_borrowed_object(vm, needle, "list")? { - SequenceIndex::Int(index) => self - .borrow_vec_mut() - .setitem_by_index(vm, index, value) - .map_err(|e| { - if e.class().is(vm.ctx.exceptions.index_error) { - vm.new_index_error("list assignment index out of range") - } else { - e - } - }), - SequenceIndex::Slice(slice) => { - let sec = extract_cloned(&value, Ok, vm)?; - self.borrow_vec_mut().setitem_by_slice(vm, slice, &sec) - } - } + self.payload._getitem(&index, vm) } fn __setitem__( @@ -341,24 +361,24 @@ impl PyList { value: PyObjectRef, vm: &VirtualMachine, ) -> PyResult<()> { - self._setitem(needle, value, vm) + self.payload._setitem(needle, value, vm) } - fn __mul__(&self, n: PySsize, vm: &VirtualMachine) -> PyResult> { - self.repeat(n, vm) + fn __mul__(&self, n: PySsize, vm: &VirtualMachine) -> PyResult { + self.payload.repeat(n, vm) } - fn __imul__(zelf: PyRef, n: PySsize, vm: &VirtualMachine) -> PyResult> { - Self::irepeat(zelf, n, vm) + fn __imul__(zelf: PyListRef, n: PySsize, vm: &VirtualMachine) -> PyResult { + PyList::irepeat(zelf, n, vm) } #[pymethod] fn count(&self, value: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.mut_count(vm, &value) + self.payload.mut_count(vm, &value) } pub(crate) fn __contains__(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { - self.mut_contains(vm, needle) + self.payload.mut_contains(vm, needle) } #[pymethod] @@ -368,8 +388,8 @@ impl PyList { range: OptionalRangeArgs, vm: &VirtualMachine, ) -> PyResult { - let (start, stop) = range.saturate(self.__len__(), vm)?; - let index = self.mut_index_range(vm, &value, start..stop)?; + let (start, stop) = range.saturate(self.borrow_vec().len(), vm)?; + let index = self.payload.mut_index_range(vm, &value, start..stop)?; if let Some(index) = index.into() { Ok(index) } else { @@ -395,7 +415,7 @@ impl PyList { #[pymethod] fn remove(&self, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - let index = self.mut_index(vm, &value)?; + let index = self.payload.mut_index(vm, &value)?; if let Some(index) = index.into() { // defer delete out of borrow @@ -410,41 +430,13 @@ impl PyList { } } - fn _delitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult<()> { - match SequenceIndex::try_from_borrowed_object(vm, needle, "list")? { - SequenceIndex::Int(i) => self.borrow_vec_mut().delitem_by_index(vm, i), - SequenceIndex::Slice(slice) => self.borrow_vec_mut().delitem_by_slice(vm, slice), - } - } - fn __delitem__(&self, subscript: &PyObject, vm: &VirtualMachine) -> PyResult<()> { - self._delitem(subscript, vm) + self.payload._delitem(subscript, vm) } #[pymethod] pub(crate) fn sort(&self, options: SortOptions, vm: &VirtualMachine) -> PyResult<()> { - // replace list contents with [] for duration of sort. - // this prevents keyfunc from messing with the list and makes it easy to - // check if it tries to append elements to it. - let (mut elements, version_before) = { - let mut guard = self.elements.write(); - let version_before = self.mutation_counter.load(Ordering::Relaxed); - (core::mem::take(&mut *guard), version_before) - }; - let res = do_sort(vm, &mut elements, options.key.as_deref(), options.reverse); - let mutated = { - let mut guard = self.elements.write(); - let mutated = self.mutation_counter.load(Ordering::Relaxed) != version_before; - core::mem::swap(&mut *guard, &mut elements); - mutated - }; - res?; - - if mutated { - return Err(vm.new_value_error("list modified during sort")); - } - - Ok(()) + self.payload.sort(options, vm) } #[pyclassmethod] @@ -559,16 +551,18 @@ impl Initializer for PyList { impl AsMapping for PyList { fn as_mapping() -> &'static PyMappingMethods { static AS_MAPPING: PyMappingMethods = PyMappingMethods { - length: atomic_func!(|mapping, _vm| Ok(PyList::mapping_downcast(mapping).__len__())), - subscript: atomic_func!( - |mapping, needle, vm| PyList::mapping_downcast(mapping)._getitem(needle, vm) - ), + length: atomic_func!(|mapping, _vm| Ok(PyList::mapping_downcast(mapping) + .borrow_vec() + .len())), + subscript: atomic_func!(|mapping, needle, vm| PyList::mapping_downcast(mapping) + .payload + ._getitem(needle, vm)), ass_subscript: atomic_func!(|mapping, needle, value, vm| { let zelf = PyList::mapping_downcast(mapping); if let Some(value) = value { - zelf._setitem(needle, value, vm) + zelf.__setitem__(needle, value, vm) } else { - zelf._delitem(needle, vm) + zelf.__delitem__(needle, vm) } }), }; @@ -579,14 +573,15 @@ impl AsMapping for PyList { impl AsSequence for PyList { fn as_sequence() -> &'static PySequenceMethods { static AS_SEQUENCE: PySequenceMethods = PySequenceMethods { - length: atomic_func!(|seq, _vm| Ok(PyList::sequence_downcast(seq).__len__())), + length: atomic_func!(|seq, _vm| Ok(PyList::sequence_downcast(seq).borrow_vec().len())), concat: atomic_func!(|seq, other, vm| { PyList::sequence_downcast(seq) - .concat(other, vm) + .__add__(other, vm) .map(|x| x.into()) }), repeat: atomic_func!(|seq, n, vm| { PyList::sequence_downcast(seq) + .payload .repeat(n, vm) .map(|x| x.into()) }), @@ -615,11 +610,10 @@ impl AsSequence for PyList { }), contains: atomic_func!(|seq, target, vm| { let zelf = PyList::sequence_downcast(seq); - zelf.mut_contains(vm, target) + zelf.__contains__(target, vm) }), inplace_concat: atomic_func!(|seq, other, vm| { - let zelf = PyList::sequence_downcast(seq); - PyList::inplace_concat(zelf, other, vm) + PyList::sequence_downcast(seq).inplace_concat(other, vm) }), inplace_repeat: atomic_func!(|seq, n, vm| { let zelf = PyList::sequence_downcast(seq); @@ -671,7 +665,7 @@ impl Comparable for PyList { impl Representable for PyList { #[inline] fn repr(zelf: &Py, vm: &VirtualMachine) -> PyResult> { - if zelf.__len__() == 0 { + if zelf.borrow_vec().is_empty() { return Ok(vm.ctx.intern_str("[]").to_owned()); } @@ -955,14 +949,16 @@ impl PyPayload for PyListIterator { impl PyListIterator { #[pymethod] fn __length_hint__(&self) -> usize { - self.internal.lock().length_hint(|obj| obj.__len__()) + self.internal + .lock() + .length_hint(|obj| obj.borrow_vec().len()) } #[pymethod] fn __setstate__(&self, object: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { self.internal .lock() - .set_state(&object, |obj, pos| pos.min(obj.__len__()), vm) + .set_state(&object, |obj, pos| pos.min(obj.borrow_vec().len()), vm) } #[pymethod] @@ -1019,14 +1015,16 @@ impl PyPayload for PyListReverseIterator { impl PyListReverseIterator { #[pymethod] fn __length_hint__(&self) -> usize { - self.internal.lock().rev_length_hint(|obj| obj.__len__()) + self.internal + .lock() + .rev_length_hint(|obj| obj.borrow_vec().len()) } #[pymethod] fn __setstate__(&self, state: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { self.internal .lock() - .set_state(&state, |obj, pos| pos.min(obj.__len__()), vm) + .set_state(&state, |obj, pos| pos.min(obj.borrow_vec().len()), vm) } #[pymethod] diff --git a/crates/vm/src/builtins/object.rs b/crates/vm/src/builtins/object.rs index dc76d29c1f4..1862aa5c289 100644 --- a/crates/vm/src/builtins/object.rs +++ b/crates/vm/src/builtins/object.rs @@ -221,7 +221,7 @@ fn object_getstate_default(obj: &PyObject, required: bool, vm: &VirtualMachine) } if let Some(slot_names) = slot_names { - let slot_names_len = slot_names.__len__(); + let slot_names_len = slot_names.borrow_vec().len(); if slot_names_len > 0 { let slots = vm.ctx.new_dict(); for i in 0..slot_names_len { From 8eb420c91cb331ea67d9e607233665f4bd404e30 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 27 Sep 2026 23:02:03 +0900 Subject: [PATCH 2/4] Rename Traverse::clear to clear_refs Traverse::clear() and MaybeTraverse::try_clear() are renamed to clear_refs() and try_clear_refs(). The old name shadowed the clear() method of containers such as Py wherever Traverse was in scope, because method lookup reached the trait impl for PyRef before dereferencing to Py. The #[pyclass(clear)] attribute and HAS_CLEAR are unchanged. Assisted-by: Claude Code:claude-opus-5-5 --- crates/derive-impl/src/pyclass.rs | 6 +++--- crates/derive-impl/src/pystructseq.rs | 4 ++-- crates/stdlib/src/elementtree.rs | 6 +++--- crates/vm/src/builtins/dict.rs | 2 +- crates/vm/src/builtins/function.rs | 2 +- crates/vm/src/builtins/list.rs | 2 +- crates/vm/src/builtins/slice.rs | 2 +- crates/vm/src/builtins/str.rs | 2 +- crates/vm/src/builtins/tuple.rs | 2 +- crates/vm/src/builtins/type.rs | 2 +- crates/vm/src/exceptions.rs | 2 +- crates/vm/src/frame.rs | 8 ++++---- crates/vm/src/object/core.rs | 4 ++-- crates/vm/src/object/traverse.rs | 4 ++-- crates/vm/src/stdlib/_collections.rs | 6 +++--- crates/vm/src/stdlib/_collections/ordered_dict.rs | 6 +++--- crates/vm/src/stdlib/_functools.rs | 2 +- crates/vm/src/stdlib/_testinternalcapi.rs | 2 +- crates/vm/src/stdlib/_thread.rs | 6 +++--- 19 files changed, 35 insertions(+), 35 deletions(-) diff --git a/crates/derive-impl/src/pyclass.rs b/crates/derive-impl/src/pyclass.rs index bf745f97304..b51ed5338f3 100644 --- a/crates/derive-impl/src/pyclass.rs +++ b/crates/derive-impl/src/pyclass.rs @@ -746,7 +746,7 @@ pub(crate) fn impl_pyclass(attr: PunctuatedNestedMeta, item: Item) -> Result Result Result) { + fn try_clear_refs(&mut self, out: &mut ::std::vec::Vec<::rustpython_vm::PyObjectRef>) { #try_clear_body } } diff --git a/crates/derive-impl/src/pystructseq.rs b/crates/derive-impl/src/pystructseq.rs index 08b5da311ff..b595b646b08 100644 --- a/crates/derive-impl/src/pystructseq.rs +++ b/crates/derive-impl/src/pystructseq.rs @@ -596,8 +596,8 @@ pub(crate) fn impl_pystruct_sequence( self.0.try_traverse(traverse_fn) } - fn try_clear(&mut self, out: &mut ::std::vec::Vec<::rustpython_vm::PyObjectRef>) { - self.0.try_clear(out) + fn try_clear_refs(&mut self, out: &mut ::std::vec::Vec<::rustpython_vm::PyObjectRef>) { + self.0.try_clear_refs(out) } } diff --git a/crates/stdlib/src/elementtree.rs b/crates/stdlib/src/elementtree.rs index 92963af6635..395c80da9a8 100644 --- a/crates/stdlib/src/elementtree.rs +++ b/crates/stdlib/src/elementtree.rs @@ -202,7 +202,7 @@ pub(crate) mod _elementtree { inner.children.traverse(traverse_fn); } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { // Only reached for an object the collector has already proven // unreachable, so grabbing the lock cannot race with user code. let Some(mut inner) = self.inner.try_write() else { @@ -1331,7 +1331,7 @@ pub(crate) mod _elementtree { st.pi_event.traverse(traverse_fn); } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { let Some(mut st) = self.state.try_write() else { return; }; @@ -1880,7 +1880,7 @@ pub(crate) mod _elementtree { st.handle_doctype.traverse(traverse_fn); } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { let Some(mut st) = self.state.try_write() else { return; }; diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index 1afb16c0316..437fe9037fa 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -44,7 +44,7 @@ unsafe impl Traverse for PyDict { self.entries.traverse(traverse_fn); } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { // Pop all entries and collect both keys and values for (key, value) in self.entries.drain_entries() { out.push(key); diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs index a63e67f2be0..450673995df 100644 --- a/crates/vm/src/builtins/function.rs +++ b/crates/vm/src/builtins/function.rs @@ -115,7 +115,7 @@ unsafe impl Traverse for PyFunction { self.doc.traverse(tracer_fn); } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { // Pop closure if present (equivalent to Py_CLEAR(func_closure)) if let Some(closure) = self.closure.take() { out.push(closure.into()); diff --git a/crates/vm/src/builtins/list.rs b/crates/vm/src/builtins/list.rs index e8ad1b220e5..2d8fc178e59 100644 --- a/crates/vm/src/builtins/list.rs +++ b/crates/vm/src/builtins/list.rs @@ -68,7 +68,7 @@ unsafe impl Traverse for PyList { self.elements.traverse(traverse_fn); } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { // During GC, we use interior mutability to access elements. // This is safe because during GC collection, the object is unreachable // and no other code should be accessing it. diff --git a/crates/vm/src/builtins/slice.rs b/crates/vm/src/builtins/slice.rs index 465e02b96be..3d93c9a2940 100644 --- a/crates/vm/src/builtins/slice.rs +++ b/crates/vm/src/builtins/slice.rs @@ -35,7 +35,7 @@ unsafe impl crate::object::Traverse for PySlice { self.step.traverse(traverse_fn); } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { if let Some(start) = self.start.take() { out.push(start); } diff --git a/crates/vm/src/builtins/str.rs b/crates/vm/src/builtins/str.rs index cac1db24ecc..cf499b4d932 100644 --- a/crates/vm/src/builtins/str.rs +++ b/crates/vm/src/builtins/str.rs @@ -2106,7 +2106,7 @@ impl MaybeTraverse for PyUtf8Str { self.0.try_traverse(traverse_fn); } - fn try_clear(&mut self, _out: &mut Vec) { + fn try_clear_refs(&mut self, _out: &mut Vec) { // No clear needed for PyUtf8Str } } diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index 2297e5aafa4..19785330b22 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -94,7 +94,7 @@ unsafe impl Traverse for PyTuple { self.elements.as_slice().traverse(traverse_fn); } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { let elements = core::mem::take(self.elements.get_mut()); if out.is_empty() { // Reuse the allocation so deallocation does not need more memory. diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index 4330e711097..72bf09f64be 100644 --- a/crates/vm/src/builtins/type.rs +++ b/crates/vm/src/builtins/type.rs @@ -261,7 +261,7 @@ unsafe impl crate::object::Traverse for PyType { } /// type_clear: break reference cycles in type objects - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { // SAFETY: tp_clear runs with exclusive access to the type object. if let Some(base) = unsafe { self.base.swap(None) } { out.push(base.into()); diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index 588754bb36c..4e0c89dea24 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -2224,7 +2224,7 @@ pub(super) mod types { self.0.0.traverse(tracer_fn); } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { let base = &mut self.0.0; if let Some(traceback) = base.traceback.get_mut().take() { out.push(traceback.into()); diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 5af8be7b4a7..85d8f35cedb 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -1531,7 +1531,7 @@ pub struct FrameObject { #[pymember(name = "f_trace_lines", writable)] pub(crate) f_trace_lines: core::sync::atomic::AtomicBool, // Owned references — keep the pointed-to objects alive for InterpreterFrame's - // raw pointers. Wrapped in Option so Traverse::clear can release them, + // raw pointers. Wrapped in Option so Traverse::clear_refs can release them, // allowing GC cycle collection to reclaim referenced objects. pub(crate) owned_code: Option>, pub(crate) owned_globals: Option, @@ -1539,7 +1539,7 @@ pub struct FrameObject { pub(crate) owned_func_obj: Option, /// Always `Some` while the frame is reachable from Python. Emptied only - /// by `Traverse::clear` during deallocation, leaving a trivially-droppable + /// by `Traverse::clear_refs` during deallocation, leaving a trivially-droppable /// husk that the freelist can cache. pub(crate) iframe: FrameUnsafeCell>, } @@ -1550,7 +1550,7 @@ impl FrameObject { /// # Safety /// Caller must ensure no concurrent mutable access (see `FrameUnsafeCell`) /// and that the frame has not been cleared (i.e. it is still reachable - /// from Python; `Traverse::clear` only runs during deallocation). + /// from Python; `Traverse::clear_refs` only runs during deallocation). #[inline(always)] pub(crate) unsafe fn iframe_ref(&self) -> &InterpreterFrame { let opt = unsafe { &*self.iframe.get() }; @@ -1682,7 +1682,7 @@ unsafe impl Traverse for FrameObject { } } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { // Extract every child before dropping the frame husk. GC drops `out` // after this exclusive payload borrow ends, so re-entrant finalizers // cannot alias frame storage or run under one of its locks. diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index ef3eaa12e12..c5e3613106d 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -284,7 +284,7 @@ pub(super) unsafe fn try_traverse_obj(x: &PyObject, tracer_fn: &mu /// Call `try_clear` on payload to extract child references (tp_clear) pub(super) unsafe fn try_clear_obj(x: *mut PyObject, out: &mut Vec) { let x = unsafe { &mut *(x as *mut Py) }; - x.payload.try_clear(out); + x.payload.try_clear_refs(out); } bitflags::bitflags! { @@ -3257,7 +3257,7 @@ mod tests { } else { Vec::new() }; - sequence.clear(&mut out); + sequence.clear_refs(&mut out); if existing { assert_eq!(out[0].try_to_value::(vm).unwrap(), 1); } else { diff --git a/crates/vm/src/object/traverse.rs b/crates/vm/src/object/traverse.rs index 0f7b13d16bb..34df3d672cf 100644 --- a/crates/vm/src/object/traverse.rs +++ b/crates/vm/src/object/traverse.rs @@ -20,7 +20,7 @@ pub trait MaybeTraverse { // if this type is traceable, then call with tracer_fn, default to do nothing fn try_traverse(&self, traverse_fn: &mut TraverseFn<'_>); // if this type has clear, extract child refs for circular reference resolution (tp_clear) - fn try_clear(&mut self, _out: &mut Vec) {} + fn try_clear_refs(&mut self, _out: &mut Vec) {} } /// Type that need traverse it's children should impl [`Traverse`] (not [`MaybeTraverse`]) @@ -38,7 +38,7 @@ pub unsafe trait Traverse { /// Extract all owned child PyObjectRefs for circular reference resolution (tp_clear). /// Called just before object deallocation to break circular references. /// Default implementation does nothing. - fn clear(&mut self, _out: &mut Vec) {} + fn clear_refs(&mut self, _out: &mut Vec) {} } unsafe impl Traverse for PyObjectRef { diff --git a/crates/vm/src/stdlib/_collections.rs b/crates/vm/src/stdlib/_collections.rs index afc91a8f482..db502cdb220 100644 --- a/crates/vm/src/stdlib/_collections.rs +++ b/crates/vm/src/stdlib/_collections.rs @@ -66,7 +66,7 @@ mod _collections { } } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { out.extend(self.deque.get_mut().drain(..)); } } @@ -948,8 +948,8 @@ mod _collections { self.default_factory.traverse(tracer_fn); } - fn clear(&mut self, out: &mut Vec) { - Traverse::clear(&mut self.dict, out); + fn clear_refs(&mut self, out: &mut Vec) { + Traverse::clear_refs(&mut self.dict, out); if let Some(factory) = self.default_factory.store(None) { out.push(factory); } diff --git a/crates/vm/src/stdlib/_collections/ordered_dict.rs b/crates/vm/src/stdlib/_collections/ordered_dict.rs index 7312784d9eb..0b5970a0c8d 100644 --- a/crates/vm/src/stdlib/_collections/ordered_dict.rs +++ b/crates/vm/src/stdlib/_collections/ordered_dict.rs @@ -162,8 +162,8 @@ pub(crate) mod ordered_dict { } } - fn clear(&mut self, out: &mut Vec) { - Traverse::clear(&mut self.dict, out); + fn clear_refs(&mut self, out: &mut Vec) { + Traverse::clear_refs(&mut self.dict, out); let links = self.links.get_mut(); for node in links.nodes.drain(..).flatten() { out.push(node.key); @@ -857,7 +857,7 @@ pub(crate) mod ordered_dict { } } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { if let Some(key) = self.current.get_mut().take() { out.push(key); } diff --git a/crates/vm/src/stdlib/_functools.rs b/crates/vm/src/stdlib/_functools.rs index 6df6b2185cc..ee96058972d 100644 --- a/crates/vm/src/stdlib/_functools.rs +++ b/crates/vm/src/stdlib/_functools.rs @@ -246,7 +246,7 @@ mod _functools { } } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { // __setstate__ can make func point directly back to this partial. // The args tuple and keywords dict have their own cycle clearing. out.push(core::mem::replace( diff --git a/crates/vm/src/stdlib/_testinternalcapi.rs b/crates/vm/src/stdlib/_testinternalcapi.rs index 33d3b172e82..a7d41af1dfa 100644 --- a/crates/vm/src/stdlib/_testinternalcapi.rs +++ b/crates/vm/src/stdlib/_testinternalcapi.rs @@ -537,7 +537,7 @@ mod _testinternalcapi { } } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { #[allow(clippy::iter_over_hash_type)] for (_, pairs) in self.buckets.drain() { for (k, v) in pairs { diff --git a/crates/vm/src/stdlib/_thread.rs b/crates/vm/src/stdlib/_thread.rs index 5721fab5610..30550c0cf00 100644 --- a/crates/vm/src/stdlib/_thread.rs +++ b/crates/vm/src/stdlib/_thread.rs @@ -1169,7 +1169,7 @@ pub(crate) mod _thread { } } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { out.append(&mut self.init_args.args); out.extend(self.init_args.kwargs.drain(..).map(|(_, value)| value)); out.extend(self.dicts.drain().map(|(_, dict)| dict.into())); @@ -1199,9 +1199,9 @@ pub(crate) mod _thread { self.inner.state.traverse(tracer_fn); } - fn clear(&mut self, out: &mut Vec) { + fn clear_refs(&mut self, out: &mut Vec) { if let Some(mut state) = self.inner.state.try_lock() { - state.clear(out); + state.clear_refs(out); } } } From 7f83162bd7273d8836b814d07e12f10aaff046cb Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 27 Sep 2026 22:33:06 +0900 Subject: [PATCH 3/4] Expose the PyList_* operations on Py Py gains pub len(), is_empty(), get_item(), set_item(), get_slice(), set_slice(), del_slice(), sort() and to_tuple(), matching PyList_Size, PyList_GetItemRef, PyList_SetItem, PyList_GetSlice, PyList_SetSlice, PyList_Sort and PyList_AsTuple. append(), extend(), insert(), clear() and reverse() (PyList_Append, PyList_Extend, PyList_Insert, PyList_Clear, PyList_Reverse) become pub. The sort pymethod is implemented by sort_with_options(). get_slice(), set_slice() and del_slice() clamp both bounds to the list. set_item(), set_slice() and del_slice() drop the replaced elements after releasing the list lock. The capi PyList_Size, PyList_Append, PyList_Insert, PyList_Reverse and PyList_AsTuple call these methods. Assisted-by: Claude Code:claude-opus-5-5 --- crates/capi/src/listobject.rs | 17 ++-- crates/vm/src/builtins/list.rs | 155 +++++++++++++++++++++++++++++++-- 2 files changed, 153 insertions(+), 19 deletions(-) diff --git a/crates/capi/src/listobject.rs b/crates/capi/src/listobject.rs index ec31e184b4b..eb6e343db4a 100644 --- a/crates/capi/src/listobject.rs +++ b/crates/capi/src/listobject.rs @@ -25,7 +25,7 @@ pub extern "C" fn PyList_New(size: isize) -> *mut PyObject { pub unsafe extern "C" fn PyList_Size(obj: *mut PyObject) -> isize { with_vm(|vm| { let list = unsafe { obj.assume_borrowed_and_cast::(vm) }?; - Ok(list.borrow_vec().len()) + Ok(list.len()) }) } @@ -77,7 +77,7 @@ pub unsafe extern "C" fn PyList_Append(list: *mut PyObject, item: *mut PyObject) with_vm(|vm| { let list = unsafe { list.assume_borrowed_and_cast::(vm) }?; let item = unsafe { item.assume_borrowed() }.to_owned(); - list.borrow_vec_mut().push(item); + list.append(item); Ok(()) }) } @@ -91,14 +91,7 @@ pub unsafe extern "C" fn PyList_Insert( with_vm(|vm| { let list = unsafe { list.assume_borrowed_and_cast::(vm) }?; let item = unsafe { item.assume_borrowed() }.to_owned(); - let mut vec = list.borrow_vec_mut(); - let index = if index < 0 { - index + vec.len() as isize - } else { - index - } - .clamp(0, vec.len() as isize) as usize; - vec.insert(index, item); + list.insert(index, item); Ok(()) }) } @@ -107,7 +100,7 @@ pub unsafe extern "C" fn PyList_Insert( pub unsafe extern "C" fn PyList_Reverse(list: *mut PyObject) -> c_int { with_vm(|vm| { let list = unsafe { list.assume_borrowed_and_cast::(vm) }?; - list.borrow_vec_mut().reverse(); + list.reverse(); Ok(()) }) } @@ -116,7 +109,7 @@ pub unsafe extern "C" fn PyList_Reverse(list: *mut PyObject) -> c_int { pub unsafe extern "C" fn PyList_AsTuple(list: *mut PyObject) -> *mut PyObject { with_vm(|vm| { let list = unsafe { list.assume_borrowed_and_cast::(vm) }?; - Ok(vm.ctx.new_tuple(list.borrow_vec().to_vec())) + Ok(list.to_tuple(vm)) }) } diff --git a/crates/vm/src/builtins/list.rs b/crates/vm/src/builtins/list.rs index 2d8fc178e59..de761ae90a3 100644 --- a/crates/vm/src/builtins/list.rs +++ b/crates/vm/src/builtins/list.rs @@ -143,6 +143,95 @@ impl Py { pub fn borrow_vec_mut(&self) -> PyRwLockWriteGuard<'_, Vec> { self.payload().borrow_vec_mut() } + + /// The number of elements (`PyList_Size`). + #[inline] + pub fn len(&self) -> usize { + self.borrow_vec().len() + } + + #[inline] + pub fn is_empty(&self) -> bool { + self.borrow_vec().is_empty() + } + + /// The element at `index` (`PyList_GetItemRef`). + pub fn get_item(&self, index: usize, vm: &VirtualMachine) -> PyResult { + self.borrow_vec() + .get(index) + .cloned() + .ok_or_else(|| vm.new_index_error("list index out of range")) + } + + /// Replace the element at `index` (`PyList_SetItem`). + pub fn set_item(&self, index: usize, item: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let old = { + let mut elements = self.borrow_vec_mut(); + let slot = elements + .get_mut(index) + .ok_or_else(|| vm.new_index_error("list assignment index out of range"))?; + core::mem::replace(slot, item) + }; + drop(old); + Ok(()) + } + + /// A new list of the elements from `low` to `high`, both clamped to the + /// list (`PyList_GetSlice`). + pub fn get_slice(&self, low: isize, high: isize, vm: &VirtualMachine) -> PyListRef { + let elements = self.borrow_vec(); + let range = clamp_slice(low, high, elements.len()); + PyList::from(elements[range].to_vec()).into_ref(&vm.ctx) + } + + /// Replace the elements from `low` to `high` with the items of `items` + /// (`PyList_SetSlice`). + pub fn set_slice( + &self, + low: isize, + high: isize, + items: &PyObject, + vm: &VirtualMachine, + ) -> PyResult<()> { + let items = extract_cloned(items, Ok, vm)?; + let removed: Vec<_> = { + let mut elements = self.borrow_vec_mut(); + let range = clamp_slice(low, high, elements.len()); + elements.splice(range, items).collect() + }; + drop(removed); + Ok(()) + } + + /// Remove the elements from `low` to `high` (`PyList_SetSlice` with + /// `NULL`). + pub fn del_slice(&self, low: isize, high: isize) { + let removed: Vec<_> = { + let mut elements = self.borrow_vec_mut(); + let range = clamp_slice(low, high, elements.len()); + elements.drain(range).collect() + }; + drop(removed); + } + + /// Sort in place with the default key and order (`PyList_Sort`). + pub fn sort(&self, vm: &VirtualMachine) -> PyResult<()> { + self.payload.sort(SortOptions::default(), vm) + } + + /// A new tuple of the elements (`PyList_AsTuple`). + pub fn to_tuple(&self, vm: &VirtualMachine) -> PyTupleRef { + vm.ctx.new_tuple(self.borrow_vec().to_vec()) + } +} + +/// `low..high` clamped to a list of `len` elements, as `PyList_GetSlice` and +/// `PyList_SetSlice` do. +fn clamp_slice(low: isize, high: isize, len: usize) -> core::ops::Range { + let len = len as isize; + let low = low.clamp(0, len); + let high = high.clamp(low, len); + low as usize..high as usize } impl PyList { @@ -275,12 +364,12 @@ impl PyList {} #[pyclass] impl Py { #[pymethod] - pub(crate) fn append(&self, object: PyObjectRef) { + pub fn append(&self, object: PyObjectRef) { self.borrow_vec_mut().push(object); } #[pymethod] - pub(crate) fn extend(&self, iterable: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + pub fn extend(&self, iterable: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { // What is already here decides whether the iterable's length hint is // believable, so it goes along with the request for the elements. It is // counted where `list_extend()` reads `Py_SIZE(self)`, after the @@ -292,7 +381,7 @@ impl Py { } #[pymethod] - pub(crate) fn insert(&self, index: PySsize, object: PyObjectRef) { + pub fn insert(&self, index: PySsize, object: PyObjectRef) { let mut elements = self.borrow_vec_mut(); let index = elements.saturate_index(index); elements.insert(index, object); @@ -322,7 +411,7 @@ impl Py { } #[pymethod] - fn clear(&self) { + pub fn clear(&self) { let _removed = core::mem::take(self.borrow_vec_mut().deref_mut()); } @@ -338,7 +427,7 @@ impl Py { } #[pymethod] - fn reverse(&self) { + pub fn reverse(&self) { self.borrow_vec_mut().reverse(); } @@ -434,8 +523,8 @@ impl Py { self.payload._delitem(subscript, vm) } - #[pymethod] - pub(crate) fn sort(&self, options: SortOptions, vm: &VirtualMachine) -> PyResult<()> { + #[pymethod(name = "sort")] + fn sort_with_options(&self, options: SortOptions, vm: &VirtualMachine) -> PyResult<()> { self.payload.sort(options, vm) } @@ -1071,3 +1160,55 @@ pub(crate) fn init(context: &'static Context) { PyListIterator::extend_class(context, context.types.list_iterator_type); PyListReverseIterator::extend_class(context, context.types.list_reverseiterator_type); } + +#[cfg(test)] +mod tests { + use super::*; + use crate::Interpreter; + + fn ints(list: &Py, vm: &VirtualMachine) -> Vec { + list.borrow_vec() + .iter() + .map(|x| x.try_to_value(vm).unwrap()) + .collect() + } + + #[test] + fn list_api() { + Interpreter::without_stdlib(Default::default()).enter(|vm| { + let int = |i: i32| vm.ctx.new_int(i).into(); + let list = vm.ctx.new_list((1..=4).map(int).collect()); + assert_eq!(list.len(), 4); + assert!(!list.is_empty()); + + assert_eq!(ints(&list.get_slice(-5, 2, vm), vm), [1, 2]); + assert_eq!(ints(&list.get_slice(3, 1, vm), vm), [] as [i32; 0]); + assert_eq!(ints(&list.get_slice(1, 10, vm), vm), [2, 3, 4]); + + let err = list.get_item(4, vm).unwrap_err(); + assert!(err.fast_isinstance(vm.ctx.exceptions.index_error)); + list.set_item(0, int(9), vm).unwrap(); + let err = list.set_item(4, int(0), vm).unwrap_err(); + assert!(err.fast_isinstance(vm.ctx.exceptions.index_error)); + let item: i32 = list.get_item(0, vm).unwrap().try_to_value(vm).unwrap(); + assert_eq!(item, 9); + + let repl = vm.ctx.new_list(vec![int(7), int(8)]); + list.set_slice(1, 3, repl.as_object(), vm).unwrap(); + assert_eq!(ints(&list, vm), [9, 7, 8, 4]); + list.set_slice(-1, 1, list.as_object(), vm).unwrap(); + assert_eq!(ints(&list, vm), [9, 7, 8, 4, 7, 8, 4]); + list.del_slice(2, 100); + assert_eq!(ints(&list, vm), [9, 7]); + + list.insert(-10, int(5)); + list.append(int(1)); + list.sort(vm).unwrap(); + assert_eq!(ints(&list, vm), [1, 5, 7, 9]); + list.reverse(); + assert_eq!(list.to_tuple(vm).as_slice().len(), 4); + list.clear(); + assert!(list.is_empty()); + }) + } +} From d4112a2808a0d030e2f12fb4f4131695a4ee7eb5 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 27 Sep 2026 23:37:21 +0900 Subject: [PATCH 4/4] Call the Py methods from the remaining capi PyList_* functions PyList_GetItemRef, PyList_SetItem, PyList_GetSlice, PyList_SetSlice and PyList_Sort call get_item(), set_item(), get_slice(), set_slice(), del_slice() and sort() on Py. - PyList_GetSlice and PyList_SetSlice clamp negative bounds to 0 instead of counting them from the end of the list. - PyList_SetSlice collects the new items before taking the list lock, so a list can be assigned into itself. - PyList_GetItemRef and PyList_SetItem raise IndexError without the index in the message. - PyList_Sort sorts the list directly instead of calling its sort method. - PyList_SetItem still appends when the index equals the length and the list has spare capacity, as after PyList_New. Assisted-by: Claude Code:claude-opus-5-5 --- crates/capi/src/listobject.rs | 84 ++++++++++++++++++++--------------- 1 file changed, 49 insertions(+), 35 deletions(-) diff --git a/crates/capi/src/listobject.rs b/crates/capi/src/listobject.rs index eb6e343db4a..ea3efbb7c00 100644 --- a/crates/capi/src/listobject.rs +++ b/crates/capi/src/listobject.rs @@ -3,10 +3,7 @@ use crate::object::define_py_check; use crate::pystate::with_vm; use crate::util::FfiPtrExt; use core::ffi::c_int; -use rustpython_vm::AsObject; -use rustpython_vm::PyObjectRef; use rustpython_vm::builtins::PyList; -use rustpython_vm::sliceable::{SaturatedSlice, SliceableSequenceMutOp, SliceableSequenceOp}; define_py_check!(fn PyList_Check, types.list_type); define_py_check!(exact fn PyList_CheckExact, types.list_type); @@ -33,11 +30,8 @@ pub unsafe extern "C" fn PyList_Size(obj: *mut PyObject) -> isize { pub unsafe extern "C" fn PyList_GetItemRef(obj: *mut PyObject, index: isize) -> *mut PyObject { with_vm(|vm| { let list = unsafe { obj.assume_borrowed_and_cast::(vm) }?; - index - .try_into() - .ok() - .and_then(|index: usize| list.borrow_vec().get(index).map(ToOwned::to_owned)) - .ok_or_else(|| vm.new_index_error(format!("list index out of range: {index}"))) + // A negative index wraps past the end and is out of range. + list.get_item(index as usize, vm) }) } @@ -50,25 +44,17 @@ pub unsafe extern "C" fn PyList_SetItem( with_vm(|vm| { let list = unsafe { list.assume_borrowed_and_cast::(vm) }?; let item = unsafe { item.assume_owned() }; - let index_error = - || vm.new_index_error(format!("list assignment index out of range: {index}")); - if index < 0 { - return Err(index_error()); - } - - let mut list_mut = list.borrow_vec_mut(); - match index - list_mut.len() as isize { - ..0 => { - list_mut[index as usize] = item; - Ok(()) - } + // A negative index wraps past the end and is out of range. + let index = index as usize; + { + let mut list_mut = list.borrow_vec_mut(); // This is somewhat a hack, we assume that we are populating a list right after PyList_New - 0 if list_mut.capacity() > index as usize => { + if index == list_mut.len() && list_mut.capacity() > index { list_mut.push(item); - Ok(()) + return Ok(()); } - 0.. => Err(index_error()), } + list.set_item(index, item, vm) }) } @@ -121,9 +107,7 @@ pub unsafe extern "C" fn PyList_GetSlice( ) -> *mut PyObject { with_vm(|vm| { let list = unsafe { list.assume_borrowed_and_cast::(vm) }?; - let vec = list.borrow_vec(); - let sliced = vec.getitem_by_slice(vm, SaturatedSlice::from_parts(low, high, 1))?; - Ok(vm.ctx.new_list(sliced)) + Ok(list.get_slice(low, high, vm)) }) } @@ -136,16 +120,11 @@ pub unsafe extern "C" fn PyList_SetSlice( ) -> c_int { with_vm(|vm| { let list = unsafe { list.assume_borrowed_and_cast::(vm) }?; - let slice = SaturatedSlice::from_parts(low, high, 1); - let mut vec = list.borrow_vec_mut(); - let Some(itemlist) = (unsafe { itemlist.assume_borrowed_or_opt() }) else { - vec.delitem_by_slice(vm, slice)?; + list.del_slice(low, high); return Ok(()); }; - - let items: Vec = itemlist.try_to_value(vm)?; - vec.setitem_by_slice(vm, slice, &items) + list.set_slice(low, high, itemlist, vm) }) } @@ -153,14 +132,14 @@ pub unsafe extern "C" fn PyList_SetSlice( pub unsafe extern "C" fn PyList_Sort(list: *mut PyObject) -> c_int { with_vm(|vm| { let list = unsafe { list.assume_borrowed_and_cast::(vm) }?; - vm.call_method(list.as_object(), "sort", ())?; - Ok(()) + list.sort(vm) }) } #[cfg(test)] mod tests { use pyo3::exceptions::PyIndexError; + use pyo3::ffi; use pyo3::prelude::*; use pyo3::types::{PyList, PyListMethods}; @@ -281,4 +260,39 @@ mod tests { assert_eq!(list.get_item(2).unwrap().extract::().unwrap(), 3); }) } + + #[test] + fn list_del_slice() { + Python::attach(|py| { + let list = PyList::new(py, [1, 2, 3, 4]).unwrap(); + list.del_slice(1, 3).unwrap(); + assert_eq!(list.extract::>().unwrap(), [1, 4]); + }) + } + + #[test] + fn list_negative_indices() { + Python::attach(|py| unsafe { + let list = PyList::new(py, [1, 2, 3, 4]).unwrap(); + + assert!(ffi::PyList_GetItemRef(list.as_ptr(), -1).is_null()); + assert!(PyErr::take(py).unwrap().is_instance_of::(py)); + + let slice = Bound::from_owned_ptr(py, ffi::PyList_GetSlice(list.as_ptr(), -1, 2)); + assert_eq!(slice.extract::>().unwrap(), [1, 2]); + + let repl = PyList::new(py, [9]).unwrap(); + assert_eq!(ffi::PyList_SetSlice(list.as_ptr(), -1, 1, repl.as_ptr()), 0); + assert_eq!(list.extract::>().unwrap(), [9, 2, 3, 4]); + }) + } + + #[test] + fn list_set_slice_self() { + Python::attach(|py| { + let list = PyList::new(py, [1, 2]).unwrap(); + list.set_slice(2, 2, list.as_any()).unwrap(); + assert_eq!(list.extract::>().unwrap(), [1, 2, 1, 2]); + }) + } }