From e006c7371d8e57db26254792c67292956e88d81d Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 8 Aug 2024 00:47:15 +0200 Subject: [PATCH] gh-105201: Add PyIter_NextItem() (#122331) Return -1 and set an exception on error; return 0 if the iterator is exhausted, and return 1 if the next item was fetched successfully. Prefer this API to PyIter_Next(), which requires the caller to use PyErr_Occurred() to differentiate between iterator exhaustion and errors. Co-authered-by: Irit Katriel --- Doc/c-api/iter.rst | 43 +++++--------- Doc/data/refcounts.dat | 4 ++ Doc/data/stable_abi.dat | 1 + Doc/whatsnew/3.14.rst | 4 ++ Include/abstract.h | 12 +++- Lib/test/test_capi/test_abstract.py | 40 +++++++++++++ Lib/test/test_stable_abi_ctypes.py | 1 + ...-07-27-00-28-35.gh-issue-105201.0-xUWq.rst | 2 + Misc/stable_abi.toml | 2 + Modules/_testcapi/abstract.c | 29 ++++++++++ Objects/abstract.c | 57 +++++++++++++++---- PC/python3dll.c | 1 + 12 files changed, 156 insertions(+), 40 deletions(-) create mode 100644 Misc/NEWS.d/next/C_API/2024-07-27-00-28-35.gh-issue-105201.0-xUWq.rst diff --git a/Doc/c-api/iter.rst b/Doc/c-api/iter.rst index 434d2021cea..bf9df62c6f1 100644 --- a/Doc/c-api/iter.rst +++ b/Doc/c-api/iter.rst @@ -10,7 +10,8 @@ There are two functions specifically for working with iterators. .. c:function:: int PyIter_Check(PyObject *o) Return non-zero if the object *o* can be safely passed to - :c:func:`PyIter_Next`, and ``0`` otherwise. This function always succeeds. + :c:func:`PyIter_NextItem` and ``0`` otherwise. + This function always succeeds. .. c:function:: int PyAIter_Check(PyObject *o) @@ -19,41 +20,27 @@ There are two functions specifically for working with iterators. .. versionadded:: 3.10 +.. c:function:: int PyIter_NextItem(PyObject *iter, PyObject **item) + + Return ``1`` and set *item* to a :term:`strong reference` of the + next value of the iterator *iter* on success. + Return ``0`` and set *item* to ``NULL`` if there are no remaining values. + Return ``-1``, set *item* to ``NULL`` and set an exception on error. + + .. versionadded:: 3.14 + .. c:function:: PyObject* PyIter_Next(PyObject *o) + This is an older version of :c:func:`!PyIter_NextItem`, + which is retained for backwards compatibility. + Prefer :c:func:`PyIter_NextItem`. + Return the next value from the iterator *o*. The object must be an iterator according to :c:func:`PyIter_Check` (it is up to the caller to check this). If there are no remaining values, returns ``NULL`` with no exception set. If an error occurs while retrieving the item, returns ``NULL`` and passes along the exception. -To write a loop which iterates over an iterator, the C code should look -something like this:: - - PyObject *iterator = PyObject_GetIter(obj); - PyObject *item; - - if (iterator == NULL) { - /* propagate error */ - } - - while ((item = PyIter_Next(iterator))) { - /* do something with item */ - ... - /* release reference when done */ - Py_DECREF(item); - } - - Py_DECREF(iterator); - - if (PyErr_Occurred()) { - /* propagate error */ - } - else { - /* continue doing useful work */ - } - - .. c:type:: PySendResult The enum value used to represent different results of :c:func:`PyIter_Send`. diff --git a/Doc/data/refcounts.dat b/Doc/data/refcounts.dat index ccef104eeef..65d48f8bea7 100644 --- a/Doc/data/refcounts.dat +++ b/Doc/data/refcounts.dat @@ -1132,6 +1132,10 @@ PyAIter_Check:PyObject*:o:0: PyIter_Next:PyObject*::+1: PyIter_Next:PyObject*:o:0: +PyIter_NextItem:int::: +PyIter_NextItem:PyObject*:iter:0: +PyIter_NextItem:PyObject**:item:+1: + PyIter_Send:int::: PyIter_Send:PyObject*:iter:0: PyIter_Send:PyObject*:arg:0: diff --git a/Doc/data/stable_abi.dat b/Doc/data/stable_abi.dat index 90ddb3fd821..592e3465824 100644 --- a/Doc/data/stable_abi.dat +++ b/Doc/data/stable_abi.dat @@ -335,6 +335,7 @@ func,PyInterpreterState_GetID,3.7,, func,PyInterpreterState_New,3.2,, func,PyIter_Check,3.8,, func,PyIter_Next,3.2,, +func,PyIter_NextItem,3.14,, func,PyIter_Send,3.10,, data,PyListIter_Type,3.2,, data,PyListRevIter_Type,3.2,, diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst index c989de26fd4..b975f6a4f8a 100644 --- a/Doc/whatsnew/3.14.rst +++ b/Doc/whatsnew/3.14.rst @@ -404,6 +404,10 @@ New Features (Contributed by Victor Stinner in :gh:`119182`.) +* Add :c:func:`PyIter_NextItem` to replace :c:func:`PyIter_Next`, + which has an ambiguous return value. + (Contributed by Irit Katriel and Erlend Aasland in :gh:`105201`.) + Porting to Python 3.14 ---------------------- diff --git a/Include/abstract.h b/Include/abstract.h index f0e49c1afb8..7cfee1332cc 100644 --- a/Include/abstract.h +++ b/Include/abstract.h @@ -397,13 +397,23 @@ PyAPI_FUNC(int) PyIter_Check(PyObject *); This function always succeeds. */ PyAPI_FUNC(int) PyAIter_Check(PyObject *); +#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030e0000 +/* Return 1 and set 'item' to the next item of 'iter' on success. + * Return 0 and set 'item' to NULL when there are no remaining values. + * Return -1, set 'item' to NULL and set an exception on error. + */ +PyAPI_FUNC(int) PyIter_NextItem(PyObject *iter, PyObject **item); +#endif + /* Takes an iterator object and calls its tp_iternext slot, returning the next value. If the iterator is exhausted, this returns NULL without setting an exception. - NULL with an exception means an error occurred. */ + NULL with an exception means an error occurred. + + Prefer PyIter_NextItem() instead. */ PyAPI_FUNC(PyObject *) PyIter_Next(PyObject *); #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000 diff --git a/Lib/test/test_capi/test_abstract.py b/Lib/test/test_capi/test_abstract.py index bc39036e90b..3a8c224126a 100644 --- a/Lib/test/test_capi/test_abstract.py +++ b/Lib/test/test_capi/test_abstract.py @@ -1007,6 +1007,46 @@ class CAPITest(unittest.TestCase): for obj in object(), 1, 'string', []: self.assertEqual(generichash(obj), object.__hash__(obj)) + def run_iter_api_test(self, next_func): + for data in (), [], (1, 2, 3), [1 , 2, 3], "123": + with self.subTest(data=data): + items = [] + it = iter(data) + while (item := next_func(it)) is not None: + items.append(item) + self.assertEqual(items, list(data)) + + class Broken: + def __init__(self): + self.count = 0 + + def __next__(self): + if self.count < 3: + self.count += 1 + return self.count + else: + raise TypeError('bad type') + + it = Broken() + self.assertEqual(next_func(it), 1) + self.assertEqual(next_func(it), 2) + self.assertEqual(next_func(it), 3) + with self.assertRaisesRegex(TypeError, 'bad type'): + next_func(it) + + def test_iter_next(self): + from _testcapi import PyIter_Next + self.run_iter_api_test(PyIter_Next) + # CRASHES PyIter_Next(10) + + def test_iter_nextitem(self): + from _testcapi import PyIter_NextItem + self.run_iter_api_test(PyIter_NextItem) + + regex = "expected.*iterator.*got.*'int'" + with self.assertRaisesRegex(TypeError, regex): + PyIter_NextItem(10) + if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_stable_abi_ctypes.py b/Lib/test/test_stable_abi_ctypes.py index d1d8a967dbe..fedad17621c 100644 --- a/Lib/test/test_stable_abi_ctypes.py +++ b/Lib/test/test_stable_abi_ctypes.py @@ -371,6 +371,7 @@ SYMBOL_NAMES = ( "PyInterpreterState_New", "PyIter_Check", "PyIter_Next", + "PyIter_NextItem", "PyIter_Send", "PyListIter_Type", "PyListRevIter_Type", diff --git a/Misc/NEWS.d/next/C_API/2024-07-27-00-28-35.gh-issue-105201.0-xUWq.rst b/Misc/NEWS.d/next/C_API/2024-07-27-00-28-35.gh-issue-105201.0-xUWq.rst new file mode 100644 index 00000000000..bf5300b1c5d --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2024-07-27-00-28-35.gh-issue-105201.0-xUWq.rst @@ -0,0 +1,2 @@ +Add :c:func:`PyIter_NextItem` to replace :c:func:`PyIter_Next`, which has an +ambiguous return value. Patch by Irit Katriel and Erlend Aasland. diff --git a/Misc/stable_abi.toml b/Misc/stable_abi.toml index 73012193d61..c38671e389a 100644 --- a/Misc/stable_abi.toml +++ b/Misc/stable_abi.toml @@ -2508,3 +2508,5 @@ [function.Py_TYPE] added = '3.14' +[function.PyIter_NextItem] + added = '3.14' diff --git a/Modules/_testcapi/abstract.c b/Modules/_testcapi/abstract.c index b126aee5b97..8c2c7137cdc 100644 --- a/Modules/_testcapi/abstract.c +++ b/Modules/_testcapi/abstract.c @@ -129,6 +129,33 @@ mapping_getoptionalitem(PyObject *self, PyObject *args) } } +static PyObject * +pyiter_next(PyObject *self, PyObject *iter) +{ + PyObject *item = PyIter_Next(iter); + if (item == NULL && !PyErr_Occurred()) { + Py_RETURN_NONE; + } + return item; +} + +static PyObject * +pyiter_nextitem(PyObject *self, PyObject *iter) +{ + PyObject *item; + int rc = PyIter_NextItem(iter, &item); + if (rc < 0) { + assert(PyErr_Occurred()); + assert(item == NULL); + return NULL; + } + assert(!PyErr_Occurred()); + if (item == NULL) { + Py_RETURN_NONE; + } + return item; +} + static PyMethodDef test_methods[] = { {"object_getoptionalattr", object_getoptionalattr, METH_VARARGS}, @@ -138,6 +165,8 @@ static PyMethodDef test_methods[] = { {"mapping_getoptionalitem", mapping_getoptionalitem, METH_VARARGS}, {"mapping_getoptionalitemstring", mapping_getoptionalitemstring, METH_VARARGS}, + {"PyIter_Next", pyiter_next, METH_O}, + {"PyIter_NextItem", pyiter_nextitem, METH_O}, {NULL}, }; diff --git a/Objects/abstract.c b/Objects/abstract.c index afb068718bb..8626584e9bf 100644 --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -2881,7 +2881,50 @@ PyAIter_Check(PyObject *obj) tp->tp_as_async->am_anext != &_PyObject_NextNotImplemented); } +static int +iternext(PyObject *iter, PyObject **item) +{ + iternextfunc tp_iternext = Py_TYPE(iter)->tp_iternext; + if ((*item = tp_iternext(iter))) { + return 1; + } + + PyThreadState *tstate = _PyThreadState_GET(); + /* When the iterator is exhausted it must return NULL; + * a StopIteration exception may or may not be set. */ + if (!_PyErr_Occurred(tstate)) { + return 0; + } + if (_PyErr_ExceptionMatches(tstate, PyExc_StopIteration)) { + _PyErr_Clear(tstate); + return 0; + } + + /* Error case: an exception (different than StopIteration) is set. */ + return -1; +} + +/* Return 1 and set 'item' to the next item of 'iter' on success. + * Return 0 and set 'item' to NULL when there are no remaining values. + * Return -1, set 'item' to NULL and set an exception on error. + */ +int +PyIter_NextItem(PyObject *iter, PyObject **item) +{ + assert(iter != NULL); + assert(item != NULL); + + if (Py_TYPE(iter)->tp_iternext == NULL) { + *item = NULL; + PyErr_Format(PyExc_TypeError, "expected an iterator, got '%T'", iter); + return -1; + } + + return iternext(iter, item); +} + /* Return next item. + * * If an error occurs, return NULL. PyErr_Occurred() will be true. * If the iteration terminates normally, return NULL and clear the * PyExc_StopIteration exception (if it was set). PyErr_Occurred() @@ -2891,17 +2934,9 @@ PyAIter_Check(PyObject *obj) PyObject * PyIter_Next(PyObject *iter) { - PyObject *result; - result = (*Py_TYPE(iter)->tp_iternext)(iter); - if (result == NULL) { - PyThreadState *tstate = _PyThreadState_GET(); - if (_PyErr_Occurred(tstate) - && _PyErr_ExceptionMatches(tstate, PyExc_StopIteration)) - { - _PyErr_Clear(tstate); - } - } - return result; + PyObject *item; + (void)iternext(iter, &item); + return item; } PySendResult diff --git a/PC/python3dll.c b/PC/python3dll.c index aa3c3965908..78bcef155f5 100755 --- a/PC/python3dll.c +++ b/PC/python3dll.c @@ -326,6 +326,7 @@ EXPORT_FUNC(PyInterpreterState_GetID) EXPORT_FUNC(PyInterpreterState_New) EXPORT_FUNC(PyIter_Check) EXPORT_FUNC(PyIter_Next) +EXPORT_FUNC(PyIter_NextItem) EXPORT_FUNC(PyIter_Send) EXPORT_FUNC(PyList_Append) EXPORT_FUNC(PyList_AsTuple)