diff --git a/mypyc/lib-rt/list_ops.c b/mypyc/lib-rt/list_ops.c index c0c6ece9da15..99933f2cd1bf 100644 --- a/mypyc/lib-rt/list_ops.c +++ b/mypyc/lib-rt/list_ops.c @@ -259,19 +259,35 @@ void CPyList_SetItemUnsafe(PyObject *list, Py_ssize_t index, PyObject *value) { } #ifdef Py_GIL_DISABLED -// The original optimized list.pop implementation doesn't work on free-threaded -// builds, so provide an alternative that is a bit slower but works. +// The original optimized list.pop implementation may reallocate the item array, +// which doesn't work on free-threaded builds, so provide an alternative that +// never reallocates it (like del a[i] in CPython, the array never shrinks). // -// Note that this implementation isn't intended to be atomic. +// The list is locked so that the item is removed atomically. A negative index +// is relative to the end of the list. static inline PyObject *list_pop_index(PyObject *list, Py_ssize_t index) { - PyObject *item = PyList_GetItemRef(list, index); - if (item == NULL) { - return NULL; + PyObject *item = NULL; + Py_BEGIN_CRITICAL_SECTION(list); + Py_ssize_t size = PyList_GET_SIZE(list); + if (index < 0) { + index += size; } - if (PySequence_DelItem(list, index) < 0) { - Py_DECREF(item); - return NULL; + if (size == 0) { + PyErr_SetString(PyExc_IndexError, "pop from empty list"); + } else if (index < 0 || index >= size) { + PyErr_SetString(PyExc_IndexError, "pop index out of range"); + } else { + // Same steps as del a[i] in CPython (list_ass_item_lock_held), except that + // the list's reference to the item is passed to the caller. Never resize + // the item array here: on free-threaded builds it must be freed using QSBR. + PyObject **items = ((PyListObject *)list)->ob_item; + item = items[index]; + for (Py_ssize_t i = index; i < size - 1; i++) { + _Py_atomic_store_ptr_release(&items[i], items[i + 1]); + } + Py_SET_SIZE(list, size - 1); } + Py_END_CRITICAL_SECTION(); return item; } #endif @@ -280,8 +296,7 @@ PyObject *CPyList_PopLast(PyObject *list) { #ifdef Py_GIL_DISABLED // The other implementation causes segfaults on a free-threaded Python 3.14b4 build. - Py_ssize_t index = PyList_GET_SIZE(list) - 1; - return list_pop_index(list, index); + return list_pop_index(list, -1); #else // I tried a specialized version of pop_impl for just removing the // last element and it wasn't any faster in microbenchmarks than @@ -296,9 +311,6 @@ PyObject *CPyList_Pop(PyObject *obj, CPyTagged index) Py_ssize_t n = CPyTagged_ShortAsSsize_t(index); #ifdef Py_GIL_DISABLED // We must use a slower implementation on free-threaded builds. - if (n < 0) { - n += PyList_GET_SIZE(obj); - } return list_pop_index(obj, n); #else return list_pop_impl((PyListObject *)obj, n); diff --git a/mypyc/test-data/run-lists.test b/mypyc/test-data/run-lists.test index d06fc9ce10b0..8b9934beab39 100644 --- a/mypyc/test-data/run-lists.test +++ b/mypyc/test-data/run-lists.test @@ -262,12 +262,28 @@ def test_pop_index() -> None: assert l.pop(-1) == 1002 assert l == [1001] +def test_pop_many_then_append() -> None: + l = [str(i) for i in range(100)] + for i in range(99, -1, -1): + assert l.pop() == str(i) + assert l == [] + for i in range(100): + l.append(str(i)) + assert l.pop(50) == "50" + assert l.pop(0) == "0" + assert l == [str(i) for i in range(1, 100) if i != 50] + def test_pop_index_errors() -> None: l = [int() + 1000] - with assertRaises(IndexError): + with assertRaises(IndexError, "pop index out of range"): l.pop(1) - with assertRaises(IndexError): + with assertRaises(IndexError, "pop index out of range"): l.pop(-2) + l.pop() + with assertRaises(IndexError, "pop from empty list"): + l.pop() + with assertRaises(IndexError, "pop from empty list"): + l.pop(0) with assertRaises(OverflowError): l.pop(1 << 100) with assertRaises(OverflowError):