From babb787047e0f7807c8238d3b1a3128dac30bd5c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 13 Nov 2023 17:14:56 +0100 Subject: [PATCH] gh-111138: Add PyList_Extend() and PyList_Clear() functions (#111862) * Split list_extend() into two sub-functions: list_extend_fast() and list_extend_iter(). * list_inplace_concat() no longer has to call Py_DECREF() on the list_extend() result, since list_extend() now returns an int. --- Doc/c-api/list.rst | 24 ++ Doc/whatsnew/3.13.rst | 4 + Include/cpython/listobject.h | 3 + Lib/test/test_capi/test_list.py | 77 ++++- ...-11-08-18-37-19.gh-issue-111138.3Ypq8h.rst | 2 + Modules/_testcapi/list.c | 21 ++ Objects/clinic/listobject.c.h | 20 +- Objects/listobject.c | 279 +++++++++++------- 8 files changed, 303 insertions(+), 127 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2023-11-08-18-37-19.gh-issue-111138.3Ypq8h.rst diff --git a/Doc/c-api/list.rst b/Doc/c-api/list.rst index c15cecd41b89d1..c8b64bad702f50 100644 --- a/Doc/c-api/list.rst +++ b/Doc/c-api/list.rst @@ -128,6 +128,30 @@ List Objects list is not supported. +.. c:function:: int PyList_Extend(PyObject *list, PyObject *iterable) + + Extend *list* with the contents of *iterable*. This is the same as + ``PyList_SetSlice(list, PY_SSIZE_T_MAX, PY_SSIZE_T_MAX, iterable)`` + and analogous to ``list.extend(iterable)`` or ``list += iterable``. + + Raise an exception and return ``-1`` if *list* is not a :class:`list` + object. Return 0 on success. + + .. versionadded:: 3.13 + + +.. c:function:: int PyList_Clear(PyObject *list) + + Remove all items from *list*. This is the same as + ``PyList_SetSlice(list, 0, PY_SSIZE_T_MAX, NULL)`` and analogous to + ``list.clear()`` or ``del list[:]``. + + Raise an exception and return ``-1`` if *list* is not a :class:`list` + object. Return 0 on success. + + .. versionadded:: 3.13 + + .. c:function:: int PyList_Sort(PyObject *list) Sort the items of *list* in place. Return ``0`` on success, ``-1`` on diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index ae3c88d28d73a0..9f9239a7eeb036 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -1164,6 +1164,10 @@ New Features :c:func:`PyErr_WriteUnraisable`, but allow to customize the warning mesage. (Contributed by Serhiy Storchaka in :gh:`108082`.) +* Add :c:func:`PyList_Extend` and :c:func:`PyList_Clear` functions: similar to + Python ``list.extend()`` and ``list.clear()`` methods. + (Contributed by Victor Stinner in :gh:`111138`.) + Porting to Python 3.13 ---------------------- diff --git a/Include/cpython/listobject.h b/Include/cpython/listobject.h index c4d9052a09ef42..8ade1b164681f9 100644 --- a/Include/cpython/listobject.h +++ b/Include/cpython/listobject.h @@ -44,3 +44,6 @@ PyList_SET_ITEM(PyObject *op, Py_ssize_t index, PyObject *value) { } #define PyList_SET_ITEM(op, index, value) \ PyList_SET_ITEM(_PyObject_CAST(op), (index), _PyObject_CAST(value)) + +PyAPI_FUNC(int) PyList_Extend(PyObject *self, PyObject *iterable); +PyAPI_FUNC(int) PyList_Clear(PyObject *self); diff --git a/Lib/test/test_capi/test_list.py b/Lib/test/test_capi/test_list.py index 197da03e07fa27..eb03d51d3def37 100644 --- a/Lib/test/test_capi/test_list.py +++ b/Lib/test/test_capi/test_list.py @@ -1,5 +1,6 @@ +import gc +import weakref import unittest -import sys from test.support import import_helper from collections import UserList _testcapi = import_helper.import_module('_testcapi') @@ -12,6 +13,15 @@ class ListSubclass(list): pass +class DelAppend: + def __init__(self, lst, item): + self.lst = lst + self.item = item + + def __del__(self): + self.lst.append(self.item) + + class CAPITest(unittest.TestCase): def test_check(self): # Test PyList_Check() @@ -196,10 +206,10 @@ def test_list_getslice(self): def test_list_setslice(self): # Test PyList_SetSlice() - setslice = _testcapi.list_setslice + list_setslice = _testcapi.list_setslice def set_slice(lst, low, high, value): lst = lst.copy() - self.assertEqual(setslice(lst, low, high, value), 0) + self.assertEqual(list_setslice(lst, low, high, value), 0) return lst # insert items @@ -231,8 +241,21 @@ def set_slice(lst, low, high, value): self.assertEqual(set_slice(lst, 0, len(lst), NULL), []) self.assertEqual(set_slice(lst, 3, len(lst), NULL), list("abc")) - self.assertRaises(SystemError, setslice, (), 0, 0, []) - self.assertRaises(SystemError, setslice, 42, 0, 0, []) + self.assertRaises(SystemError, list_setslice, (), 0, 0, []) + self.assertRaises(SystemError, list_setslice, 42, 0, 0, []) + + # Item finalizer modify the list (clear the list) + lst = [] + lst.append(DelAppend(lst, 'zombie')) + self.assertEqual(list_setslice(lst, 0, len(lst), NULL), 0) + self.assertEqual(lst, ['zombie']) + + # Item finalizer modify the list (remove an list item) + lst = [] + lst.append(DelAppend(lst, 'zombie')) + lst.extend("abc") + self.assertEqual(list_setslice(lst, 0, 1, NULL), 0) + self.assertEqual(lst, ['a', 'b', 'c', 'zombie']) # CRASHES setslice(NULL, 0, 0, []) @@ -275,3 +298,47 @@ def test_list_astuple(self): self.assertRaises(SystemError, astuple, ()) self.assertRaises(SystemError, astuple, object()) self.assertRaises(SystemError, astuple, NULL) + + def test_list_clear(self): + # Test PyList_Clear() + list_clear = _testcapi.list_clear + + lst = [1, 2, 3] + self.assertEqual(list_clear(lst), 0) + self.assertEqual(lst, []) + + lst = [] + self.assertEqual(list_clear(lst), 0) + self.assertEqual(lst, []) + + self.assertRaises(SystemError, list_clear, ()) + self.assertRaises(SystemError, list_clear, object()) + + # Item finalizer modify the list + lst = [] + lst.append(DelAppend(lst, 'zombie')) + list_clear(lst) + self.assertEqual(lst, ['zombie']) + + # CRASHES list_clear(NULL) + + def test_list_extend(self): + # Test PyList_Extend() + list_extend = _testcapi.list_extend + + for other_type in (list, tuple, str, iter): + lst = list("ab") + arg = other_type("def") + self.assertEqual(list_extend(lst, arg), 0) + self.assertEqual(lst, list("abdef")) + + # PyList_Extend(lst, lst) + lst = list("abc") + self.assertEqual(list_extend(lst, lst), 0) + self.assertEqual(lst, list("abcabc")) + + self.assertRaises(TypeError, list_extend, [], object()) + self.assertRaises(SystemError, list_extend, (), list("abc")) + + # CRASHES list_extend(NULL, []) + # CRASHES list_extend([], NULL) diff --git a/Misc/NEWS.d/next/C API/2023-11-08-18-37-19.gh-issue-111138.3Ypq8h.rst b/Misc/NEWS.d/next/C API/2023-11-08-18-37-19.gh-issue-111138.3Ypq8h.rst new file mode 100644 index 00000000000000..15c3b9b3a6b9ad --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-11-08-18-37-19.gh-issue-111138.3Ypq8h.rst @@ -0,0 +1,2 @@ +Add :c:func:`PyList_Extend` and :c:func:`PyList_Clear` functions: similar to +Python ``list.extend()`` and ``list.clear()`` methods. Patch by Victor Stinner. diff --git a/Modules/_testcapi/list.c b/Modules/_testcapi/list.c index 6ba0e7ab27c5d7..10e18699f01bc1 100644 --- a/Modules/_testcapi/list.c +++ b/Modules/_testcapi/list.c @@ -162,6 +162,25 @@ list_astuple(PyObject* Py_UNUSED(module), PyObject *obj) } +static PyObject * +list_clear(PyObject* Py_UNUSED(module), PyObject *obj) +{ + NULLABLE(obj); + RETURN_INT(PyList_Clear(obj)); +} + + +static PyObject * +list_extend(PyObject* Py_UNUSED(module), PyObject *args) +{ + PyObject *obj, *arg; + if (!PyArg_ParseTuple(args, "OO", &obj, &arg)) { + return NULL; + } + NULLABLE(obj); + NULLABLE(arg); + RETURN_INT(PyList_Extend(obj, arg)); +} static PyMethodDef test_methods[] = { @@ -181,6 +200,8 @@ static PyMethodDef test_methods[] = { {"list_sort", list_sort, METH_O}, {"list_reverse", list_reverse, METH_O}, {"list_astuple", list_astuple, METH_O}, + {"list_clear", list_clear, METH_O}, + {"list_extend", list_extend, METH_VARARGS}, {NULL}, }; diff --git a/Objects/clinic/listobject.c.h b/Objects/clinic/listobject.c.h index b2178cd7f5f383..54e6060451f3ff 100644 --- a/Objects/clinic/listobject.c.h +++ b/Objects/clinic/listobject.c.h @@ -50,22 +50,22 @@ list_insert(PyListObject *self, PyObject *const *args, Py_ssize_t nargs) return return_value; } -PyDoc_STRVAR(list_clear__doc__, +PyDoc_STRVAR(py_list_clear__doc__, "clear($self, /)\n" "--\n" "\n" "Remove all items from list."); -#define LIST_CLEAR_METHODDEF \ - {"clear", (PyCFunction)list_clear, METH_NOARGS, list_clear__doc__}, +#define PY_LIST_CLEAR_METHODDEF \ + {"clear", (PyCFunction)py_list_clear, METH_NOARGS, py_list_clear__doc__}, static PyObject * -list_clear_impl(PyListObject *self); +py_list_clear_impl(PyListObject *self); static PyObject * -list_clear(PyListObject *self, PyObject *Py_UNUSED(ignored)) +py_list_clear(PyListObject *self, PyObject *Py_UNUSED(ignored)) { - return list_clear_impl(self); + return py_list_clear_impl(self); } PyDoc_STRVAR(list_copy__doc__, @@ -95,14 +95,14 @@ PyDoc_STRVAR(list_append__doc__, #define LIST_APPEND_METHODDEF \ {"append", (PyCFunction)list_append, METH_O, list_append__doc__}, -PyDoc_STRVAR(list_extend__doc__, +PyDoc_STRVAR(py_list_extend__doc__, "extend($self, iterable, /)\n" "--\n" "\n" "Extend list by appending elements from the iterable."); -#define LIST_EXTEND_METHODDEF \ - {"extend", (PyCFunction)list_extend, METH_O, list_extend__doc__}, +#define PY_LIST_EXTEND_METHODDEF \ + {"extend", (PyCFunction)py_list_extend, METH_O, py_list_extend__doc__}, PyDoc_STRVAR(list_pop__doc__, "pop($self, index=-1, /)\n" @@ -384,4 +384,4 @@ list___reversed__(PyListObject *self, PyObject *Py_UNUSED(ignored)) { return list___reversed___impl(self); } -/*[clinic end generated code: output=5dea9dd3bb219a7f input=a9049054013a1b77]*/ +/*[clinic end generated code: output=f2d7b63119464ff4 input=a9049054013a1b77]*/ diff --git a/Objects/listobject.c b/Objects/listobject.c index af006ef0f61850..2d04218439bd20 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -592,26 +592,33 @@ list_repeat(PyListObject *a, Py_ssize_t n) return (PyObject *) np; } -static int -_list_clear(PyListObject *a) +static void +list_clear(PyListObject *a) { - Py_ssize_t i; - PyObject **item = a->ob_item; - if (item != NULL) { - /* Because XDECREF can recursively invoke operations on - this list, we make it empty first. */ - i = Py_SIZE(a); - Py_SET_SIZE(a, 0); - a->ob_item = NULL; - a->allocated = 0; - while (--i >= 0) { - Py_XDECREF(item[i]); - } - PyMem_Free(item); + PyObject **items = a->ob_item; + if (items == NULL) { + return; + } + + /* Because XDECREF can recursively invoke operations on + this list, we make it empty first. */ + Py_ssize_t i = Py_SIZE(a); + Py_SET_SIZE(a, 0); + a->ob_item = NULL; + a->allocated = 0; + while (--i >= 0) { + Py_XDECREF(items[i]); } - /* Never fails; the return value can be ignored. - Note that there is no guarantee that the list is actually empty - at this point, because XDECREF may have populated it again! */ + PyMem_Free(items); + + // Note that there is no guarantee that the list is actually empty + // at this point, because XDECREF may have populated it indirectly again! +} + +static int +list_clear_slot(PyListObject *self) +{ + list_clear(self); return 0; } @@ -675,7 +682,8 @@ list_ass_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v) d = n - norig; if (Py_SIZE(a) + d == 0) { Py_XDECREF(v_as_SF); - return _list_clear(a); + list_clear(a); + return 0; } item = a->ob_item; /* recycle the items that we are about to remove */ @@ -745,7 +753,7 @@ list_inplace_repeat(PyListObject *self, Py_ssize_t n) } if (n < 1) { - (void)_list_clear(self); + list_clear(self); return Py_NewRef(self); } @@ -801,16 +809,16 @@ list_insert_impl(PyListObject *self, Py_ssize_t index, PyObject *object) } /*[clinic input] -list.clear +list.clear as py_list_clear Remove all items from list. [clinic start generated code]*/ static PyObject * -list_clear_impl(PyListObject *self) -/*[clinic end generated code: output=67a1896c01f74362 input=ca3c1646856742f6]*/ +py_list_clear_impl(PyListObject *self) +/*[clinic end generated code: output=83726743807e3518 input=378711e10f545c53]*/ { - _list_clear(self); + list_clear(self); Py_RETURN_NONE; } @@ -846,83 +854,61 @@ list_append(PyListObject *self, PyObject *object) Py_RETURN_NONE; } -/*[clinic input] -list.extend - - iterable: object - / - -Extend list by appending elements from the iterable. -[clinic start generated code]*/ - -static PyObject * -list_extend(PyListObject *self, PyObject *iterable) -/*[clinic end generated code: output=630fb3bca0c8e789 input=9ec5ba3a81be3a4d]*/ +static int +list_extend_fast(PyListObject *self, PyObject *iterable) { - PyObject *it; /* iter(v) */ - Py_ssize_t m; /* size of self */ - Py_ssize_t n; /* guess for size of iterable */ - Py_ssize_t i; - PyObject *(*iternext)(PyObject *); + Py_ssize_t n = PySequence_Fast_GET_SIZE(iterable); + if (n == 0) { + /* short circuit when iterable is empty */ + return 0; + } - /* Special cases: - 1) lists and tuples which can use PySequence_Fast ops - 2) extending self to self requires making a copy first - */ - if (PyList_CheckExact(iterable) || PyTuple_CheckExact(iterable) || - (PyObject *)self == iterable) { - PyObject **src, **dest; - iterable = PySequence_Fast(iterable, "argument must be iterable"); - if (!iterable) - return NULL; - n = PySequence_Fast_GET_SIZE(iterable); - if (n == 0) { - /* short circuit when iterable is empty */ - Py_DECREF(iterable); - Py_RETURN_NONE; - } - m = Py_SIZE(self); - /* It should not be possible to allocate a list large enough to cause - an overflow on any relevant platform */ - assert(m < PY_SSIZE_T_MAX - n); - if (self->ob_item == NULL) { - if (list_preallocate_exact(self, n) < 0) { - return NULL; - } - Py_SET_SIZE(self, n); - } - else if (list_resize(self, m + n) < 0) { - Py_DECREF(iterable); - return NULL; - } - /* note that we may still have self == iterable here for the - * situation a.extend(a), but the following code works - * in that case too. Just make sure to resize self - * before calling PySequence_Fast_ITEMS. - */ - /* populate the end of self with iterable's items */ - src = PySequence_Fast_ITEMS(iterable); - dest = self->ob_item + m; - for (i = 0; i < n; i++) { - PyObject *o = src[i]; - dest[i] = Py_NewRef(o); + Py_ssize_t m = Py_SIZE(self); + // It should not be possible to allocate a list large enough to cause + // an overflow on any relevant platform. + assert(m < PY_SSIZE_T_MAX - n); + if (self->ob_item == NULL) { + if (list_preallocate_exact(self, n) < 0) { + return -1; } - Py_DECREF(iterable); - Py_RETURN_NONE; + Py_SET_SIZE(self, n); + } + else if (list_resize(self, m + n) < 0) { + return -1; } - it = PyObject_GetIter(iterable); - if (it == NULL) - return NULL; - iternext = *Py_TYPE(it)->tp_iternext; + // note that we may still have self == iterable here for the + // situation a.extend(a), but the following code works + // in that case too. Just make sure to resize self + // before calling PySequence_Fast_ITEMS. + // + // populate the end of self with iterable's items. + PyObject **src = PySequence_Fast_ITEMS(iterable); + PyObject **dest = self->ob_item + m; + for (Py_ssize_t i = 0; i < n; i++) { + PyObject *o = src[i]; + dest[i] = Py_NewRef(o); + } + return 0; +} + +static int +list_extend_iter(PyListObject *self, PyObject *iterable) +{ + PyObject *it = PyObject_GetIter(iterable); + if (it == NULL) { + return -1; + } + PyObject *(*iternext)(PyObject *) = *Py_TYPE(it)->tp_iternext; /* Guess a result list size. */ - n = PyObject_LengthHint(iterable, 8); + Py_ssize_t n = PyObject_LengthHint(iterable, 8); if (n < 0) { Py_DECREF(it); - return NULL; + return -1; } - m = Py_SIZE(self); + + Py_ssize_t m = Py_SIZE(self); if (m > PY_SSIZE_T_MAX - n) { /* m + n overflowed; on the chance that n lied, and there really * is enough room, ignore it. If n was telling the truth, we'll @@ -935,8 +921,10 @@ list_extend(PyListObject *self, PyObject *iterable) } else { /* Make room. */ - if (list_resize(self, m + n) < 0) + if (list_resize(self, m + n) < 0) { goto error; + } + /* Make the list sane again. */ Py_SET_SIZE(self, m); } @@ -953,10 +941,10 @@ list_extend(PyListObject *self, PyObject *iterable) } break; } + if (Py_SIZE(self) < self->allocated) { - /* steals ref */ Py_ssize_t len = Py_SIZE(self); - PyList_SET_ITEM(self, len, item); + PyList_SET_ITEM(self, len, item); // steals item ref Py_SET_SIZE(self, len + 1); } else { @@ -972,28 +960,95 @@ list_extend(PyListObject *self, PyObject *iterable) } Py_DECREF(it); - Py_RETURN_NONE; + return 0; error: Py_DECREF(it); - return NULL; + return -1; +} + + +static int +list_extend(PyListObject *self, PyObject *iterable) +{ + // Special cases: + // 1) lists and tuples which can use PySequence_Fast ops + // 2) extending self to self requires making a copy first + if (PyList_CheckExact(iterable) + || PyTuple_CheckExact(iterable) + || (PyObject *)self == iterable) + { + iterable = PySequence_Fast(iterable, "argument must be iterable"); + if (!iterable) { + return -1; + } + + int res = list_extend_fast(self, iterable); + Py_DECREF(iterable); + return res; + } + else { + return list_extend_iter(self, iterable); + } } + PyObject * _PyList_Extend(PyListObject *self, PyObject *iterable) { - return list_extend(self, iterable); + if (list_extend(self, iterable) < 0) { + return NULL; + } + Py_RETURN_NONE; } + +/*[clinic input] +list.extend as py_list_extend + + iterable: object + / + +Extend list by appending elements from the iterable. +[clinic start generated code]*/ + static PyObject * -list_inplace_concat(PyListObject *self, PyObject *other) +py_list_extend(PyListObject *self, PyObject *iterable) +/*[clinic end generated code: output=b8e0bff0ceae2abd input=9a8376a8633ed3ba]*/ { - PyObject *result; + return _PyList_Extend(self, iterable); +} + - result = list_extend(self, other); - if (result == NULL) - return result; - Py_DECREF(result); +int +PyList_Extend(PyObject *self, PyObject *iterable) +{ + if (!PyList_Check(self)) { + PyErr_BadInternalCall(); + return -1; + } + return list_extend((PyListObject*)self, iterable); +} + + +int +PyList_Clear(PyObject *self) +{ + if (!PyList_Check(self)) { + PyErr_BadInternalCall(); + return -1; + } + list_clear((PyListObject*)self); + return 0; +} + + +static PyObject * +list_inplace_concat(PyListObject *self, PyObject *other) +{ + if (list_extend(self, other) < 0) { + return NULL; + } return Py_NewRef(self); } @@ -1032,7 +1087,8 @@ list_pop_impl(PyListObject *self, Py_ssize_t index) const Py_ssize_t size_after_pop = Py_SIZE(self) - 1; if (size_after_pop == 0) { Py_INCREF(v); - status = _list_clear(self); + list_clear(self); + status = 0; } else { if ((size_after_pop - index) > 0) { @@ -2501,7 +2557,7 @@ list_sort_impl(PyListObject *self, PyObject *keyfunc, int reverse) self->ob_item = saved_ob_item; self->allocated = saved_allocated; if (final_ob_item != NULL) { - /* we cannot use _list_clear() for this because it does not + /* we cannot use list_clear() for this because it does not guarantee that the list is really empty when it returns */ while (--i >= 0) { Py_XDECREF(final_ob_item[i]); @@ -2789,13 +2845,12 @@ list___init___impl(PyListObject *self, PyObject *iterable) /* Empty previous contents */ if (self->ob_item != NULL) { - (void)_list_clear(self); + list_clear(self); } if (iterable != NULL) { - PyObject *rv = list_extend(self, iterable); - if (rv == NULL) + if (list_extend(self, iterable) < 0) { return -1; - Py_DECREF(rv); + } } return 0; } @@ -2849,11 +2904,11 @@ static PyMethodDef list_methods[] = { PyDoc_STR("__getitem__($self, index, /)\n--\n\nReturn self[index].")}, LIST___REVERSED___METHODDEF LIST___SIZEOF___METHODDEF - LIST_CLEAR_METHODDEF + PY_LIST_CLEAR_METHODDEF LIST_COPY_METHODDEF LIST_APPEND_METHODDEF LIST_INSERT_METHODDEF - LIST_EXTEND_METHODDEF + PY_LIST_EXTEND_METHODDEF LIST_POP_METHODDEF LIST_REMOVE_METHODDEF LIST_INDEX_METHODDEF @@ -3124,7 +3179,7 @@ PyTypeObject PyList_Type = { _Py_TPFLAGS_MATCH_SELF | Py_TPFLAGS_SEQUENCE, /* tp_flags */ list___init____doc__, /* tp_doc */ (traverseproc)list_traverse, /* tp_traverse */ - (inquiry)_list_clear, /* tp_clear */ + (inquiry)list_clear_slot, /* tp_clear */ list_richcompare, /* tp_richcompare */ 0, /* tp_weaklistoffset */ list_iter, /* tp_iter */