From 347d4783f7721bec4b6860de1cbdc5af98e31bf2 Mon Sep 17 00:00:00 2001 From: Nicolas Tessore Date: Tue, 23 May 2023 00:03:41 +0100 Subject: [PATCH 1/8] let generator.close() return value --- Doc/reference/expressions.rst | 12 +++-- Lib/test/test_generators.py | 82 +++++++++++++++++++++++++++++++++++ Objects/genobject.c | 30 +++++++++++-- 3 files changed, 118 insertions(+), 6 deletions(-) diff --git a/Doc/reference/expressions.rst b/Doc/reference/expressions.rst index b97a08f25d92a2..d6242aded1aeb3 100644 --- a/Doc/reference/expressions.rst +++ b/Doc/reference/expressions.rst @@ -595,13 +595,19 @@ is already executing raises a :exc:`ValueError` exception. .. method:: generator.close() Raises a :exc:`GeneratorExit` at the point where the generator function was - paused. If the generator function then exits gracefully, is already closed, - or raises :exc:`GeneratorExit` (by not catching the exception), close - returns to its caller. If the generator yields a value, a + paused. If the generator function then exits gracefully, its return value + is returned by :meth:`close`. If the generator function is already closed, + or raises :exc:`GeneratorExit` (by not catching the exception), + :meth:`close` returns to its caller. If the generator yields a value, a :exc:`RuntimeError` is raised. If the generator raises any other exception, it is propagated to the caller. :meth:`close` does nothing if the generator has already exited due to an exception or normal exit. + .. versionchanged:: 3.13 + + If a generator exits gracefully, its return value is returned by + :meth:`close`. + .. index:: single: yield; examples Examples diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index 31680b5a92e0f3..a8a344ab8de48d 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -451,6 +451,88 @@ def g(): self.assertEqual(cm.exception.value.value, 2) +class GeneratorCloseTest(unittest.TestCase): + + def test_close_no_return_value(self): + def f(): + yield + + gen = f() + gen.send(None) + self.assertIsNone(gen.close()) + + def test_close_return_value(self): + def f(): + try: + yield + # close() raises GeneratorExit here, which is caught + except GeneratorExit: + return 0 + + gen = f() + gen.send(None) + self.assertEqual(gen.close(), 0) + + def test_close_not_catching_exit(self): + def f(): + yield + # close() raises GeneratorExit here, which isn't caught and + # therefore propagates -- no return value + return 0 + + gen = f() + gen.send(None) + self.assertIsNone(gen.close()) + + def test_close_not_started(self): + def f(): + try: + yield + except GeneratorExit: + return 0 + + gen = f() + self.assertIsNone(gen.close()) + + def test_close_exhausted(self): + def f(): + try: + yield + except GeneratorExit: + return 0 + + gen = f() + next(gen) + with self.assertRaises(StopIteration): + next(gen) + self.assertIsNone(gen.close()) + + def test_close_closed(self): + def f(): + try: + yield + except GeneratorExit: + return 0 + + gen = f() + gen.send(None) + self.assertEqual(gen.close(), 0) + self.assertIsNone(gen.close()) + + def test_close_raises(self): + def f(): + try: + yield + except GeneratorExit: + pass + raise RuntimeError + + gen = f() + gen.send(None) + with self.assertRaises(RuntimeError): + gen.close() + + class GeneratorThrowTest(unittest.TestCase): def test_exception_context_with_yield(self): diff --git a/Objects/genobject.c b/Objects/genobject.c index 9252c654934565..9e1c66226cdf39 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -408,9 +408,33 @@ gen_close(PyGenObject *gen, PyObject *args) PyErr_SetString(PyExc_RuntimeError, msg); return NULL; } - if (PyErr_ExceptionMatches(PyExc_StopIteration) - || PyErr_ExceptionMatches(PyExc_GeneratorExit)) { - PyErr_Clear(); /* ignore these errors */ + if (PyErr_ExceptionMatches(PyExc_StopIteration)) { + /* retrieve the StopIteration exception instance being handled, and + * extract its value */ + PyObject *exc, *args, *value; + PyThreadState *tstate = _PyThreadState_GET(); + if (tstate == NULL) { + PyErr_Clear(); + Py_RETURN_NONE; + } + exc = tstate->current_exception; + if (exc == NULL || !PyExceptionInstance_Check(exc)) { + PyErr_Clear(); + Py_RETURN_NONE; + } + args = ((PyBaseExceptionObject*)exc)->args; + if (args == NULL || !PyTuple_Check(args) + || PyTuple_GET_SIZE(args) == 0) { + PyErr_Clear(); + Py_RETURN_NONE; + } + value = PyTuple_GET_ITEM(args, 0); + Py_INCREF(value); + PyErr_Clear(); + return value; + } + if (PyErr_ExceptionMatches(PyExc_GeneratorExit)) { + PyErr_Clear(); /* ignore this error */ Py_RETURN_NONE; } return NULL; From cd94e7f68d9cbc58dd2c75ac8d3a5e9ebd4f8d48 Mon Sep 17 00:00:00 2001 From: Nicolas Tessore Date: Tue, 23 May 2023 00:39:33 +0100 Subject: [PATCH 2/8] add NEWS.d entry --- Misc/NEWS.d/next/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst diff --git a/Misc/NEWS.d/next/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst b/Misc/NEWS.d/next/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst new file mode 100644 index 00000000000000..15acdb2a045d8d --- /dev/null +++ b/Misc/NEWS.d/next/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst @@ -0,0 +1,2 @@ +Let :meth:`generator.close` forward the return value of a generator function +on graceful exit. From f134f7fb671102521312902a87d42c64f5797f32 Mon Sep 17 00:00:00 2001 From: Nicolas Tessore Date: Tue, 23 May 2023 00:43:01 +0100 Subject: [PATCH 3/8] move NEWS.d entry --- .../2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Misc/NEWS.d/next/{ => Core and Builtins}/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst (100%) diff --git a/Misc/NEWS.d/next/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst similarity index 100% rename from Misc/NEWS.d/next/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst rename to Misc/NEWS.d/next/Core and Builtins/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst From 2175dc266dc7945b79783822bba88ab649b8ba8b Mon Sep 17 00:00:00 2001 From: Nicolas Tessore Date: Tue, 23 May 2023 08:38:11 +0100 Subject: [PATCH 4/8] use PyErr_GetRaisedException() and improve wording of docs --- Doc/reference/expressions.rst | 19 ++++++++++--------- ...-05-23-00-36-02.gh-issue-104770.poSkyY.rst | 4 ++-- Objects/genobject.c | 13 ++++--------- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/Doc/reference/expressions.rst b/Doc/reference/expressions.rst index d6242aded1aeb3..1d02c752b84a18 100644 --- a/Doc/reference/expressions.rst +++ b/Doc/reference/expressions.rst @@ -595,18 +595,19 @@ is already executing raises a :exc:`ValueError` exception. .. method:: generator.close() Raises a :exc:`GeneratorExit` at the point where the generator function was - paused. If the generator function then exits gracefully, its return value - is returned by :meth:`close`. If the generator function is already closed, - or raises :exc:`GeneratorExit` (by not catching the exception), - :meth:`close` returns to its caller. If the generator yields a value, a - :exc:`RuntimeError` is raised. If the generator raises any other exception, - it is propagated to the caller. :meth:`close` does nothing if the generator - has already exited due to an exception or normal exit. + paused. If the generator function then returns a value (by catching + :exc:`GeneratorExit`), it is returned by :meth:`close`. If the generator + function is already closed, or raises :exc:`GeneratorExit` (by not catching + the exception), :meth:`close` returns :const:`None`. If the generator + yields a value, a :exc:`RuntimeError` is raised. If the generator raises + any other exception, it is propagated to the caller. If the generator has + already exited due to an exception or normal exit, :meth:`close` returns + :const:`None`. .. versionchanged:: 3.13 - If a generator exits gracefully, its return value is returned by - :meth:`close`. + If a generator returns a value after being closed, the value is returned + by :meth:`close`. .. index:: single: yield; examples diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst index 15acdb2a045d8d..14e709cbaaeae0 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst @@ -1,2 +1,2 @@ -Let :meth:`generator.close` forward the return value of a generator function -on graceful exit. +If a generator returns a value after being closed, the value is returned +by :meth:`generator.close`. diff --git a/Objects/genobject.c b/Objects/genobject.c index 9e1c66226cdf39..e091f24d10af1f 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -412,25 +412,20 @@ gen_close(PyGenObject *gen, PyObject *args) /* retrieve the StopIteration exception instance being handled, and * extract its value */ PyObject *exc, *args, *value; - PyThreadState *tstate = _PyThreadState_GET(); - if (tstate == NULL) { - PyErr_Clear(); - Py_RETURN_NONE; - } - exc = tstate->current_exception; + exc = PyErr_GetRaisedException(); /* clears the error indicator */ if (exc == NULL || !PyExceptionInstance_Check(exc)) { - PyErr_Clear(); + Py_XDECREF(exc); Py_RETURN_NONE; } args = ((PyBaseExceptionObject*)exc)->args; if (args == NULL || !PyTuple_Check(args) || PyTuple_GET_SIZE(args) == 0) { - PyErr_Clear(); + Py_DECREF(exc); Py_RETURN_NONE; } value = PyTuple_GET_ITEM(args, 0); Py_INCREF(value); - PyErr_Clear(); + Py_DECREF(exc); return value; } if (PyErr_ExceptionMatches(PyExc_GeneratorExit)) { From ecfaa1fcc0c992f287153ee2a33795cfa031187a Mon Sep 17 00:00:00 2001 From: Nicolas Tessore Date: Tue, 23 May 2023 10:19:50 +0100 Subject: [PATCH 5/8] Apply suggestions from code review Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> --- Objects/genobject.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index e091f24d10af1f..e257d89298f938 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -411,20 +411,8 @@ gen_close(PyGenObject *gen, PyObject *args) if (PyErr_ExceptionMatches(PyExc_StopIteration)) { /* retrieve the StopIteration exception instance being handled, and * extract its value */ - PyObject *exc, *args, *value; - exc = PyErr_GetRaisedException(); /* clears the error indicator */ - if (exc == NULL || !PyExceptionInstance_Check(exc)) { - Py_XDECREF(exc); - Py_RETURN_NONE; - } - args = ((PyBaseExceptionObject*)exc)->args; - if (args == NULL || !PyTuple_Check(args) - || PyTuple_GET_SIZE(args) == 0) { - Py_DECREF(exc); - Py_RETURN_NONE; - } - value = PyTuple_GET_ITEM(args, 0); - Py_INCREF(value); + PyObject *exc = PyErr_GetRaisedException(); /* clears the error indicator */ + PyObject *value = Py_NewRef(((PyStopIterationObject *)exc)->value); Py_DECREF(exc); return value; } From 678332a88317ef2436b83e47bd736ddbb48247c4 Mon Sep 17 00:00:00 2001 From: Nicolas Tessore Date: Tue, 23 May 2023 15:48:18 +0100 Subject: [PATCH 6/8] use _PyGen_FetchStopIterationValue() --- Objects/genobject.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index e257d89298f938..beede1e737a49d 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -409,11 +409,10 @@ gen_close(PyGenObject *gen, PyObject *args) return NULL; } if (PyErr_ExceptionMatches(PyExc_StopIteration)) { - /* retrieve the StopIteration exception instance being handled, and - * extract its value */ - PyObject *exc = PyErr_GetRaisedException(); /* clears the error indicator */ - PyObject *value = Py_NewRef(((PyStopIterationObject *)exc)->value); - Py_DECREF(exc); + /* fetch the value of the StopIteration instance being handled; + * this clears the error indicator */ + PyObject *value; + _PyGen_FetchStopIterationValue(&value); return value; } if (PyErr_ExceptionMatches(PyExc_GeneratorExit)) { From 206dcd0bf286665c34bd954e5c4e089f8e1a0d28 Mon Sep 17 00:00:00 2001 From: Nicolas Tessore Date: Tue, 23 May 2023 16:44:38 +0100 Subject: [PATCH 7/8] simplify return logic --- Objects/genobject.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index beede1e737a49d..1abfc83ab678ef 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -408,17 +408,16 @@ gen_close(PyGenObject *gen, PyObject *args) PyErr_SetString(PyExc_RuntimeError, msg); return NULL; } - if (PyErr_ExceptionMatches(PyExc_StopIteration)) { - /* fetch the value of the StopIteration instance being handled; - * this clears the error indicator */ - PyObject *value; - _PyGen_FetchStopIterationValue(&value); - return value; - } + assert(PyErr_Occurred()); if (PyErr_ExceptionMatches(PyExc_GeneratorExit)) { PyErr_Clear(); /* ignore this error */ Py_RETURN_NONE; } + /* if the generator returned a value while closing, StopIteration was + * raised in gen_send_ex() above; retrieve and return the value here */ + if (_PyGen_FetchStopIterationValue(&retval) == 0) { + return retval; + } return NULL; } From c0c4b513fda08a874a21580ca87f95692f6f20a6 Mon Sep 17 00:00:00 2001 From: Nicolas Tessore Date: Tue, 23 May 2023 19:37:10 +0100 Subject: [PATCH 8/8] apply changes to documentation Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> --- Doc/reference/expressions.rst | 18 +++++++++--------- ...3-05-23-00-36-02.gh-issue-104770.poSkyY.rst | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Doc/reference/expressions.rst b/Doc/reference/expressions.rst index 1d02c752b84a18..0c700f908d6878 100644 --- a/Doc/reference/expressions.rst +++ b/Doc/reference/expressions.rst @@ -595,18 +595,18 @@ is already executing raises a :exc:`ValueError` exception. .. method:: generator.close() Raises a :exc:`GeneratorExit` at the point where the generator function was - paused. If the generator function then returns a value (by catching - :exc:`GeneratorExit`), it is returned by :meth:`close`. If the generator - function is already closed, or raises :exc:`GeneratorExit` (by not catching - the exception), :meth:`close` returns :const:`None`. If the generator - yields a value, a :exc:`RuntimeError` is raised. If the generator raises - any other exception, it is propagated to the caller. If the generator has - already exited due to an exception or normal exit, :meth:`close` returns - :const:`None`. + paused. If the generator function catches the exception and returns a + value, this value is returned from :meth:`close`. If the generator function + is already closed, or raises :exc:`GeneratorExit` (by not catching the + exception), :meth:`close` returns :const:`None`. If the generator yields a + value, a :exc:`RuntimeError` is raised. If the generator raises any other + exception, it is propagated to the caller. If the generator has already + exited due to an exception or normal exit, :meth:`close` returns + :const:`None` and has no other effect. .. versionchanged:: 3.13 - If a generator returns a value after being closed, the value is returned + If a generator returns a value upon being closed, the value is returned by :meth:`close`. .. index:: single: yield; examples diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst index 14e709cbaaeae0..2103fb7d61c21a 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-05-23-00-36-02.gh-issue-104770.poSkyY.rst @@ -1,2 +1,2 @@ -If a generator returns a value after being closed, the value is returned +If a generator returns a value upon being closed, the value is now returned by :meth:`generator.close`.