| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Documentation build overview1 file changed ± library/dis.html |
Sorry, something went wrong.
There was a problem hiding this comment.
Seems promising, but I think there are still some unresolved issues around the slot re-use.
Sorry, something went wrong.
| if (skip_if_free != NULL) { | ||
| int enclosing = _PyST_GetScope(skip_if_free, k); | ||
| RETURN_IF_ERROR(enclosing); | ||
| if (enclosing == FREE) { |
There was a problem hiding this comment.
This skips a comprehension cell for special class-closure names, but _PyCompile_GetRefType() still unconditionally returns CELL for those names in class scope. The two paths then disagree about which slot to use. This source alone segfaults during compilation on this PR, without even calling outer; it compiles successfully on the parent commit (9d22a5334bd):
def outer(__class__):
class C:
result = [lambda: __class__ for __class__ in __class__]__classdict__ reproduces the same crash. Without the lambda, the mismatch can instead mutate the enclosing cell:
def outer(__class__):
class C:
result = [__class__ for __class__ in __class__]
return C.result, __class__
print(outer([1, 2]))The parent prints ([1, 2], [1, 2]); this PR prints ([1, 2], 2). Could we reconcile the special-name handling in reference resolution, cell allocation, and save/restore before reusing these slots, with coverage for both variants?
Sorry, something went wrong.
| if (reftype == FREE) { | ||
| // Reuse the enclosing free slot. Save that cell, then | ||
| // install a fresh empty cell for the comprehension. | ||
| ADDOP_NAME(c, loc, LOAD_CLOSURE_AND_CLEAR, k, freevars); |
There was a problem hiding this comment.
Between this instruction and MAKE_CELL, the CO_FAST_FREE slot is NULL. Opcode tracing can observe that interval, but PyFrame_GetVar() assumes that a free-variable slot always contains a cell. This reproducer prints OK on main, but aborts on this PR at the assertion in frame_get_var() (Objects/frameobject.c:2223):
import sys
import _testcapi
def outer(x):
def inner():
return [x for x in x]
return inner
f = outer([1])
def trace(frame, event, arg):
if frame.f_code is f.__code__:
frame.f_trace_opcodes = True
if event == "opcode":
try:
_testcapi.frame_getvar(frame, "x")
except NameError:
pass
return trace
sys.settrace(trace)
f()
sys.settrace(None)
print("OK")_testcapi.frame_getvar exercises the public PyFrame_GetVar() API. Assigning frame.f_locals["x"] in the same interval also hits a free-cell invariant assertion.
I guess one option could be to just handle NULL somehow in the affected frame introspection APIs, but it seems even better if we can avoid creating this invalid state.
One way to do this would be to use LOAD_CLOSURE here instead of LOAD_CLOSURE_AND_CLEAR, and instead create a new opcode MAKE_CELL_EMPTY, which always initializes the new cell as empty instead of using the existing slot contents. Or maybe we don't even need MAKE_CELL_EMPTY? MAKE_CELL could decide to initialize empty based on CO_FAST_FREE, if that's not too implicit. The borrow optimizer would also need to recognize this subtlety in the behavior of MAKE_CELL in order to recognize when the existing value is or isn't kept alive.
Sorry, something went wrong.
| if (enclosing != NULL) { | ||
| int enclosing_scope = _PyST_GetScope(enclosing, name); | ||
| RETURN_IF_ERROR(enclosing_scope); | ||
| if (enclosing_scope == FREE) { |
There was a problem hiding this comment.
Reusing the implicit __class__ free slot changes what zero-argument super() sees. The parent commit (9d22a5334bd) returns a super(C, self) here, but this PR raises TypeError because super() reads the comprehension's int value from that slot:
class C:
def method(self):
__class__
return [super() for __class__ in (int,)]
print(C().method()[0].__thisclass__)The explicit __class__ reference makes it an enclosing free variable. This is separate from the class-body compilation crash: save/restore is internally consistent here, but the builtin observes the temporary cell during the comprehension.
Sorry, something went wrong.
| if (scope == CELL) { | ||
| ADDOP_NAME(c, loc, MAKE_CELL, k, cellvars); | ||
| } | ||
| if (METADATA(c)->u_fasthidden != NULL) { |
There was a problem hiding this comment.
The new FREE branch reuses the enclosing free-variable slot for the comprehension target and skips hidden-local marking here. Previously, the target had a separate CO_FAST_HIDDEN slot that was populated only while the comprehension was active.
That hidden slot also controls which locals mapping a class frame exposes. Normally, class-body locals() uses the class namespace dictionary. When _PyFrame_HasHiddenLocals() finds a populated hidden slot, the runtime switches to a FrameLocalsProxy, which includes the comprehension target stored in the frame. Clearing the hidden slot on exit restores the normal class-namespace view.
With this change, if there are no other populated hidden slots, that switch never happens: locals() keeps returning the class dictionary, even though the comprehension has correctly stored its iteration value in the reused free slot. The iteration variable is therefore missing from locals():
def outer(x):
class C:
values = [locals()["x"] for x in x]
return C.values
print(outer([1, 2]))The parent commit (9d22a5334bd) prints [1, 2]; this PR raises KeyError: 'x'. Replacing locals()["x"] with eval("x") likewise changes success to NameError. Adding another target, such as for y in (0,), masks the bug because the hidden slot for y activates the proxy.
Unfortunately I don't see an easy fix for this. Two possible fixes:
Sorry, something went wrong.
| RETURN_IF_ERROR(enclosing_scope); | ||
| if (enclosing_scope == FREE) { | ||
| *ste = enclosing; | ||
| return FREE; |
There was a problem hiding this comment.
Resolving a comprehension-local name as FREE also changes the exception for reading it before initialization. The resulting LOAD_DEREF uses a free-slot offset, so _PyEval_FormatExcUnbound() now chooses NameError rather than UnboundLocalError:
def outer(x):
def inner():
return [x for y in x for x in x]
return inner()
try:
outer([1, 2])
except UnboundLocalError:
print("caught uninitialized comprehension local")The parent commit (9d22a5334bd) reaches the handler; this PR lets a NameError escape, describing x as an uninitialized variable in an enclosing scope. The same change occurs with lambda: x as the element expression.
Sorry, something went wrong.
| if (enclosing != NULL) { | ||
| int enclosing_scope = _PyST_GetScope(enclosing, name); | ||
| RETURN_IF_ERROR(enclosing_scope); | ||
| if (enclosing_scope == FREE) { |
There was a problem hiding this comment.
This condition also needs to handle DEF_FREE_CLASS: a class-local name can have that flag and an entry in u_freevars, even though _PyST_GetScope() returns LOCAL. Checking only enclosing_scope == FREE leaves the comprehension using a separate same-named slot:
import sys
def outer():
x = 1
class C:
x = 2
vals = [dict(**sys._getframe().f_locals) for x in [3]]
def m():
return x
return C
print(outer().vals[0]["x"])The parent commit (9d22a5334bd) prints 3; this PR raises TypeError: dict() got multiple values for keyword argument 'x'. The proxy's keys(), values(), items(), and len() also expose/count the duplicate entries.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This reverts a change that was made as part of #156819 , to sometimes have separate entries in the frame locals for a nested comprehension target and a variable of the same name in the enclosing scope. Instead, this PR goes back to the scheme we had before where the same frame slot is reused.