PEP 765: Disallow return/break/continue that exit a finally block

I’ve never been able to bring myself to enable any linter for my projects because every time I’ve tried has led to experiences like Oscar’s followed by a disgruntled git reset --hard of however far I got before my patience cracked.

If we’re going to lean on linters then we need some better way of categorizing linter rules so that the cost of even finding the right rules, let alone fixing them, isn’t so high as to be a deterrent.

7 Likes

If the DeprecationWarning was emitted at compile time, it would have run into the same problems as the SyntaxWarning has. If it was emitted at runtime, then the standard warnings filtering mechanisms would have worked, but even folks running third party libraries that they precompiled at installation time would get the warnings.

If we were to make further changes in future releases, my personal preference would be for something along the lines of an opt-out compiler/runtime flag for syntax warnings in general (including the ability to disable them via a dedicated CLI option and environment variable, such as -X no-syntax-warnings and PYTHONSYNTAXWARNINGS=0). (We used to have a flag along those lines in Python 2.x, albeit one running in opt-in mode rather than opt-out: the -b option to ask the runtime to emit implicit bytes conversion warnings)

I still don’t like the idea of ever escalating this particular problem to a full SyntaxError though, since that’s so much harder for end users to work around when third party code hasn’t been updated to avoid the construct (that is, this part of the PEP wasn’t a compromise necessary to obtain PEP approval from my perspective, it’s a compromise with the practical reality of mitigating the negative side effects, while still strongly discouraging the use of an error prone construct in favour of more explicit alternatives).

3 Likes

If I didnt think it wouldn’t be more disruptive to revert the change at this point, or change the kind of error emitted, I would propose reverting this or creating a new warning category specifically for this that is hopefully never reused.

This is as far as I know, the first time a SyntaxWarning has been issued for something that was using the language as it was designed with no intent for that syntax to ever become invalid. This is part of why it was so disruptive. It isn’t something that people who use warnings as errors to catch actual issues, like resource warnings, or things that will become errors in the future, such as deprecation warnings would want to see, it’s churn for something that should have just been a linter setting if this is the stance on the feature.

I don’t think this was the right way to do this, and either commiting more to making it an error or not going so far as to make it a warning would both have been better.

Maybe we need an official set of core developer endorsed lints that can be run separately from the interpreter if people want a means to warn on “suspect use” that they don’t have the conviction to make illegal.

3 Likes

It was already a linter setting, but the problem with that approach for this particular issue is that projects would have to lint all of their dependencies in order to be sure they weren’t calling anything that might be inadvertently suppressing real errors. It’s primarily the compiler that has the opportunity to systematically check everything for particularly problematic features (this one was so problematic, we intentionally omitted it when specifying the except * syntax).

However, the problem with ever escalating this to a full SyntaxError (and one of the key reasons PEP 601’s proposal to do that was rejected) is that it means end users are straight up out of options (other than patching) if they have a dependency that is using the feature (either correctly, or in a code path that isn’t being accessed).

By instead keeping it as a SyntaxWarning we’re saying “This should be reporting SyntaxError, but for historical reasons we can’t reasonably make do that”.

Right now, there are some ergonomic issues in the warning system that mean folks using dependencies that are affected by this PEP have trouble running with -We without ensuring they have precompiled their dependencies first. There’s (intentionally) a straightforward workaround for that: ensure the precompilation step happens for the affected code, even if the package installer isn’t doing it automatically. (Running without -We is also an option in many situations, but not a useful one for CI use cases)

As far as future SyntaxWarning usability goes, making it easier to suppress specifically compile-time warnings while enabling -We for runtime warnings (or even doing that by default, and requiring an extra level of opt-in for SyntaxWarning to trigger -We processing) is one plausible improvement.

We have other cases of “this is technically legal, but probably not what you want” constructs. Using is with non-singleton literals is the first one I found:

>>> 1 is 1
<python-input-0>:1: SyntaxWarning: "is" with 'int' literal. Did you mean "=="?
True

They’re certainly not common, though (I couldn’t think of any off the top of my head either, hence having to go look for one).

3 Likes

But isn’t that always the case? There are any number of things dependencies could be doing that might be masking errors or be risky in other ways. I don’t see how we can get around the assumption that you can’t use a dependency if you don’t trust it.

3 Likes

PEP 765 is taking the position that this is not a problem in the dependency, but rather in the Python language. There is no problem using a dependency that has linting/coding style issues. It’s another matter when the dependency is using a flawed language feature.

I gave a talk about this last week at PyCon UK, with brief mention of what happens in other languages. The video is here.

6 Likes

The words I used were intentional. Returning in finally having exception suppressing behavior was an intentional design feature of the language. Identity comparisons weren’t explicitly designed for use for comparing to literals, and it’s quite literally non-guaranteed implementation details that those comparisons would or wouldn’t return True. The fact that this is the best comparison you have after having to go look for one is enough confirmation for me.

Then by all means, actually change the language. This warning was already more disruptive than the standard deprecation process would have been because the warnings don’t work within existing systems, and is effectively having the message that these dependencies are what are broken, because of the bad interaction.

1 Like

Was it? That would surprise me given how implicit and non-obvious that behavior is. Do you have a reference for that intentionality?

6 Likes

It was definitely intentional. I re-confirmed that in my response to PEP 601, where I pointed out a legitimate use case (after which the very first SC voted 4/4 to reject that PEP).

Also note that it was (eventually) implemented correctly. To me, that’s another clue it wasn’t an accident.

For me, the only two solutions that make sense are:

A) Revert the syntax warning and rely on PEP 8 and linters instead.

B) Deprecate it and plan to remove it in 2-5 releases. This could be a simple follow-up PEP.

If there are problems with when the warning is issued (making it impossible to filter) I’m sure there are creative solutions for that that can help make the transition easier (and can be introduced in 3.14.1).

4 Likes

Ironically, linters play a part in making syntax warnings hard to filter (they don’t like it when code precedes import lines, and syntax warnings are often emitted at import time, so suppressing them involves manipulating the warnings filter before the main module imports anything else).

I genuinely don’t understand the rationale behind seeing “this is a syntax error for everyone” as a better end state than “this is a syntax warning for users that don’t precompile their code, which turns into an error if the warning system is configured that way”.

The warning should be enough to discourage the construct in new code, while being buried in dependency installation logs for many end users (uv is popular and rapidly growing, but still not the most common installation tool).

That said, I’d prefer deprecation and escalating to a full error over dropping the warning, so if anyone did write a follow up PEP for full removal, I wouldn’t argue too hard against it (just request that it reference and address the reasons PEP 601 failed in proposing that and PEP 765 refrained from proposing it).

4 Likes

From PEP601:

Java allows to return from within a finally block, but its use is discouraged
Ruby allows return from inside ensure (Python’s finally), but it should be an explicit return. It is discouraged and handled by linters
Like Ruby, JavaScript also allows use of return /break /continue within a finally but it is seen as unsafe and it is handled by eslint
C# forbids the use of ending statements like return /goto /break within a finally

So 75% of mentioned languages handle this by linters.
Would I be right saying that C# is quite far away on the scale of “consenting adults”?

1 Like

First of all, precompilation is supposed to be an optimization that preserves behavior. In the abstract sense, the existence of a compiler and a bytecode interpreter and all such machinery is an implementation detail that shouldn’t change how we think about our code. So an error or warning that is hidden (or revealed!) only when using precompilation is an abomination.

More importantly, I think it’s a terrible idea to have a category of syntax that is neither rejected nor accepted, but somewhere in between. A warning is acceptable as a temporary backwards compatibility measure during a deprecation process (we do this all the time with runtime features), but not as a permanent feature of the language, especially if it behaves differently when precompiling.

20 Likes

Hmm, I guess I don’t see it as notably different from other situations where caching effects mean a warning in a calculation only shows up on a cache miss (and we have two levels of caching in play for syntax warnings, with both the import cache and on-disk recompilation potentially resulting in code not needing to be compiled again and hence not generating warnings).

So my read is that there seem to be two main points of contention:

  • is it reasonable to have “warning, sharp edge here” syntactic constructs in the language indefinitely? (I’m personally OK with it, many apparently aren’t)
  • given the previous point, is it OK if caching effects mean that those warnings are only visible when the relevant caches are either cold or intentionally bypassed? (again, I’m on the “that’s OK” side)

Ideas to tweak compilation so cached files save and emit their syntax warnings when executed target the second point of disagreement, while proposals to escalate to full syntax warnings target the first.

2 Likes

This is okay if we start with the caches filled by default. That is, in order to see the warning, you need to intentionally bypass the cache.

(And yes, that’s just another way of saying “run a linter if you want to see code warnings”, but there are more than enough ways to say that that we can keep going for a while.)

1 Like

I’m new to this community, but regarding this discussion I wonder how you all think about this edge case of raise in finally:

def cleanup():
    # oops, this is buggy
    raise Exception("cleanup failed")

def feature():
    try:
        name = input("name?")
        print("hi", name)
    finally:
        cleanup()

def main():
    try:
        feature()
    except Exception as e:
        print("Feature failed:", e)
    print("Continuing")

If the user provides a name, the code logs the cleanup error and continues.
If input() raises something like EOFError, the same thing happens.
But if the user hits Ctrl + C, the KeyboardInterrupt is replaced by the cleanup error, and main() swallows it. The program keeps running, which is very surprising if you only look at feature() or main() in isolation.

Yes, letting finally raise unchecked is an anti-pattern. But it happens, and in practice the masking isn’t always obvious: as @tim.one noted in #57, both exceptions are shown via chaining. The real footgun is when an outer exception handler swallows the second exception, unintentionally discarding the original BaseException (like KeyboardInterrupt).


FWIW, we are currently having a very similar language discussion in the Squeak/Smalltalk community. Smalltalk supports non-local returns (similar to Kotlin), allowing us to do things like:

[^1] ensure: [^2]

which is the direct equivalent to Python’s:

try:
    return 1
finally:
    return 2

However, in Squeak, suppressing an exception uses the same non-local return mechanism as above:

[[^1] ensure: [self error]]
	on: Error do: [:ex | ex return].
^3

corresponding to Python’s:

try:
    try:
        return 1
    finally:
        raise Exception()
except Exception:
    pass
return 3

Because of that, a runtime check forbidding non-local returns would automatically forbid suppressing exceptions in ensure/finally, too (in the Smalltalk architecture a runtime check during unwinding makes more sense than a static compile time check). We’re debating whether that’s desirable. I’m sharing this perspective here because if Squeak’s and Python’s conceptual unwinding models are similar enough to each other, I hope it might be relevant for consideration of PEP 765 as well.


I’m curious about your thoughts on that. Is raising from finally just as evil as returning from finally in your opinion? Is this something you would cover by PEP 765 (or any good linter) as well if it was detectable at compile time?

2 Likes

FWIW, This is arguably a good reason to simply revert and keep the existing decades long behavior, and leave doing either of these when statically visible as lint warnings.

Python can’t actually forbid raising in finally because there isn’t a static way to ensure the things done in finally can’t raise, and any automatic suppression of an exception to “hold on to it until after finally” would be even more surprising:

what would you have the following do?

try:
     foo()
finally:
    func_raises()
    func_never_runs()

And even more than this, the python interpreter synthesizes python exceptions for certain internally handled things (like MemoryError on allocation failures, SystemExit with interpreter managed signals (by default), etc). Is it possible for the interpreter to delay raising SystemExit while in a finally block? Probably, with some changes, though I’m sure the consequences of that would also be surprising to anyone not already doing their own signal handling, Preventing or somehow deferring a MemoryError here might not be reasonably feasible.

6 Likes

IMO this is a general flaw in chaining, and not specifically related to finally. The same type-shifting would happen if, e.g., you wrote:

try:
    raise KeyboardInterrupt()
except:
    f()
    raise

The intention is to re-raise whatever exception is being handled, but if f() raises a different exception, the KeyboardInterrupt() will disappear in the same way as in your example with finally.

A similar concern is why PEP 654 added both ExceptionGroup and BaseExceptionGroup. The former is an Exception subclass and can only wrap Exceptionsubclasses. The latter is not an Exception subclass and it can wrap BaseExceptions. It’s not possible to do this for chaining (i.e., forbid chaining non-Exception to an Exception). So I don’t have a solution.

3 Likes

It’s certainly dubious, since the potential for the “exception downgrade” effect definitely isn’t as obvious as it would be with an explicit except BaseException: ... clause.

It would be a separate PEP to consider doing anything at the language level about it, though, and there are more potential design options when it comes to the exception chaining case than there are for the control flow statement case (for example, it might be reasonable to add a warning check in the Exception initialisation logic that picks up when __context__ is being set to a value that doesn’t itself inherit from Exception).

1 Like

What about cases like this one, where we don’t fully control the cache management, though? The package installer shipped with CPython does prepopulate the compiled code cache at installation time (primarily so the compiled code generation always runs with the same filesystem access permissions as writing the source files). This means anyone using just pip (or pip based tools) always sees filled compiled code caches for their dependencies.

pip isn’t the only installer though, and other installers (most notably uv) may decide to make their default assumption “install time and runtime permissions will be the same, so there’s no need to eagerly populate the compiled code cache”. Doesn’t the responsibility for the consequences of that UX choice lie with the tool developers that decided to deviate from the reference component behaviour rather than with us?

1 Like

I believe they do make that choice.

I think the responsibility lies with us to not design features on the assumption that tools conform to behaviours which we don’t require of them.

4 Likes