Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in except handlers: the caught exception symbol should be cleared at the end of the except clause. #111

Open
tristanlatr opened this issue Nov 20, 2024 · 4 comments

Comments

@tristanlatr
Copy link
Contributor

Hello @serge-sans-paille, hope you are well.

I've just found out that the reasoning around the try:/except Exception as e: nodes is boggus, here is the relevant part of the documentation:

When an exception has been assigned using as target, it is cleared at the end of the except clause.
This is as if

except E as N:
    foo

was translated to

except E as N:
    try:
        foo
    finally:
        del N

This means the exception must be assigned to a different name to be able to refer to it after the except clause. Exceptions are cleared because with the traceback attached to them, they form a reference cycle with the stack frame, keeping all locals in that frame alive until the next garbage collection occurs.

Reference: https://docs.python.org/3/reference/compound_stmts.html#the-try-statement

So a reproducer for this issue is the following code (which directly taken from our test suite):

try: 
    open("/dev/null")
except Exception as e: 
    pass
e

This should generate an unbound identifier warning for e.

@serge-sans-paille
Copy link
Owner

That's a very surprising behavior! I have a patch, still need to double check it. And indeed it's very similar to #112

@tristanlatr
Copy link
Contributor Author

Hey @serge-sans-paille, what is the idea behind your patch? I could try to implement it if you lack of time. Thanks

@serge-sans-paille
Copy link
Owner

I'm trying to solve #112 first, and my current approach would be to add a list that contains the currently deleted nodes, per level/scope

@serge-sans-paille
Copy link
Owner

serge-sans-paille commented Jan 19, 2025

Approach changed: I'll consider an ast.Del as an ast.Load (as usual) plus a silent special store which, if reached, is ignored. It seems to blend elegantly in current approach, more about this later :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants