Skip to content

Commit d4112a2

Browse files
youknowoneyouknowone
committed
Call the Py<PyList> 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>. - 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
1 parent 7f83162 commit d4112a2

1 file changed

Lines changed: 49 additions & 35 deletions

File tree

‎crates/capi/src/listobject.rs‎

Lines changed: 49 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,7 @@ use crate::object::define_py_check;
33
use crate::pystate::with_vm;
44
use crate::util::FfiPtrExt;
55
use core::ffi::c_int;
6-
use rustpython_vm::AsObject;
7-
use rustpython_vm::PyObjectRef;
86
use rustpython_vm::builtins::PyList;
9-
use rustpython_vm::sliceable::{SaturatedSlice, SliceableSequenceMutOp, SliceableSequenceOp};
107

118
define_py_check!(fn PyList_Check, types.list_type);
129
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 {
3330
pub unsafe extern "C" fn PyList_GetItemRef(obj: *mut PyObject, index: isize) -> *mut PyObject {
3431
with_vm(|vm| {
3532
let list = unsafe { obj.assume_borrowed_and_cast::<PyList>(vm) }?;
36-
index
37-
.try_into()
38-
.ok()
39-
.and_then(|index: usize| list.borrow_vec().get(index).map(ToOwned::to_owned))
40-
.ok_or_else(|| vm.new_index_error(format!("list index out of range: {index}")))
33+
// A negative index wraps past the end and is out of range.
34+
list.get_item(index as usize, vm)
4135
})
4236
}
4337

@@ -50,25 +44,17 @@ pub unsafe extern "C" fn PyList_SetItem(
5044
with_vm(|vm| {
5145
let list = unsafe { list.assume_borrowed_and_cast::<PyList>(vm) }?;
5246
let item = unsafe { item.assume_owned() };
53-
let index_error =
54-
|| vm.new_index_error(format!("list assignment index out of range: {index}"));
55-
if index < 0 {
56-
return Err(index_error());
57-
}
58-
59-
let mut list_mut = list.borrow_vec_mut();
60-
match index - list_mut.len() as isize {
61-
..0 => {
62-
list_mut[index as usize] = item;
63-
Ok(())
64-
}
47+
// A negative index wraps past the end and is out of range.
48+
let index = index as usize;
49+
{
50+
let mut list_mut = list.borrow_vec_mut();
6551
// This is somewhat a hack, we assume that we are populating a list right after PyList_New
66-
0 if list_mut.capacity() > index as usize => {
52+
if index == list_mut.len() && list_mut.capacity() > index {
6753
list_mut.push(item);
68-
Ok(())
54+
return Ok(());
6955
}
70-
0.. => Err(index_error()),
7156
}
57+
list.set_item(index, item, vm)
7258
})
7359
}
7460

@@ -121,9 +107,7 @@ pub unsafe extern "C" fn PyList_GetSlice(
121107
) -> *mut PyObject {
122108
with_vm(|vm| {
123109
let list = unsafe { list.assume_borrowed_and_cast::<PyList>(vm) }?;
124-
let vec = list.borrow_vec();
125-
let sliced = vec.getitem_by_slice(vm, SaturatedSlice::from_parts(low, high, 1))?;
126-
Ok(vm.ctx.new_list(sliced))
110+
Ok(list.get_slice(low, high, vm))
127111
})
128112
}
129113

@@ -136,31 +120,26 @@ pub unsafe extern "C" fn PyList_SetSlice(
136120
) -> c_int {
137121
with_vm(|vm| {
138122
let list = unsafe { list.assume_borrowed_and_cast::<PyList>(vm) }?;
139-
let slice = SaturatedSlice::from_parts(low, high, 1);
140-
let mut vec = list.borrow_vec_mut();
141-
142123
let Some(itemlist) = (unsafe { itemlist.assume_borrowed_or_opt() }) else {
143-
vec.delitem_by_slice(vm, slice)?;
124+
list.del_slice(low, high);
144125
return Ok(());
145126
};
146-
147-
let items: Vec<PyObjectRef> = itemlist.try_to_value(vm)?;
148-
vec.setitem_by_slice(vm, slice, &items)
127+
list.set_slice(low, high, itemlist, vm)
149128
})
150129
}
151130

152131
#[unsafe(no_mangle)]
153132
pub unsafe extern "C" fn PyList_Sort(list: *mut PyObject) -> c_int {
154133
with_vm(|vm| {
155134
let list = unsafe { list.assume_borrowed_and_cast::<PyList>(vm) }?;
156-
vm.call_method(list.as_object(), "sort", ())?;
157-
Ok(())
135+
list.sort(vm)
158136
})
159137
}
160138

161139
#[cfg(test)]
162140
mod tests {
163141
use pyo3::exceptions::PyIndexError;
142+
use pyo3::ffi;
164143
use pyo3::prelude::*;
165144
use pyo3::types::{PyList, PyListMethods};
166145

@@ -281,4 +260,39 @@ mod tests {
281260
assert_eq!(list.get_item(2).unwrap().extract::<u32>().unwrap(), 3);
282261
})
283262
}
263+
264+
#[test]
265+
fn list_del_slice() {
266+
Python::attach(|py| {
267+
let list = PyList::new(py, [1, 2, 3, 4]).unwrap();
268+
list.del_slice(1, 3).unwrap();
269+
assert_eq!(list.extract::<Vec<u32>>().unwrap(), [1, 4]);
270+
})
271+
}
272+
273+
#[test]
274+
fn list_negative_indices() {
275+
Python::attach(|py| unsafe {
276+
let list = PyList::new(py, [1, 2, 3, 4]).unwrap();
277+
278+
assert!(ffi::PyList_GetItemRef(list.as_ptr(), -1).is_null());
279+
assert!(PyErr::take(py).unwrap().is_instance_of::<PyIndexError>(py));
280+
281+
let slice = Bound::from_owned_ptr(py, ffi::PyList_GetSlice(list.as_ptr(), -1, 2));
282+
assert_eq!(slice.extract::<Vec<u32>>().unwrap(), [1, 2]);
283+
284+
let repl = PyList::new(py, [9]).unwrap();
285+
assert_eq!(ffi::PyList_SetSlice(list.as_ptr(), -1, 1, repl.as_ptr()), 0);
286+
assert_eq!(list.extract::<Vec<u32>>().unwrap(), [9, 2, 3, 4]);
287+
})
288+
}
289+
290+
#[test]
291+
fn list_set_slice_self() {
292+
Python::attach(|py| {
293+
let list = PyList::new(py, [1, 2]).unwrap();
294+
list.set_slice(2, 2, list.as_any()).unwrap();
295+
assert_eq!(list.extract::<Vec<u32>>().unwrap(), [1, 2, 1, 2]);
296+
})
297+
}
284298
}

0 commit comments

Comments
 (0)