Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
gh-109219: propagate free vars through type param scopes#109377
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks! I'm happy the fix may be this simple.
I'll spend some more time later today coming up with more test cases.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
The failing test is some concurrent.futures thing on FreeBSD, not relevant to this bug. |
Thanks@carljm for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
…nGH-109377)(cherry picked from commit909adb5)Co-authored-by: Carl Meyer <carl@oddbird.net>Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
GH-109410 is a backport of this pull request to the3.12 branch. |
bedevere-bot commentedSep 14, 2023
|
bedevere-bot commentedSep 14, 2023
|
bedevere-bot commentedSep 14, 2023
|
bedevere-bot commentedSep 14, 2023
|
You're breaking a lot of buildbots today Carl :) |
It's especially impressive how many I managed to break by landing a change in code under |
Uh oh!
There was an error while loading.Please reload this page.
When an annotation scope is inside a class scope, unbound name references in the annotation scope that are bound in the enclosing class scope and would otherwise be free variables are overridden (in
analyze_name
) to useGLOBAL_EXPLICIT
orGLOBAL_IMPLICIT
scope instead, since they should behave like name references in the class scope, which see only the class-scope and globals, not other enclosing scopes. E.g. in this example, the reference toT
in D's bases (which is in a nested annotation scope, sinceD
is generic) should haveGLOBAL_IMPLICIT
scope, which results in it usingLOAD_FROM_DICT_OR_GLOBALS
(where the dict is the class namespace); just as if it were a reference directly in C's scope:But in the case where the same name is also free in a child scope of the annotation scope (replace
pass
withT
in the above example), it must be included in the free vars of the annotation scope, so that the child scope has access to the cell when its closure is constructed.The symbol flag
DEF_CLASS_FREE
is intended for these class-scope cases, where a name must have a scope other thanFREE
in order for references to it to behave correctly, but must still be included in freevars because it is free in a child scope.So we must add
DEF_CLASS_FREE
to such names not only in class scopes, but also in annotation scopes enclosed by class scopes.Previously, we only added
DEF_CLASS_FREE
if the name was bound in the class scope; otherwise, it would beFREE
anyway andDEF_CLASS_FREE
would be redundant. To preserve this check in annotation scope cases, we'd have to check if the name is bound in the enclosing class scope, not the annotation scope. But this check is unnecessary, since a redundantDEF_CLASS_FREE
on aFREE
name is harmless; either way the name will be added to freevars, and this is the only impact ofDEF_CLASS_FREE
. So it's simpler to just remove the check.