-
-
Notifications
You must be signed in to change notification settings - Fork 32k
gh-74929: Implement PEP 667 #115153
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
gh-74929: Implement PEP 667 #115153
Changes from all commits
42d7186
7eeab1b
60e70e7
1454ce4
0045274
de73bc9
ca92393
ff886ff
9690a2d
6e9848a
b84b0df
bebff28
d846de9
d00a742
1a4344d
f720e12
64d3772
2eadbf0
cbae199
9e7edf8
523cb75
4b83311
ae2db7c
bf45c02
026e15e
f42980d
e693ad0
5dd045b
30ecd4d
f35c5e3
5844fb4
06277f9
e1c3f56
b672d84
3e32572
8dc4664
652f641
f29e6a3
e0ca4fe
f78156a
4503145
cdac22c
49287ff
378aacf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -622,9 +622,14 @@ def test_exception_in_post_comp_call(self): | |
|
||
def test_frame_locals(self): | ||
code = """ | ||
val = [sys._getframe().f_locals for a in [0]][0]["a"] | ||
val = "a" in [sys._getframe().f_locals for a in [0]][0] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we've changed the intent of the test here. It was previously checking that To cover both aspects, I think we need two test cases (first one shows that def test_frame_locals_in_listcomp(self):
# Iteration variable is accessible via f_locals proxy while the listcomp is running
code = """
val = [sys._getframe().f_locals["a"] for a in [0]][0]
"""
import sys
self._check_in_scopes(code, {"val": 0}, ns={"sys": sys})
def test_frame_locals_after_listcomp(self):
# Iteration variable is no longer accessible via f_locals proxy after listcomp finishes
code = """
val = "a" in [sys._getframe().f_locals for a in [0]][0]
"""
import sys
self._check_in_scopes(code, {"val": False}, ns={"sys": sys}) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a very interesting point because it introduced a new issue - should we include the hidden fast local in val = [sys._getframe().f_locals["a"] for a in [0]][0] What should the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hidden fast-locals were designed to mimic the prior behavior when comprehensions were previously implemented as one-shot functions. So as much as is feasible, the behavior of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The second proposed test in @ncoghlan 's comment is not correct. Nor is the test as currently modified in the PR. If you access Of course if you access the frame outside the comprehension, the comprehension's hidden fast locals should not be present in its Currently in main, module globals are present in the
I suspect this is also not a problem in practice, though if we can reasonably return a proxy that can support it, it would be even better. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't think this is correct. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the key here is "as if it's a one-shot function". That's infeasible with the current compiler because it's not a function anymore. For a function, we have a dedicated frame object that keeps the local variables, which can last longer than the intepreter frame as long as there are references on it. In that way, we can always access the variables on that frame even when the actual interpreter frame is gone. However, with inline comprehension, it fakes the "function call" and clear the hidden variable - there's no mechanism currently to prevent the variable from being cleared. If we want this, we'll probably need to change the compiler and the interpreter which will definitely postpone this PEP to 3.14 (and would probably make the compiler code more complicated). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For class and module level comprehensions, can we return a proxy if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It depends whether we are aiming for "correctness" for the actual situation (that the comprehension doesn't have its own frame), or for the backwards-compatible fiction (which PEP 709 only partially maintained, but did jump through hoops to maintain in terms of visibility of class-scoped variables) that comprehensions still behave as if they have their own frame. But as @gaogaotiantian points out, it may be very hard or impossible to maintain that fiction in this case, while still having TBH in the long term I think it would be better to move away from that fiction anyway, but it will have backwards-compatibility implications. Today in I don't know why someone relying on that couldn't just use
Certainly the dict returned when I think returning a proxy inside the module- or class-scoped comprehension is more internally consistent for PEP 667; returning a dict (that includes the comprehension iteration variable) would be a bit more backward-compatible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I follow the details, but I agree that people who access |
||
""" | ||
import sys | ||
self._check_in_scopes(code, {"val": False}, ns={"sys": sys}) | ||
|
||
code = """ | ||
val = [sys._getframe().f_locals["a"] for a in [0]][0] | ||
""" | ||
self._check_in_scopes(code, {"val": 0}, ns={"sys": sys}) | ||
|
||
def _recursive_replace(self, maybe_code): | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Implement PEP 667 - converted ``frame.f_locals`` to a write through proxy |
Uh oh!
There was an error while loading. Please reload this page.