From 8f96d08971fe7a491bf216d9e1ba522ba6f2f7ba Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 23 Jan 2024 21:46:46 +0000 Subject: [PATCH 1/5] gh-114329: Add `PyList_GetItemRef` function The new `PyList_GetItemRef` is similar to `PyList_GetItem`, but returns a strong reference instead of a borrowed reference. Additionally, if the passed "list" object is not a list, the function sets a `TypeError` instead of calling `PyErr_BadInternalCall()`. --- Doc/c-api/list.rst | 15 +++++++++++++++ Doc/data/stable_abi.dat | 1 + Doc/whatsnew/3.13.rst | 4 ++++ Include/listobject.h | 1 + Lib/test/test_capi/test_list.py | 9 +++++++++ ...2024-01-23-21-45-02.gh-issue-114329.YRaBoe.rst | 3 +++ Misc/stable_abi.toml | 2 ++ Modules/_testcapi/list.c | 13 +++++++++++++ Objects/listobject.c | 15 +++++++++++++++ PC/python3dll.c | 1 + 10 files changed, 64 insertions(+) create mode 100644 Misc/NEWS.d/next/C API/2024-01-23-21-45-02.gh-issue-114329.YRaBoe.rst diff --git a/Doc/c-api/list.rst b/Doc/c-api/list.rst index c8b64bad702f50..a2f9b86187b86e 100644 --- a/Doc/c-api/list.rst +++ b/Doc/c-api/list.rst @@ -64,6 +64,21 @@ List Objects return ``NULL`` and set an :exc:`IndexError` exception. +.. c:function:: PyObject* PyList_GetItemRef(PyObject *list, Py_ssize_t index) + + Return a :term:`strong reference` to the object at position *index* in the + list pointed to by *list*. The position must be non-negative; indexing from + the end of the list is not supported. If *index* is out of bounds + (<0 or >=len(list)), return ``NULL`` and set an :exc:`IndexError` exception. + If *list* is not a :class:`list` object, return ``NULL`` and set a + :exc:`TypeError` exception. + + This behaves like :c:func:`PyList_GetItem`, but returns a + :term:`strong reference` instead of a :term:`borrowed reference`. + + .. versionadded:: 3.13 + + .. c:function:: PyObject* PyList_GET_ITEM(PyObject *list, Py_ssize_t i) Similar to :c:func:`PyList_GetItem`, but without error checking. diff --git a/Doc/data/stable_abi.dat b/Doc/data/stable_abi.dat index 811b1bd84d2417..1022171aff271f 100644 --- a/Doc/data/stable_abi.dat +++ b/Doc/data/stable_abi.dat @@ -335,6 +335,7 @@ var,PyListRevIter_Type,3.2,, function,PyList_Append,3.2,, function,PyList_AsTuple,3.2,, function,PyList_GetItem,3.2,, +function,PyList_GetItemRef,3.13,, function,PyList_GetSlice,3.2,, function,PyList_Insert,3.2,, function,PyList_New,3.2,, diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 40f0cd37fe9318..acc02f8b62b80f 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -1305,6 +1305,10 @@ New Features UTF-8 encoded bytes string, rather than a :c:expr:`PyObject*`. (Contributed by Victor Stinner in :gh:`108314`.) +* Added :c:func:`PyList_GetItemRef` function: similar to + :c:func:`PyList_GetItem` but returns a :term:`strong reference` instead of + a :term:`borrowed reference`. + * Add :c:func:`Py_IsFinalizing` function: check if the main Python interpreter is :term:`shutting down `. (Contributed by Victor Stinner in :gh:`108014`.) diff --git a/Include/listobject.h b/Include/listobject.h index 6b7041ba0b05d5..4e4084b43483a2 100644 --- a/Include/listobject.h +++ b/Include/listobject.h @@ -29,6 +29,7 @@ PyAPI_FUNC(PyObject *) PyList_New(Py_ssize_t size); PyAPI_FUNC(Py_ssize_t) PyList_Size(PyObject *); PyAPI_FUNC(PyObject *) PyList_GetItem(PyObject *, Py_ssize_t); +PyAPI_FUNC(PyObject *) PyList_GetItemRef(PyObject *, Py_ssize_t); PyAPI_FUNC(int) PyList_SetItem(PyObject *, Py_ssize_t, PyObject *); PyAPI_FUNC(int) PyList_Insert(PyObject *, Py_ssize_t, PyObject *); PyAPI_FUNC(int) PyList_Append(PyObject *, PyObject *); diff --git a/Lib/test/test_capi/test_list.py b/Lib/test/test_capi/test_list.py index eb03d51d3def37..116ca366c34135 100644 --- a/Lib/test/test_capi/test_list.py +++ b/Lib/test/test_capi/test_list.py @@ -112,6 +112,15 @@ def test_list_get_item(self): # CRASHES get_item(21, 2) # CRASHES get_item(NULL, 1) + def test_list_get_item_ref(self): + # Test PyList_GetItemRef() + get_item_ref = _testcapi.list_get_item_ref + lst = [1, 2, [1, 2, 3]] + self.assertEqual(get_item_ref(lst, 0), 1) + self.assertEqual(get_item_ref(lst, 2), [1, 2, 3]) + self.assertRaises(IndexError, get_item_ref, lst, 3) + self.assertRaises(IndexError, get_item_ref, lst, -1) + self.assertRaises(TypeError, get_item_ref, "not a list", 0) def test_list_setitem(self): # Test PyList_SetItem() diff --git a/Misc/NEWS.d/next/C API/2024-01-23-21-45-02.gh-issue-114329.YRaBoe.rst b/Misc/NEWS.d/next/C API/2024-01-23-21-45-02.gh-issue-114329.YRaBoe.rst new file mode 100644 index 00000000000000..62d4ce0cfb8de5 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2024-01-23-21-45-02.gh-issue-114329.YRaBoe.rst @@ -0,0 +1,3 @@ +Add :c:func:`PyList_GetItemRef`, which is similar to +:c:func:`PyList_GetItem` but returns a :term:`strong reference` instead of a +:term:`borrowed reference`. diff --git a/Misc/stable_abi.toml b/Misc/stable_abi.toml index 22b25dd0ec141f..64b7262eb265fa 100644 --- a/Misc/stable_abi.toml +++ b/Misc/stable_abi.toml @@ -905,6 +905,8 @@ added = '3.2' [function.PyList_GetItem] added = '3.2' +[function.PyList_GetItemRef] + added = '3.13' [function.PyList_GetSlice] added = '3.2' [function.PyList_Insert] diff --git a/Modules/_testcapi/list.c b/Modules/_testcapi/list.c index 10e18699f01bc1..2cb6499e28336d 100644 --- a/Modules/_testcapi/list.c +++ b/Modules/_testcapi/list.c @@ -59,6 +59,18 @@ list_get_item(PyObject *Py_UNUSED(module), PyObject *args) return Py_XNewRef(PyList_GET_ITEM(obj, i)); } +static PyObject * +list_get_item_ref(PyObject *Py_UNUSED(module), PyObject *args) +{ + PyObject *obj; + Py_ssize_t i; + if (!PyArg_ParseTuple(args, "On", &obj, &i)) { + return NULL; + } + NULLABLE(obj); + return PyList_GetItemRef(obj, i); +} + static PyObject * list_setitem(PyObject *Py_UNUSED(module), PyObject *args) { @@ -191,6 +203,7 @@ static PyMethodDef test_methods[] = { {"list_get_size", list_get_size, METH_O}, {"list_getitem", list_getitem, METH_VARARGS}, {"list_get_item", list_get_item, METH_VARARGS}, + {"list_get_item_ref", list_get_item_ref, METH_VARARGS}, {"list_setitem", list_setitem, METH_VARARGS}, {"list_set_item", list_set_item, METH_VARARGS}, {"list_insert", list_insert, METH_VARARGS}, diff --git a/Objects/listobject.c b/Objects/listobject.c index 401d1026133f4e..c82d0f461ac90d 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -253,6 +253,21 @@ PyList_GetItem(PyObject *op, Py_ssize_t i) return ((PyListObject *)op) -> ob_item[i]; } +PyObject * +PyList_GetItemRef(PyObject *op, Py_ssize_t i) +{ + if (!PyList_Check(op)) { + PyErr_SetString(PyExc_TypeError, "expected a list"); + return NULL; + } + if (!valid_index(i, Py_SIZE(op))) { + _Py_DECLARE_STR(list_err, "list index out of range"); + PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); + return NULL; + } + return Py_NewRef(PyList_GET_ITEM(op, i)); +} + int PyList_SetItem(PyObject *op, Py_ssize_t i, PyObject *newitem) diff --git a/PC/python3dll.c b/PC/python3dll.c index 07aa84c91f9fc7..a2cf0c7510d361 100755 --- a/PC/python3dll.c +++ b/PC/python3dll.c @@ -324,6 +324,7 @@ EXPORT_FUNC(PyIter_Send) EXPORT_FUNC(PyList_Append) EXPORT_FUNC(PyList_AsTuple) EXPORT_FUNC(PyList_GetItem) +EXPORT_FUNC(PyList_GetItemRef) EXPORT_FUNC(PyList_GetSlice) EXPORT_FUNC(PyList_Insert) EXPORT_FUNC(PyList_New) From 8c92a40e47287f36a6ed45a9ca1acb0dcddfe51e Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 23 Jan 2024 21:52:06 +0000 Subject: [PATCH 2/5] Run regen-limited-abi --- Lib/test/test_stable_abi_ctypes.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_stable_abi_ctypes.py b/Lib/test/test_stable_abi_ctypes.py index 4976ac3642bbe4..203e4588b0c1bc 100644 --- a/Lib/test/test_stable_abi_ctypes.py +++ b/Lib/test/test_stable_abi_ctypes.py @@ -364,6 +364,7 @@ def test_windows_feature_macros(self): "PyList_Append", "PyList_AsTuple", "PyList_GetItem", + "PyList_GetItemRef", "PyList_GetSlice", "PyList_Insert", "PyList_New", From 1761633cb7f259adc9e7b32be8547196dbccc3ee Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 23 Jan 2024 21:59:06 +0000 Subject: [PATCH 3/5] Add PyList_GetItemRef to refcounts.dat --- Doc/data/refcounts.dat | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Doc/data/refcounts.dat b/Doc/data/refcounts.dat index f719ce153b239a..62a96146d605ff 100644 --- a/Doc/data/refcounts.dat +++ b/Doc/data/refcounts.dat @@ -1133,6 +1133,10 @@ PyList_GetItem:PyObject*::0: PyList_GetItem:PyObject*:list:0: PyList_GetItem:Py_ssize_t:index:: +PyList_GetItemRef:PyObject*::+1: +PyList_GetItemRef:PyObject*:list:0: +PyList_GetItemRef:Py_ssize_t:index:: + PyList_GetSlice:PyObject*::+1: PyList_GetSlice:PyObject*:list:0: PyList_GetSlice:Py_ssize_t:low:: From b91363f838de276402eb5ea1d523a62a89cb4640 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 30 Jan 2024 17:27:22 +0000 Subject: [PATCH 4/5] Changes from review --- Doc/c-api/list.rst | 20 +++++++++----------- Lib/test/test_capi/test_list.py | 31 +++++++++++++------------------ Misc/stable_abi.toml | 4 ++-- 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/Doc/c-api/list.rst b/Doc/c-api/list.rst index a2f9b86187b86e..d368966e1b4bdb 100644 --- a/Doc/c-api/list.rst +++ b/Doc/c-api/list.rst @@ -56,27 +56,25 @@ List Objects Similar to :c:func:`PyList_Size`, but without error checking. -.. c:function:: PyObject* PyList_GetItem(PyObject *list, Py_ssize_t index) +.. c:function:: PyObject* PyList_GetItemRef(PyObject *list, Py_ssize_t index) Return the object at position *index* in the list pointed to by *list*. The position must be non-negative; indexing from the end of the list is not supported. If *index* is out of bounds (<0 or >=len(list)), return ``NULL`` and set an :exc:`IndexError` exception. + .. versionadded:: 3.13 -.. c:function:: PyObject* PyList_GetItemRef(PyObject *list, Py_ssize_t index) - Return a :term:`strong reference` to the object at position *index* in the - list pointed to by *list*. The position must be non-negative; indexing from - the end of the list is not supported. If *index* is out of bounds - (<0 or >=len(list)), return ``NULL`` and set an :exc:`IndexError` exception. - If *list* is not a :class:`list` object, return ``NULL`` and set a - :exc:`TypeError` exception. +.. c:function:: PyObject* PyList_GetItem(PyObject *list, Py_ssize_t index) - This behaves like :c:func:`PyList_GetItem`, but returns a - :term:`strong reference` instead of a :term:`borrowed reference`. + Return the object at position *index* in the list pointed to by *list*. The + position must be non-negative; indexing from the end of the list is not + supported. If *index* is out of bounds (<0 or >=len(list)), + return ``NULL`` and set an :exc:`IndexError` exception. - .. versionadded:: 3.13 + This behaves like :c:func:`PyList_GetItemRef`, but returns a + :term:`borrowed reference` instead of a :term:`strong reference`. .. c:function:: PyObject* PyList_GET_ITEM(PyObject *list, Py_ssize_t i) diff --git a/Lib/test/test_capi/test_list.py b/Lib/test/test_capi/test_list.py index 116ca366c34135..dceb4fce3c077b 100644 --- a/Lib/test/test_capi/test_list.py +++ b/Lib/test/test_capi/test_list.py @@ -82,10 +82,8 @@ def test_list_get_size(self): # CRASHES size(UserList()) # CRASHES size(NULL) - - def test_list_getitem(self): - # Test PyList_GetItem() - getitem = _testcapi.list_getitem + def check_list_get_item(self, getitem, exctype): + # Common test cases for PyList_GetItem() and PyList_GetItemRef() lst = [1, 2, 3] self.assertEqual(getitem(lst, 0), 1) self.assertEqual(getitem(lst, 2), 3) @@ -93,12 +91,19 @@ def test_list_getitem(self): self.assertRaises(IndexError, getitem, lst, -1) self.assertRaises(IndexError, getitem, lst, PY_SSIZE_T_MIN) self.assertRaises(IndexError, getitem, lst, PY_SSIZE_T_MAX) - self.assertRaises(SystemError, getitem, 42, 1) - self.assertRaises(SystemError, getitem, (1, 2, 3), 1) - self.assertRaises(SystemError, getitem, {1: 2}, 1) - + self.assertRaises(exctype, getitem, 42, 1) + self.assertRaises(exctype, getitem, (1, 2, 3), 1) + self.assertRaises(exctype, getitem, {1: 2}, 1) # CRASHES getitem(NULL, 1) + def test_list_getitem(self): + # Test PyList_GetItem() + self.check_list_get_item(_testcapi.list_getitem, SystemError) + + def test_list_get_item_ref(self): + # Test PyList_GetItemRef() + self.check_list_get_item(_testcapi.list_get_item_ref, TypeError) + def test_list_get_item(self): # Test PyList_GET_ITEM() get_item = _testcapi.list_get_item @@ -112,16 +117,6 @@ def test_list_get_item(self): # CRASHES get_item(21, 2) # CRASHES get_item(NULL, 1) - def test_list_get_item_ref(self): - # Test PyList_GetItemRef() - get_item_ref = _testcapi.list_get_item_ref - lst = [1, 2, [1, 2, 3]] - self.assertEqual(get_item_ref(lst, 0), 1) - self.assertEqual(get_item_ref(lst, 2), [1, 2, 3]) - self.assertRaises(IndexError, get_item_ref, lst, 3) - self.assertRaises(IndexError, get_item_ref, lst, -1) - self.assertRaises(TypeError, get_item_ref, "not a list", 0) - def test_list_setitem(self): # Test PyList_SetItem() setitem = _testcapi.list_setitem diff --git a/Misc/stable_abi.toml b/Misc/stable_abi.toml index 64b7262eb265fa..adab73b6b83366 100644 --- a/Misc/stable_abi.toml +++ b/Misc/stable_abi.toml @@ -905,8 +905,6 @@ added = '3.2' [function.PyList_GetItem] added = '3.2' -[function.PyList_GetItemRef] - added = '3.13' [function.PyList_GetSlice] added = '3.2' [function.PyList_Insert] @@ -2483,3 +2481,5 @@ [function._Py_SetRefcnt] added = '3.13' abi_only = true +[function.PyList_GetItemRef] + added = '3.13' From dae4945091297bfce4d8f21021c36a35f92fc098 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 1 Feb 2024 13:04:29 -0500 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- Doc/c-api/list.rst | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/Doc/c-api/list.rst b/Doc/c-api/list.rst index d368966e1b4bdb..53eb54d3e1021a 100644 --- a/Doc/c-api/list.rst +++ b/Doc/c-api/list.rst @@ -60,7 +60,7 @@ List Objects Return the object at position *index* in the list pointed to by *list*. The position must be non-negative; indexing from the end of the list is not - supported. If *index* is out of bounds (<0 or >=len(list)), + supported. If *index* is out of bounds (:code:`<0 or >=len(list)`), return ``NULL`` and set an :exc:`IndexError` exception. .. versionadded:: 3.13 @@ -68,12 +68,7 @@ List Objects .. c:function:: PyObject* PyList_GetItem(PyObject *list, Py_ssize_t index) - Return the object at position *index* in the list pointed to by *list*. The - position must be non-negative; indexing from the end of the list is not - supported. If *index* is out of bounds (<0 or >=len(list)), - return ``NULL`` and set an :exc:`IndexError` exception. - - This behaves like :c:func:`PyList_GetItemRef`, but returns a + Like :c:func:`PyList_GetItemRef`, but returns a :term:`borrowed reference` instead of a :term:`strong reference`.