From 5ba270a86b65480f4e6d738ee9a4cee1d3ac964c Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 14 Nov 2024 19:32:45 +0000 Subject: [PATCH 01/10] first draft --- peps/pep-9999.rst | 164 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 peps/pep-9999.rst diff --git a/peps/pep-9999.rst b/peps/pep-9999.rst new file mode 100644 index 00000000000..4093a85dbb2 --- /dev/null +++ b/peps/pep-9999.rst @@ -0,0 +1,164 @@ +PEP: 9999 +Title: Disallow return/break/continue that exit a finally block +Author: Alyssa Coghlan and Irit Katriel +Status: Draft +Type: Standards Track +Created: 14-Nov-2024 +Post-History: + +.. highlight:: rst + +Abstract +======== + +This PEP proposes to withdraw support for ``return``, ``break`` and +``continue`` statements that break out of a ``finally`` block. +This was proposed in the past by :pep:`601`. The current PEP +is based on empirical evidence regarding the cost/benefit of +this change, which did not exist at the time that :pep:`601` +was rejected. It also proposes a slightly different solution +than that which was proposed by :pep:`601`. + +Rationale +========= + +The semantics of ``return``, ``break`` and ``continue`` in a +finally block are surprising for many developers. +The documentation [1]_ mentions that + +1. If the ``finally`` clause executes a ``break``, ``continue`` +or ``return`` statement, exceptions are not re-raised. +2. If a ``finally`` clause includes a ``return`` statement, the +returned value will be the one from the ``finally`` clause’s +``return`` statement, not the value from the ``try`` clause’s +``return`` statement. + +Both of these behaviours cause confusion, but the first is +particularly dangerous because a swallowed exception is more +likely to slip through testing, than an incorrect return value. + +In 2019, :pep:`601` proposed to change Python to emit a +``SyntaxWarning`` for a few releases and then turn it into a +``SyntaxError``. It was rejected in favour of viewing this +as a programming style issue, to be handled by linters and :pep:`8`. +Indeed, :pep:`8` now recommends not to used control flow statements +in a ``finally`` block, and linters such as pylint [2]_, ruff [3]_ +and flake8-bugbear [4]_ flag them as a problem. + +A recent analysis of real world code [5]_ shows that:: + +- These features are rare (2 per million LOC in the top 8000 PyPI +packages, 4 per million LOC in a random selection of packages). +This could be thanks to the linters that flag this pattern. +- Most of the usages are incorrect, and introduce unintended +exception-swallowing bugs. +- Code owners are typically receptive to fixing the bugs, and +find that easy to do. + +This new data indicates that it would benefit Python's users if +Python itself moved them away from this harmful feature. + +One of the arguments brought up [6]_ in the discussion about :pep:`601` +was that language features should be orthogonal, and combine without +context-based restrictions. However, in the meantime :pep:`654` has +been implemented, and it forbids `return`, `break` and `continue` +in an ``except*`` clause because the semantics of that would violate +the property that ``except*`` clauses operate *in parallel*, so the +code of one clause should not suppress the invocation of another. +In that case we accepted that a combination of features can be +harmful enough that it makes sense to disallow it. + + +Specification +============= + +The change is to specify as part of the language spec that +Python's compiler may emit a ``SyntaxWarning`` or ``SyntaxError`` +when a ``return``, ``break`` or ``continue`` would transfer +control flow from within a ``finally`` block to a location outside +of it. + +This includes the following examples: + +.. code-block:: + + def f(): + try: + ... + finally: + return 42 + + for x in o: + try: + ... + finally: + break (or continue) + +But excludes these: + +.. code-block:: + + try: + ... + finally: + def f(): + return 42 + + try: + ... + finally: + for x in o: + break (or continue) + + +CPython will emit a ``SyntaxWarning`` in version 3.14, and we leave +it open whether, and when, this will become a ``SyntaxError``. +However, we specify here that a ``SyntaxError`` is permitted by +the language spec, so that other Python implementations can choose +to implement that. + +Backwards Compatibility +======================= + +For backwards compatibility reasons, we are proposing that CPython +emit only a ``SyntaxWarning``, with no concrete plan to upgrade that +to an error. Code running with ``-We`` may stop working once this +is introduced. + +Security Implications +===================== + +The warning/error will help programmers avoid some hard to find bugs, +so will have a security benefit. We are not aware of security issues +related to raising a new ``SyntaxWarning`` or ``SyntaxError``. + +How to Teach This +================= + +The change will be documented in the language spec and in the +What's New documentation. The `SyntaxWarning` will alert users +that their code needs to change. The empirical evidence [5]_ +shows that the changes necessary are typically quite +straightforward. + +References +========== + +.. [1] https://docs.python.org/3/tutorial/errors.html#defining-clean-up-actions + +.. [2] https://pylint.readthedocs.io/en/stable/ + +.. [3] https://docs.astral.sh/ruff/ + +.. [4] https://github.com/PyCQA/flake8-bugbear + +.. [5] https://github.com/iritkatriel/finally/blob/main/README.md + +.. [6] https://discuss.python.org/t/pep-601-forbid-return-break-continue-breaking-out-of-finally/2239/24 + + +Copyright +========= + +This document is placed in the public domain or under the +CC0-1.0-Universal license, whichever is more permissive. From 66238e15eeb65203ab6c71f29d5c1bdb81186d73 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 15 Nov 2024 21:23:48 +0000 Subject: [PATCH 02/10] apply Alissa's comments --- peps/pep-9999.rst | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/peps/pep-9999.rst b/peps/pep-9999.rst index 4093a85dbb2..626b898a069 100644 --- a/peps/pep-9999.rst +++ b/peps/pep-9999.rst @@ -1,10 +1,11 @@ PEP: 9999 Title: Disallow return/break/continue that exit a finally block -Author: Alyssa Coghlan and Irit Katriel +Author: Irit Katriel , Alyssa Coghlan +Discussions-To: https://discuss.python.org/t/an-analysis-of-return-in-finally-in-the-wild/70633 Status: Draft Type: Standards Track -Created: 14-Nov-2024 -Post-History: +Created: 15-Nov-2024 +Post-History: 15-Nov-2024 .. highlight:: rst @@ -41,7 +42,7 @@ In 2019, :pep:`601` proposed to change Python to emit a ``SyntaxWarning`` for a few releases and then turn it into a ``SyntaxError``. It was rejected in favour of viewing this as a programming style issue, to be handled by linters and :pep:`8`. -Indeed, :pep:`8` now recommends not to used control flow statements +Indeed, :pep:`8` now recommends not to use control flow statements in a ``finally`` block, and linters such as pylint [2]_, ruff [3]_ and flake8-bugbear [4]_ flag them as a problem. @@ -92,7 +93,7 @@ This includes the following examples: try: ... finally: - break (or continue) + break # (or continue) But excludes these: @@ -108,7 +109,7 @@ But excludes these: ... finally: for x in o: - break (or continue) + break # (or continue) CPython will emit a ``SyntaxWarning`` in version 3.14, and we leave From c895bb81212f5d028b39665032a864efc705177f Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 15 Nov 2024 21:37:37 +0000 Subject: [PATCH 03/10] add python version --- peps/pep-9999.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/peps/pep-9999.rst b/peps/pep-9999.rst index 626b898a069..7e53a8812b7 100644 --- a/peps/pep-9999.rst +++ b/peps/pep-9999.rst @@ -5,6 +5,7 @@ Discussions-To: https://discuss.python.org/t/an-analysis-of-return-in-finally-in Status: Draft Type: Standards Track Created: 15-Nov-2024 +Python-Version: 3.14 Post-History: 15-Nov-2024 .. highlight:: rst From e975cd3432aa947a50c300aa932fd88d5ab1a4d7 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 15 Nov 2024 21:41:35 +0000 Subject: [PATCH 04/10] fix lint errors. Add Rationale headline --- peps/pep-9999.rst | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/peps/pep-9999.rst b/peps/pep-9999.rst index 7e53a8812b7..961a8e95d3d 100644 --- a/peps/pep-9999.rst +++ b/peps/pep-9999.rst @@ -21,8 +21,8 @@ this change, which did not exist at the time that :pep:`601` was rejected. It also proposes a slightly different solution than that which was proposed by :pep:`601`. -Rationale -========= +Motivation +========== The semantics of ``return``, ``break`` and ``continue`` in a finally block are surprising for many developers. @@ -47,6 +47,9 @@ Indeed, :pep:`8` now recommends not to use control flow statements in a ``finally`` block, and linters such as pylint [2]_, ruff [3]_ and flake8-bugbear [4]_ flag them as a problem. +Rationale +========= + A recent analysis of real world code [5]_ shows that:: - These features are rare (2 per million LOC in the top 8000 PyPI @@ -63,7 +66,7 @@ Python itself moved them away from this harmful feature. One of the arguments brought up [6]_ in the discussion about :pep:`601` was that language features should be orthogonal, and combine without context-based restrictions. However, in the meantime :pep:`654` has -been implemented, and it forbids `return`, `break` and `continue` +been implemented, and it forbids ``return``, ``break`` and ``continue` in an ``except*`` clause because the semantics of that would violate the property that ``except*`` clauses operate *in parallel*, so the code of one clause should not suppress the invocation of another. @@ -138,7 +141,7 @@ How to Teach This ================= The change will be documented in the language spec and in the -What's New documentation. The `SyntaxWarning` will alert users +What's New documentation. The ``SyntaxWarning`` will alert users that their code needs to change. The empirical evidence [5]_ shows that the changes necessary are typically quite straightforward. From ea35c8203c09922095ca6556016e9d7d8c9a827e Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 15 Nov 2024 21:43:04 +0000 Subject: [PATCH 05/10] typo --- peps/pep-9999.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/peps/pep-9999.rst b/peps/pep-9999.rst index 961a8e95d3d..05e6a8f7e04 100644 --- a/peps/pep-9999.rst +++ b/peps/pep-9999.rst @@ -66,7 +66,7 @@ Python itself moved them away from this harmful feature. One of the arguments brought up [6]_ in the discussion about :pep:`601` was that language features should be orthogonal, and combine without context-based restrictions. However, in the meantime :pep:`654` has -been implemented, and it forbids ``return``, ``break`` and ``continue` +been implemented, and it forbids ``return``, ``break`` and ``continue`` in an ``except*`` clause because the semantics of that would violate the property that ``except*`` clauses operate *in parallel*, so the code of one clause should not suppress the invocation of another. From 0450fe4422f71e6854105d43526a2c6c4841191e Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 15 Nov 2024 21:46:34 +0000 Subject: [PATCH 06/10] emails as elsewhere --- peps/pep-9999.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/peps/pep-9999.rst b/peps/pep-9999.rst index 05e6a8f7e04..39c3efdc7ee 100644 --- a/peps/pep-9999.rst +++ b/peps/pep-9999.rst @@ -1,6 +1,6 @@ PEP: 9999 Title: Disallow return/break/continue that exit a finally block -Author: Irit Katriel , Alyssa Coghlan +Author: Irit Katriel , Alyssa Coghlan Discussions-To: https://discuss.python.org/t/an-analysis-of-return-in-finally-in-the-wild/70633 Status: Draft Type: Standards Track From 8971a6fe68fb0f0ee7a95773b249d059ac8cba7e Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 15 Nov 2024 22:06:24 +0000 Subject: [PATCH 07/10] assign pep number. Fix build errors --- .github/CODEOWNERS | 1 + peps/{pep-9999.rst => pep-0765.rst} | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) rename peps/{pep-9999.rst => pep-0765.rst} (95%) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index fa38b768e7e..449127b2a4f 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -641,6 +641,7 @@ peps/pep-0760.rst @pablogsal @brettcannon peps/pep-0761.rst @sethmlarson @hugovk peps/pep-0762.rst @pablogsal @ambv @lysnikolaou @emilyemorehouse peps/pep-0763.rst @dstufft +peps/pep-0765.rst @iritkatriel @ncoghlan # ... peps/pep-0777.rst @warsaw # ... diff --git a/peps/pep-9999.rst b/peps/pep-0765.rst similarity index 95% rename from peps/pep-9999.rst rename to peps/pep-0765.rst index 39c3efdc7ee..c9579077809 100644 --- a/peps/pep-9999.rst +++ b/peps/pep-0765.rst @@ -1,4 +1,4 @@ -PEP: 9999 +PEP: 765 Title: Disallow return/break/continue that exit a finally block Author: Irit Katriel , Alyssa Coghlan Discussions-To: https://discuss.python.org/t/an-analysis-of-return-in-finally-in-the-wild/70633 @@ -50,15 +50,15 @@ and flake8-bugbear [4]_ flag them as a problem. Rationale ========= -A recent analysis of real world code [5]_ shows that:: +A recent analysis of real world code [5]_ shows that: - These features are rare (2 per million LOC in the top 8000 PyPI -packages, 4 per million LOC in a random selection of packages). -This could be thanks to the linters that flag this pattern. + packages, 4 per million LOC in a random selection of packages). + This could be thanks to the linters that flag this pattern. - Most of the usages are incorrect, and introduce unintended -exception-swallowing bugs. + exception-swallowing bugs. - Code owners are typically receptive to fixing the bugs, and -find that easy to do. + find that easy to do. This new data indicates that it would benefit Python's users if Python itself moved them away from this harmful feature. From b06a4b68cf541d74403bf9d49e7a858cc9a1f5c7 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Fri, 15 Nov 2024 22:08:56 +0000 Subject: [PATCH 08/10] fix date --- peps/pep-0765.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/peps/pep-0765.rst b/peps/pep-0765.rst index c9579077809..494366cac27 100644 --- a/peps/pep-0765.rst +++ b/peps/pep-0765.rst @@ -6,7 +6,7 @@ Status: Draft Type: Standards Track Created: 15-Nov-2024 Python-Version: 3.14 -Post-History: 15-Nov-2024 +Post-History: 9-Nov-2024 .. highlight:: rst From 48b787b5f2620dd1237f99afb07591968dba53dd Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Fri, 15 Nov 2024 22:11:44 +0000 Subject: [PATCH 09/10] typo --- peps/pep-0765.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/peps/pep-0765.rst b/peps/pep-0765.rst index 494366cac27..9faf46270f4 100644 --- a/peps/pep-0765.rst +++ b/peps/pep-0765.rst @@ -6,7 +6,7 @@ Status: Draft Type: Standards Track Created: 15-Nov-2024 Python-Version: 3.14 -Post-History: 9-Nov-2024 +Post-History: 09-Nov-2024 .. highlight:: rst From 7ec364a768f8a1c3b9bbdba2465d2d89efec3854 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Sat, 16 Nov 2024 09:36:27 +0000 Subject: [PATCH 10/10] Apply suggestions from code review Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> --- peps/pep-0765.rst | 59 ++++++++++++++++++++--------------------------- 1 file changed, 25 insertions(+), 34 deletions(-) diff --git a/peps/pep-0765.rst b/peps/pep-0765.rst index 9faf46270f4..104a1b8210f 100644 --- a/peps/pep-0765.rst +++ b/peps/pep-0765.rst @@ -6,9 +6,7 @@ Status: Draft Type: Standards Track Created: 15-Nov-2024 Python-Version: 3.14 -Post-History: 09-Nov-2024 - -.. highlight:: rst +Post-History: `09-Nov-2024 `__, Abstract ======== @@ -26,14 +24,15 @@ Motivation The semantics of ``return``, ``break`` and ``continue`` in a finally block are surprising for many developers. -The documentation [1]_ mentions that +The :ref:`documentation ` mentions that: + +- If the ``finally`` clause executes a ``break``, ``continue`` + or ``return`` statement, exceptions are not re-raised. -1. If the ``finally`` clause executes a ``break``, ``continue`` -or ``return`` statement, exceptions are not re-raised. -2. If a ``finally`` clause includes a ``return`` statement, the -returned value will be the one from the ``finally`` clause’s -``return`` statement, not the value from the ``try`` clause’s -``return`` statement. +- If a ``finally`` clause includes a ``return`` statement, the + returned value will be the one from the ``finally`` clause’s + ``return`` statement, not the value from the ``try`` clause’s + ``return`` statement. Both of these behaviours cause confusion, but the first is particularly dangerous because a swallowed exception is more @@ -44,15 +43,19 @@ In 2019, :pep:`601` proposed to change Python to emit a ``SyntaxError``. It was rejected in favour of viewing this as a programming style issue, to be handled by linters and :pep:`8`. Indeed, :pep:`8` now recommends not to use control flow statements -in a ``finally`` block, and linters such as pylint [2]_, ruff [3]_ -and flake8-bugbear [4]_ flag them as a problem. +in a ``finally`` block, and linters such as +`Pylint `__, +`Ruff `__ and +`flake8-bugbear `__ +flag them as a problem. Rationale ========= -A recent analysis of real world code [5]_ shows that: +A recent `analysis of real world code +`_ shows that: -- These features are rare (2 per million LOC in the top 8000 PyPI +- These features are rare (2 per million LOC in the top 8,000 PyPI packages, 4 per million LOC in a random selection of packages). This could be thanks to the linters that flag this pattern. - Most of the usages are incorrect, and introduce unintended @@ -63,7 +66,8 @@ A recent analysis of real world code [5]_ shows that: This new data indicates that it would benefit Python's users if Python itself moved them away from this harmful feature. -One of the arguments brought up [6]_ in the discussion about :pep:`601` +`One of the arguments brought up +`__ was that language features should be orthogonal, and combine without context-based restrictions. However, in the meantime :pep:`654` has been implemented, and it forbids ``return``, ``break`` and ``continue`` @@ -86,6 +90,7 @@ of it. This includes the following examples: .. code-block:: + :class: bad def f(): try: @@ -97,11 +102,12 @@ This includes the following examples: try: ... finally: - break # (or continue) + break # (or continue) But excludes these: .. code-block:: + :class: good try: ... @@ -113,7 +119,7 @@ But excludes these: ... finally: for x in o: - break # (or continue) + break # (or continue) CPython will emit a ``SyntaxWarning`` in version 3.14, and we leave @@ -142,26 +148,11 @@ How to Teach This The change will be documented in the language spec and in the What's New documentation. The ``SyntaxWarning`` will alert users -that their code needs to change. The empirical evidence [5]_ +that their code needs to change. The `empirical evidence +`__ shows that the changes necessary are typically quite straightforward. -References -========== - -.. [1] https://docs.python.org/3/tutorial/errors.html#defining-clean-up-actions - -.. [2] https://pylint.readthedocs.io/en/stable/ - -.. [3] https://docs.astral.sh/ruff/ - -.. [4] https://github.com/PyCQA/flake8-bugbear - -.. [5] https://github.com/iritkatriel/finally/blob/main/README.md - -.. [6] https://discuss.python.org/t/pep-601-forbid-return-break-continue-breaking-out-of-finally/2239/24 - - Copyright =========