From 2a0c9b01ad8bdf69e3514bb3e722624eb8c06a18 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 6 Oct 2026 18:06:15 +0100 Subject: [PATCH 1/4] gh-124697: avoid duplicate names in the same frame --- Doc/library/dis.rst | 20 +++++++ Include/internal/pycore_opcode_metadata.h | 32 +++++++--- Include/opcode_ids.h | 12 ++-- InternalDocs/inlined_comprehensions.md | 45 ++++++++++---- Lib/_opcode_metadata.py | 12 ++-- Lib/test/test_listcomps.py | 73 +++++++++++++++++++++++ Python/bytecodes.c | 14 +++++ Python/codegen.c | 43 +++++++++---- Python/compile.c | 59 +++++++++++++++--- Python/flowgraph.c | 10 ++++ 10 files changed, 268 insertions(+), 52 deletions(-) diff --git a/Doc/library/dis.rst b/Doc/library/dis.rst index 73e77f4707cf92..d9f8519f6cf819 100644 --- a/Doc/library/dis.rst +++ b/Doc/library/dis.rst @@ -2003,6 +2003,26 @@ but are replaced by real opcodes or removed before bytecode is generated. .. versionchanged:: 3.13 This opcode is now a pseudo-instruction. +.. opcode:: LOAD_CLOSURE_AND_CLEAR (i) + + Pushes a reference to the cell contained in slot ``i`` of the "fast locals" + storage and clears that slot. Used to isolate an inlined comprehension + local that reuses an enclosing free variable. + + Note that ``LOAD_CLOSURE_AND_CLEAR`` is replaced with + ``LOAD_FAST_AND_CLEAR`` in the assembler. + + .. versionadded:: next + +.. opcode:: STORE_CLOSURE (i) + + Stores the TOS into the cell slot ``i`` of the "fast locals" storage. + Used to restore a cell saved by ``LOAD_CLOSURE_AND_CLEAR``. + + Note that ``STORE_CLOSURE`` is replaced with ``STORE_FAST`` in the assembler. + + .. versionadded:: next + .. _opcode_collections: diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index d3546119bc9ee0..4f2021975635c7 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -19,6 +19,8 @@ extern "C" { #define IS_PSEUDO_INSTR(OP) ( \ ((OP) == LOAD_CLOSURE) || \ + ((OP) == LOAD_CLOSURE_AND_CLEAR) || \ + ((OP) == STORE_CLOSURE) || \ ((OP) == STORE_FAST_MAYBE_NULL) || \ ((OP) == ANNOTATIONS_PLACEHOLDER) || \ ((OP) == JUMP) || \ @@ -334,6 +336,8 @@ int _PyOpcode_num_popped(int opcode, int oparg) { return 0; case LOAD_CLOSURE: return 0; + case LOAD_CLOSURE_AND_CLEAR: + return 0; case LOAD_COMMON_CONSTANT: return 0; case LOAD_CONST: @@ -460,6 +464,8 @@ int _PyOpcode_num_popped(int opcode, int oparg) { return 2; case STORE_ATTR_WITH_HINT: return 2; + case STORE_CLOSURE: + return 1; case STORE_DEREF: return 1; case STORE_FAST: @@ -829,6 +835,8 @@ int _PyOpcode_num_pushed(int opcode, int oparg) { return 1; case LOAD_CLOSURE: return 1; + case LOAD_CLOSURE_AND_CLEAR: + return 1; case LOAD_COMMON_CONSTANT: return 1; case LOAD_CONST: @@ -955,6 +963,8 @@ int _PyOpcode_num_pushed(int opcode, int oparg) { return 0; case STORE_ATTR_WITH_HINT: return 0; + case STORE_CLOSURE: + return 0; case STORE_DEREF: return 0; case STORE_FAST: @@ -1037,7 +1047,7 @@ enum InstructionFormat { }; #define IS_VALID_OPCODE(OP) \ - (((OP) >= 0) && ((OP) < 267) && \ + (((OP) >= 0) && ((OP) < 269) && \ (_PyOpcode_opcode_metadata[(OP)].valid_entry)) #define HAS_ARG_FLAG (1) @@ -1097,9 +1107,9 @@ struct opcode_metadata { uint32_t flags; }; -PyAPI_DATA(const struct opcode_metadata) _PyOpcode_opcode_metadata[267]; +PyAPI_DATA(const struct opcode_metadata) _PyOpcode_opcode_metadata[269]; #ifdef NEED_OPCODE_METADATA -const struct opcode_metadata _PyOpcode_opcode_metadata[267] = { +const struct opcode_metadata _PyOpcode_opcode_metadata[269] = { [BINARY_OP] = { true, INSTR_FMT_IBC0000, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG | HAS_RECORDS_VALUE_FLAG }, [BINARY_OP_ADD_FLOAT] = { true, INSTR_FMT_IXC0000, HAS_EXIT_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG }, [BINARY_OP_ADD_INT] = { true, INSTR_FMT_IXC0000, HAS_EXIT_FLAG }, @@ -1337,10 +1347,12 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[267] = { [JUMP_IF_TRUE] = { true, -1, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [JUMP_NO_INTERRUPT] = { true, -1, HAS_ARG_FLAG | HAS_JUMP_FLAG }, [LOAD_CLOSURE] = { true, -1, HAS_ARG_FLAG | HAS_LOCAL_FLAG | HAS_PURE_FLAG }, + [LOAD_CLOSURE_AND_CLEAR] = { true, -1, HAS_ARG_FLAG | HAS_LOCAL_FLAG }, [POP_BLOCK] = { true, -1, HAS_PURE_FLAG }, [SETUP_CLEANUP] = { true, -1, HAS_PURE_FLAG | HAS_ARG_FLAG }, [SETUP_FINALLY] = { true, -1, HAS_PURE_FLAG | HAS_ARG_FLAG }, [SETUP_WITH] = { true, -1, HAS_PURE_FLAG | HAS_ARG_FLAG }, + [STORE_CLOSURE] = { true, -1, HAS_ARG_FLAG | HAS_LOCAL_FLAG | HAS_ESCAPES_FLAG }, [STORE_FAST_MAYBE_NULL] = { true, -1, HAS_ARG_FLAG | HAS_LOCAL_FLAG | HAS_ESCAPES_FLAG }, }; #endif @@ -1549,9 +1561,9 @@ _PyOpcode_macro_expansion[256] = { }; #endif // NEED_OPCODE_METADATA -PyAPI_DATA(const char) *_PyOpcode_OpName[267]; +PyAPI_DATA(const char) *_PyOpcode_OpName[269]; #ifdef NEED_OPCODE_METADATA -const char *_PyOpcode_OpName[267] = { +const char *_PyOpcode_OpName[269] = { [ANNOTATIONS_PLACEHOLDER] = "ANNOTATIONS_PLACEHOLDER", [BINARY_OP] = "BINARY_OP", [BINARY_OP_ADD_FLOAT] = "BINARY_OP_ADD_FLOAT", @@ -1701,6 +1713,7 @@ const char *_PyOpcode_OpName[267] = { [LOAD_ATTR_WITH_HINT] = "LOAD_ATTR_WITH_HINT", [LOAD_BUILD_CLASS] = "LOAD_BUILD_CLASS", [LOAD_CLOSURE] = "LOAD_CLOSURE", + [LOAD_CLOSURE_AND_CLEAR] = "LOAD_CLOSURE_AND_CLEAR", [LOAD_COMMON_CONSTANT] = "LOAD_COMMON_CONSTANT", [LOAD_CONST] = "LOAD_CONST", [LOAD_DEREF] = "LOAD_DEREF", @@ -1764,6 +1777,7 @@ const char *_PyOpcode_OpName[267] = { [STORE_ATTR_INSTANCE_VALUE] = "STORE_ATTR_INSTANCE_VALUE", [STORE_ATTR_SLOT] = "STORE_ATTR_SLOT", [STORE_ATTR_WITH_HINT] = "STORE_ATTR_WITH_HINT", + [STORE_CLOSURE] = "STORE_CLOSURE", [STORE_DEREF] = "STORE_DEREF", [STORE_FAST] = "STORE_FAST", [STORE_FAST_LOAD_FAST] = "STORE_FAST_LOAD_FAST", @@ -2119,10 +2133,12 @@ struct pseudo_targets { uint8_t as_sequence; uint8_t targets[4]; }; -extern const struct pseudo_targets _PyOpcode_PseudoTargets[11]; +extern const struct pseudo_targets _PyOpcode_PseudoTargets[13]; #ifdef NEED_OPCODE_METADATA -const struct pseudo_targets _PyOpcode_PseudoTargets[11] = { +const struct pseudo_targets _PyOpcode_PseudoTargets[13] = { [LOAD_CLOSURE-256] = { 0, { LOAD_FAST, 0, 0, 0 } }, + [LOAD_CLOSURE_AND_CLEAR-256] = { 0, { LOAD_FAST_AND_CLEAR, 0, 0, 0 } }, + [STORE_CLOSURE-256] = { 0, { STORE_FAST, 0, 0, 0 } }, [STORE_FAST_MAYBE_NULL-256] = { 0, { STORE_FAST, 0, 0, 0 } }, [ANNOTATIONS_PLACEHOLDER-256] = { 0, { NOP, 0, 0, 0 } }, [JUMP-256] = { 0, { JUMP_FORWARD, JUMP_BACKWARD, 0, 0 } }, @@ -2138,7 +2154,7 @@ const struct pseudo_targets _PyOpcode_PseudoTargets[11] = { #endif // NEED_OPCODE_METADATA static inline bool is_pseudo_target(int pseudo, int target) { - if (pseudo < 256 || pseudo >= 267) { + if (pseudo < 256 || pseudo >= 269) { return false; } for (int i = 0; _PyOpcode_PseudoTargets[pseudo-256].targets[i]; i++) { diff --git a/Include/opcode_ids.h b/Include/opcode_ids.h index 11342ae451b9f6..3a2929eef8a35e 100644 --- a/Include/opcode_ids.h +++ b/Include/opcode_ids.h @@ -247,11 +247,13 @@ extern "C" { #define JUMP_IF_TRUE 259 #define JUMP_NO_INTERRUPT 260 #define LOAD_CLOSURE 261 -#define POP_BLOCK 262 -#define SETUP_CLEANUP 263 -#define SETUP_FINALLY 264 -#define SETUP_WITH 265 -#define STORE_FAST_MAYBE_NULL 266 +#define LOAD_CLOSURE_AND_CLEAR 262 +#define POP_BLOCK 263 +#define SETUP_CLEANUP 264 +#define SETUP_FINALLY 265 +#define SETUP_WITH 266 +#define STORE_CLOSURE 267 +#define STORE_FAST_MAYBE_NULL 268 #define HAVE_ARGUMENT 41 #define MIN_SPECIALIZED_OPCODE 129 diff --git a/InternalDocs/inlined_comprehensions.md b/InternalDocs/inlined_comprehensions.md index e1ccd485c165b1..be50c04b310403 100644 --- a/InternalDocs/inlined_comprehensions.md +++ b/InternalDocs/inlined_comprehensions.md @@ -86,15 +86,35 @@ The walk stops at a class: nested scopes do not see class locals. Class-closure names that would otherwise be free through a class become `GLOBAL_IMPLICIT`. +If the inlined name is `LOCAL` or `CELL` but the nearest non-inlined +enclosing table has it as `FREE`, resolve it as `FREE` so the +comprehension reuses that localsplus slot. `compiler_cellvars()` also +skips adding those child cells, which would otherwise create a second +same-named entry. + ### Isolating iteration variables `codegen_push_inlined_comprehension_locals()` in -[`Python/codegen.c`](../Python/codegen.c) isolates names bound in the -comprehension: - -* `LOAD_FAST_AND_CLEAR` saves the enclosing value (possibly `NULL`) and - clears the slot. -* `MAKE_CELL` runs if the name is a cell for this comprehension. +[`Python/codegen.c`](../Python/codegen.c) isolates each name bound in +the comprehension on one of two paths: + +Reuse an enclosing free (`_PyCompile_GetRefType()` is `FREE`): + +* `LOAD_CLOSURE_AND_CLEAR` saves the enclosing cell. +* `MAKE_CELL` always runs, so the slot holds a fresh empty cell. + The comprehension then uses `DEREF`; it must not store into the + enclosing cell. +* Restore uses `STORE_CLOSURE`. +* Those pseudo instructions carry a cell/free index so + `fix_cell_offsets` can remap them; they become `LOAD_FAST_AND_CLEAR` + / `STORE_FAST` in the assembler. + +Own fast-local slot (everything else): + +* `LOAD_FAST_AND_CLEAR` saves the enclosing value (possibly `NULL`) + and clears the slot. +* `MAKE_CELL` runs only if the name is a cell for this comprehension. +* Restore uses `STORE_FAST_MAYBE_NULL`. * In module and class units the name is added to `u_fasthidden` so assemble can set `CO_FAST_HIDDEN`. @@ -105,10 +125,13 @@ or `finally` sees the original values. Runtime ------- -An inlined comprehension cell can share a localsplus name with an -enclosing free variable (for example `[lambda: x for x in x]` inside a -nested function). `FrameLocalsProxy` keys, values, items, and `len` -keep the first slot of each name so they agree with `getitem`. +An inlined comprehension local that collides with an enclosing free +(for example `[x for x in x]` or `[lambda: x for x in x]` inside a +nested function) reuses the free slot. Isolation saves that cell and +installs a temporary one so `STORE_DEREF` does not change the value +seen by existing closures; lambdas that capture the iteration variable +share the temporary cell. After the comprehension, the original cell +is restored. Source ------ @@ -128,5 +151,3 @@ Source `InlinedComprehensionBlock` * [`Include/internal/pycore_compile.h`](../Include/internal/pycore_compile.h): `_PyCompile_InlinedComprehensionState` -* [`Objects/frameobject.c`](../Objects/frameobject.c): - `FrameLocalsProxy` duplicate-name handling diff --git a/Lib/_opcode_metadata.py b/Lib/_opcode_metadata.py index df92eae151d248..efe394d40cf898 100644 --- a/Lib/_opcode_metadata.py +++ b/Lib/_opcode_metadata.py @@ -373,11 +373,13 @@ JUMP_IF_TRUE=259, JUMP_NO_INTERRUPT=260, LOAD_CLOSURE=261, - POP_BLOCK=262, - SETUP_CLEANUP=263, - SETUP_FINALLY=264, - SETUP_WITH=265, - STORE_FAST_MAYBE_NULL=266, + LOAD_CLOSURE_AND_CLEAR=262, + POP_BLOCK=263, + SETUP_CLEANUP=264, + SETUP_FINALLY=265, + SETUP_WITH=266, + STORE_CLOSURE=267, + STORE_FAST_MAYBE_NULL=268, ) HAVE_ARGUMENT = 41 diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index f02f3223a513d2..d278452cd726d0 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -316,6 +316,49 @@ def inner(): outputs = {"z": [2, 2], "w": 99} self._check_in_scopes(code, outputs) + def test_inlined_comp_reuses_enclosing_free_slot(self): + # An inlined local that collides with an enclosing free reuses that + # free slot instead of adding a second same-named localsplus entry. + def outer(x): + def inner(): + return [x for x in x] + return inner + code = outer([1]).__code__ + self.assertEqual(code.co_varnames, ()) + self.assertEqual(code.co_cellvars, ()) + self.assertEqual(code.co_freevars, ('x',)) + + def test_inlined_comp_cell_reuses_enclosing_free_slot(self): + def outer(x): + def inner(): + return [lambda: x for x in x] + return inner + code = outer([1]).__code__ + self.assertEqual(code.co_varnames, ()) + self.assertEqual(code.co_cellvars, ()) + self.assertEqual(code.co_freevars, ('x',)) + + def test_nested_inlined_comp_reuses_enclosing_free_slot(self): + def outer(x): + def inner(): + return [[x for _ in (0,)] for x in x] + return inner + code = outer([1]).__code__ + self.assertNotIn('x', code.co_varnames) + self.assertNotIn('x', code.co_cellvars) + self.assertEqual(code.co_freevars, ('x',)) + + def test_inlined_comp_exception_restores_enclosing_free(self): + def outer(x): + def inner(): + try: + [1 / 0 for x in x] + except ZeroDivisionError: + pass + return x + return inner() + self.assertEqual(outer([1, 2]), [1, 2]) + def test_free_inner_cell_outer(self): code = """ g = 2 @@ -944,6 +987,36 @@ def inner(): {"snaps": [1, 2], "vals": [2, 2], "consistent": [True, True]}, ns={"sys": sys}, scopes=["module", "function"]) + def test_frame_locals_comp_local_and_enclosing_free(self): + # Same-name collision without a lambda: the inlined local reuses the + # enclosing free slot. f_locals keys must still be unique. + code = """ + def outer(x): + def inner(): + return [(dict(**sys._getframe().f_locals), + len(sys._getframe().f_locals), + list(sys._getframe().f_locals.keys()), + list(sys._getframe().f_locals.values()), + list(sys._getframe().f_locals.items()), + dict(sys._getframe().f_locals.items())) + for x in x] + return inner() + result = outer([1, 2]) + snaps = [d['x'] for d, *_ in result] + consistent = [] + for d, n, ks, vs, it, d_items in result: + consistent.append( + n == len(ks) == len(vs) == len(it) + and ks.count('x') == 1 + and d == d_items == dict(zip(ks, vs)) + ) + """ + import sys + self._check_in_scopes( + code, + {"snaps": [1, 2], "consistent": [True, True]}, + ns={"sys": sys}, scopes=["module", "function"]) + def test_frame_locals_nested_comp_cell_and_enclosing_free(self): # Stress a nested inlined shape where a comp cell and enclosing free # share a name; all f_locals views must stay consistent. diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 31eaeab0d67841..e2ff87739d8b3d 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -270,6 +270,20 @@ dummy_func( LOAD_FAST, }; + /* Like LOAD_FAST_AND_CLEAR, but the oparg is a cell/free index + * remapped in fix_cell_offsets. Used to isolate an inlined + * comprehension local that reuses an enclosing free slot. */ + pseudo(LOAD_CLOSURE_AND_CLEAR, (-- unused)) = { + LOAD_FAST_AND_CLEAR, + }; + + /* Like STORE_FAST, but the oparg is a cell/free index remapped + * in fix_cell_offsets. Restores the cell saved by + * LOAD_CLOSURE_AND_CLEAR. */ + pseudo(STORE_CLOSURE, (unused --)) = { + STORE_FAST, + }; + inst(LOAD_FAST_CHECK, (-- value)) { _PyStackRef value_s = GETLOCAL(oparg); if (PyStackRef_IsNull(value_s)) { diff --git a/Python/codegen.c b/Python/codegen.c index 7143d9abc4255e..257a948948fee2 100644 --- a/Python/codegen.c +++ b/Python/codegen.c @@ -4915,22 +4915,32 @@ codegen_push_inlined_comprehension_locals(compiler *c, location loc, return ERROR; } } - // in the case of a cell, this will actually push the cell - // itself to the stack, then we'll create a new one for the - // comprehension and restore the original one after - ADDOP_NAME(c, loc, LOAD_FAST_AND_CLEAR, k, varnames); - if (scope == CELL) { - ADDOP_NAME(c, loc, MAKE_CELL, k, cellvars); + int reftype = _PyCompile_GetRefType(c, k); + RETURN_IF_ERROR(reftype); + 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); + ADDOP_NAME(c, loc, MAKE_CELL, k, freevars); + } + else { + // in the case of a cell, this will actually push the cell + // itself to the stack, then we'll create a new one for the + // comprehension and restore the original one after + ADDOP_NAME(c, loc, LOAD_FAST_AND_CLEAR, k, varnames); + if (scope == CELL) { + ADDOP_NAME(c, loc, MAKE_CELL, k, cellvars); + } + if (METADATA(c)->u_fasthidden != NULL) { + /* For Module/Class scopes, assemble needs to set CO_FAST_HIDDEN on these names */ + if (PySet_Add(METADATA(c)->u_fasthidden, k) < 0) { + return ERROR; + } + } } if (PyList_Append(state->pushed_locals, k) < 0) { return ERROR; } - if (METADATA(c)->u_fasthidden != NULL) { - /* For Module/Class scopes, assemble needs to set CO_FAST_HIDDEN on these names */ - if (PySet_Add(METADATA(c)->u_fasthidden, k) < 0) { - return ERROR; - } - } } } if (state->pushed_locals) { @@ -4987,7 +4997,14 @@ restore_inlined_comprehension_locals(compiler *c, location loc, if (k == NULL) { return ERROR; } - ADDOP_NAME(c, loc, STORE_FAST_MAYBE_NULL, k, varnames); + int reftype = _PyCompile_GetRefType(c, k); + RETURN_IF_ERROR(reftype); + if (reftype == FREE) { + ADDOP_NAME(c, loc, STORE_CLOSURE, k, freevars); + } + else { + ADDOP_NAME(c, loc, STORE_FAST_MAYBE_NULL, k, varnames); + } } return SUCCESS; } diff --git a/Python/compile.c b/Python/compile.c index ee29f7a9a5d589..11088e51a5e6a4 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -596,7 +596,8 @@ dictbytype(PyObject *src, int scope_type, int flag, Py_ssize_t offset) } static int -add_cell_names_from_symbols(PyObject *symbols, PyObject *names) +add_cell_names_from_symbols(PyObject *symbols, PyObject *names, + PySTEntryObject *skip_if_free) { Py_ssize_t pos = 0; PyObject *k, *v; @@ -605,17 +606,28 @@ add_cell_names_from_symbols(PyObject *symbols, PyObject *names) if (flags == -1 && PyErr_Occurred()) { return ERROR; } - if (SYMBOL_TO_SCOPE(flags) == CELL) { - if (PySet_Add(names, k) < 0) { - return ERROR; + if (SYMBOL_TO_SCOPE(flags) != CELL) { + continue; + } + /* Inlined cells that collide with an enclosing free reuse that + * slot instead of adding a second same-named localsplus entry. */ + if (skip_if_free != NULL) { + int enclosing = _PyST_GetScope(skip_if_free, k); + RETURN_IF_ERROR(enclosing); + if (enclosing == FREE) { + continue; } } + if (PySet_Add(names, k) < 0) { + return ERROR; + } } return SUCCESS; } static int -add_inlined_comprehension_cell_names(PySTEntryObject *ste, PyObject *names) +add_inlined_comprehension_cell_names(PySTEntryObject *unit, + PySTEntryObject *ste, PyObject *names) { for (Py_ssize_t i = 0; i < PyList_GET_SIZE(ste->ste_children); i++) { PySTEntryObject *child = @@ -623,10 +635,10 @@ add_inlined_comprehension_cell_names(PySTEntryObject *ste, PyObject *names) if (child->ste_type != InlinedComprehensionBlock) { continue; } - if (add_cell_names_from_symbols(child->ste_symbols, names) < 0) { + if (add_cell_names_from_symbols(child->ste_symbols, names, unit) < 0) { return ERROR; } - if (add_inlined_comprehension_cell_names(child, names) < 0) { + if (add_inlined_comprehension_cell_names(unit, child, names) < 0) { return ERROR; } } @@ -642,11 +654,11 @@ compiler_cellvars(PySTEntryObject *ste) if (names == NULL) { return NULL; } - if (add_cell_names_from_symbols(ste->ste_symbols, names) < 0) { + if (add_cell_names_from_symbols(ste->ste_symbols, names, NULL) < 0) { Py_DECREF(names); return NULL; } - if (add_inlined_comprehension_cell_names(ste, names) < 0) { + if (add_inlined_comprehension_cell_names(ste, ste, names) < 0) { Py_DECREF(names); return NULL; } @@ -979,6 +991,15 @@ compiler_mod(compiler *c, mod_ty mod) return co; } +static PySTEntryObject * +enclosing_non_inlined_ste(PySTEntryObject *ste) +{ + while (ste != NULL && ste->ste_type == InlinedComprehensionBlock) { + ste = ste->ste_parent; + } + return ste; +} + /* Inlined comprehensions are compiled in the enclosing unit. If a name is * FREE in the comprehension, or is absent from its table (scope 0), resolve * it in enclosing tables until it is bound. Stop if the next table is a class: @@ -986,6 +1007,10 @@ compiler_mod(compiler *c, mod_ty mod) * the name stays FREE. __class__ and friends are not allowed to be free * through a class; treat those loads as implicit globals. * + * A LOCAL or CELL on the inlined table that is FREE in the nearest + * non-inlined enclosing table reuses that free slot, so the compilation + * unit does not grow a second same-named localsplus entry. + * * Names with no entry (scope 0) include loads synthesized by codegen, such as * the implicit receiver for zero-arg super(). */ static int @@ -1007,6 +1032,22 @@ compiler_resolve_inlined_free(PySTEntryObject **ste, PyObject *name) scope = _PyST_GetScope(*ste, name); RETURN_IF_ERROR(scope); } + /* After the walk we may be on an inlined LOCAL/CELL that collides + * with an enclosing free. Reuse that free slot (including when a + * nested inlined load walked here). */ + if ((*ste)->ste_type == InlinedComprehensionBlock && + (scope == LOCAL || scope == CELL)) + { + PySTEntryObject *enclosing = enclosing_non_inlined_ste(*ste); + if (enclosing != NULL) { + int enclosing_scope = _PyST_GetScope(enclosing, name); + RETURN_IF_ERROR(enclosing_scope); + if (enclosing_scope == FREE) { + *ste = enclosing; + return FREE; + } + } + } return scope; } diff --git a/Python/flowgraph.c b/Python/flowgraph.c index a5138d1a1fa284..6ce89ccfd4e590 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -3654,6 +3654,14 @@ convert_pseudo_ops(cfg_builder *g) assert(is_pseudo_target(LOAD_CLOSURE, LOAD_FAST)); instr->i_opcode = LOAD_FAST; } + else if (instr->i_opcode == LOAD_CLOSURE_AND_CLEAR) { + assert(is_pseudo_target(LOAD_CLOSURE_AND_CLEAR, LOAD_FAST_AND_CLEAR)); + instr->i_opcode = LOAD_FAST_AND_CLEAR; + } + else if (instr->i_opcode == STORE_CLOSURE) { + assert(is_pseudo_target(STORE_CLOSURE, STORE_FAST)); + instr->i_opcode = STORE_FAST; + } else if (instr->i_opcode == STORE_FAST_MAYBE_NULL) { assert(is_pseudo_target(STORE_FAST_MAYBE_NULL, STORE_FAST)); instr->i_opcode = STORE_FAST; @@ -3975,6 +3983,8 @@ fix_cell_offsets(_PyCompile_CodeUnitMetadata *umd, basicblock *entryblock, int * switch(inst->i_opcode) { case MAKE_CELL: case LOAD_CLOSURE: + case LOAD_CLOSURE_AND_CLEAR: + case STORE_CLOSURE: case LOAD_DEREF: case STORE_DEREF: case DELETE_DEREF: From 3c5c71088a3972a4064631eb050f2069b6863c69 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 6 Oct 2026 18:12:24 +0100 Subject: [PATCH 2/4] remove deduplicaiton in frame proxy --- Objects/frameobject.c | 106 ++++++------------------------------------ 1 file changed, 13 insertions(+), 93 deletions(-) diff --git a/Objects/frameobject.c b/Objects/frameobject.c index a4cc14a6eaad45..5872ff6606bd5a 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -94,22 +94,6 @@ framelocalsproxy_hasval(_PyInterpreterFrame *frame, PyCodeObject *co, int i) return true; } -static int -framelocalsproxy_is_first_occurrence(PyObject *seen, PyObject *name) -{ - int found = PySet_Contains(seen, name); - if (found < 0) { - return -1; - } - if (found) { - return 0; - } - if (PySet_Add(seen, name) < 0) { - return -1; - } - return 1; -} - static int framelocalsproxy_getkeyindex(PyFrameObject *frame, PyObject *key, bool read, PyObject **value_ptr) { @@ -396,28 +380,16 @@ framelocalsproxy_keys(PyObject *self, PyObject *Py_UNUSED(ignored)) if (names == NULL) { return NULL; } - // An inlined comprehension cell can share a name with a free var. - PyObject *seen = PySet_New(NULL); - if (seen == NULL) { - Py_DECREF(names); - return NULL; - } for (int i = 0; i < co->co_nlocalsplus; i++) { if (framelocalsproxy_hasval(frame->f_frame, co, i)) { PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); - int first = framelocalsproxy_is_first_occurrence(seen, name); - if (first < 0) { - goto error; - } - if (first) { - if (PyList_Append(names, name) < 0) { - goto error; - } + if (PyList_Append(names, name) < 0) { + Py_DECREF(names); + return NULL; } } } - Py_DECREF(seen); // Iterate through the extra locals if (frame->f_extra_locals) { @@ -436,11 +408,6 @@ framelocalsproxy_keys(PyObject *self, PyObject *Py_UNUSED(ignored)) } return names; - -error: - Py_DECREF(seen); - Py_DECREF(names); - return NULL; } static void @@ -622,30 +589,18 @@ framelocalsproxy_values(PyObject *self, PyObject *Py_UNUSED(ignored)) if (values == NULL) { return NULL; } - PyObject *seen = PySet_New(NULL); - if (seen == NULL) { - Py_DECREF(values); - return NULL; - } for (int i = 0; i < co->co_nlocalsplus; i++) { PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i); if (value) { - PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); - int first = framelocalsproxy_is_first_occurrence(seen, name); - if (first == 1) { - if (PyList_Append(values, value) < 0) { - Py_DECREF(value); - goto error; - } + if (PyList_Append(values, value) < 0) { + Py_DECREF(value); + Py_DECREF(values); + return NULL; } Py_DECREF(value); - if (first < 0) { - goto error; - } } } - Py_DECREF(seen); // Iterate through the extra locals if (frame->f_extra_locals) { @@ -661,11 +616,6 @@ framelocalsproxy_values(PyObject *self, PyObject *Py_UNUSED(ignored)) } return values; - -error: - Py_DECREF(seen); - Py_DECREF(values); - return NULL; } static PyObject * @@ -677,37 +627,21 @@ framelocalsproxy_items(PyObject *self, PyObject *Py_UNUSED(ignored)) if (items == NULL) { return NULL; } - PyObject *seen = PySet_New(NULL); - if (seen == NULL) { - Py_DECREF(items); - return NULL; - } for (int i = 0; i < co->co_nlocalsplus; i++) { PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i); if (value) { - int first = framelocalsproxy_is_first_occurrence(seen, name); - if (first == 1) { - PyObject *pair = _PyTuple_FromPairSteal(Py_NewRef(name), value); - if (pair == NULL) { - goto error; - } - if (_PyList_AppendTakeRef((PyListObject *)items, pair) < 0) { - goto error; - } + PyObject *pair = _PyTuple_FromPairSteal(Py_NewRef(name), value); + if (pair == NULL) { + goto error; } - else { - Py_DECREF(value); - if (first < 0) { - goto error; - } + if (_PyList_AppendTakeRef((PyListObject *)items, pair) < 0) { + goto error; } } } - Py_DECREF(seen); - seen = NULL; // Iterate through the extra locals if (frame->f_extra_locals) { @@ -729,7 +663,6 @@ framelocalsproxy_items(PyObject *self, PyObject *Py_UNUSED(ignored)) return items; error: - Py_XDECREF(seen); Py_DECREF(items); return NULL; } @@ -746,24 +679,11 @@ framelocalsproxy_length(PyObject *self) size += PyDict_Size(frame->f_extra_locals); } - PyObject *seen = PySet_New(NULL); - if (seen == NULL) { - return -1; - } for (int i = 0; i < co->co_nlocalsplus; i++) { if (framelocalsproxy_hasval(frame->f_frame, co, i)) { - PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); - int first = framelocalsproxy_is_first_occurrence(seen, name); - if (first < 0) { - Py_DECREF(seen); - return -1; - } - else if (first) { - size++; - } + size++; } } - Py_DECREF(seen); return size; } From df243abf38a63ac7f8e59c23716777a518b2c4a0 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 7 Oct 2026 16:59:41 +0100 Subject: [PATCH 3/4] gh-124697: fix slot-reuse edge cases for inlined comprehensions Skip reuse for class-closure names; isolate reused frees with LOAD_CLOSURE + empty MAKE_CELL; treat DEF_FREE_CLASS as reusable; and handle temp frees in FrameLocalsProxy and LOAD_DEREF. Co-authored-by: Cursor --- Doc/library/dis.rst | 14 +-- Include/internal/pycore_interpframe.h | 6 + Include/internal/pycore_opcode_metadata.h | 24 ++-- Include/opcode_ids.h | 13 +-- InternalDocs/inlined_comprehensions.md | 30 +++-- Lib/_opcode_metadata.py | 13 +-- Lib/test/test_listcomps.py | 130 +++++++++++++++++++++- Modules/_testinternalcapi/test_cases.c.h | 17 ++- Objects/frameobject.c | 37 ++++++ Python/bytecodes.c | 30 +++-- Python/codegen.c | 8 +- Python/compile.c | 45 ++++++-- Python/executor_cases.c.h | 17 ++- Python/flowgraph.c | 5 - Python/generated_cases.c.h | 17 ++- 15 files changed, 317 insertions(+), 89 deletions(-) diff --git a/Doc/library/dis.rst b/Doc/library/dis.rst index d9f8519f6cf819..6424e4a8295997 100644 --- a/Doc/library/dis.rst +++ b/Doc/library/dis.rst @@ -2003,21 +2003,11 @@ but are replaced by real opcodes or removed before bytecode is generated. .. versionchanged:: 3.13 This opcode is now a pseudo-instruction. -.. opcode:: LOAD_CLOSURE_AND_CLEAR (i) - - Pushes a reference to the cell contained in slot ``i`` of the "fast locals" - storage and clears that slot. Used to isolate an inlined comprehension - local that reuses an enclosing free variable. - - Note that ``LOAD_CLOSURE_AND_CLEAR`` is replaced with - ``LOAD_FAST_AND_CLEAR`` in the assembler. - - .. versionadded:: next - .. opcode:: STORE_CLOSURE (i) Stores the TOS into the cell slot ``i`` of the "fast locals" storage. - Used to restore a cell saved by ``LOAD_CLOSURE_AND_CLEAR``. + Used to restore a cell saved by ``LOAD_CLOSURE`` when isolating an + inlined comprehension that reuses an enclosing free variable. Note that ``STORE_CLOSURE`` is replaced with ``STORE_FAST`` in the assembler. diff --git a/Include/internal/pycore_interpframe.h b/Include/internal/pycore_interpframe.h index a85fa9bd32853c..3f049429ddb9a6 100644 --- a/Include/internal/pycore_interpframe.h +++ b/Include/internal/pycore_interpframe.h @@ -376,6 +376,12 @@ _PyFrame_Traverse(_PyInterpreterFrame *frame, visitproc visit, void *arg); bool _PyFrame_HasHiddenLocals(_PyInterpreterFrame *frame); +/* True when localsplus[oparg] is a free cell that currently differs from + * the function's func_closure cell — i.e. an inlined comprehension has + * temporarily replaced it. */ +PyAPI_FUNC(bool) +_PyFrame_IsInlinedCompTempFree(_PyInterpreterFrame *frame, int oparg); + PyObject * _PyFrame_GetLocals(_PyInterpreterFrame *frame); diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index 4f2021975635c7..b3882bf1fd4f64 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -19,7 +19,6 @@ extern "C" { #define IS_PSEUDO_INSTR(OP) ( \ ((OP) == LOAD_CLOSURE) || \ - ((OP) == LOAD_CLOSURE_AND_CLEAR) || \ ((OP) == STORE_CLOSURE) || \ ((OP) == STORE_FAST_MAYBE_NULL) || \ ((OP) == ANNOTATIONS_PLACEHOLDER) || \ @@ -336,8 +335,6 @@ int _PyOpcode_num_popped(int opcode, int oparg) { return 0; case LOAD_CLOSURE: return 0; - case LOAD_CLOSURE_AND_CLEAR: - return 0; case LOAD_COMMON_CONSTANT: return 0; case LOAD_CONST: @@ -835,8 +832,6 @@ int _PyOpcode_num_pushed(int opcode, int oparg) { return 1; case LOAD_CLOSURE: return 1; - case LOAD_CLOSURE_AND_CLEAR: - return 1; case LOAD_COMMON_CONSTANT: return 1; case LOAD_CONST: @@ -1047,7 +1042,7 @@ enum InstructionFormat { }; #define IS_VALID_OPCODE(OP) \ - (((OP) >= 0) && ((OP) < 269) && \ + (((OP) >= 0) && ((OP) < 268) && \ (_PyOpcode_opcode_metadata[(OP)].valid_entry)) #define HAS_ARG_FLAG (1) @@ -1107,9 +1102,9 @@ struct opcode_metadata { uint32_t flags; }; -PyAPI_DATA(const struct opcode_metadata) _PyOpcode_opcode_metadata[269]; +PyAPI_DATA(const struct opcode_metadata) _PyOpcode_opcode_metadata[268]; #ifdef NEED_OPCODE_METADATA -const struct opcode_metadata _PyOpcode_opcode_metadata[269] = { +const struct opcode_metadata _PyOpcode_opcode_metadata[268] = { [BINARY_OP] = { true, INSTR_FMT_IBC0000, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG | HAS_RECORDS_VALUE_FLAG }, [BINARY_OP_ADD_FLOAT] = { true, INSTR_FMT_IXC0000, HAS_EXIT_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG }, [BINARY_OP_ADD_INT] = { true, INSTR_FMT_IXC0000, HAS_EXIT_FLAG }, @@ -1347,7 +1342,6 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[269] = { [JUMP_IF_TRUE] = { true, -1, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [JUMP_NO_INTERRUPT] = { true, -1, HAS_ARG_FLAG | HAS_JUMP_FLAG }, [LOAD_CLOSURE] = { true, -1, HAS_ARG_FLAG | HAS_LOCAL_FLAG | HAS_PURE_FLAG }, - [LOAD_CLOSURE_AND_CLEAR] = { true, -1, HAS_ARG_FLAG | HAS_LOCAL_FLAG }, [POP_BLOCK] = { true, -1, HAS_PURE_FLAG }, [SETUP_CLEANUP] = { true, -1, HAS_PURE_FLAG | HAS_ARG_FLAG }, [SETUP_FINALLY] = { true, -1, HAS_PURE_FLAG | HAS_ARG_FLAG }, @@ -1561,9 +1555,9 @@ _PyOpcode_macro_expansion[256] = { }; #endif // NEED_OPCODE_METADATA -PyAPI_DATA(const char) *_PyOpcode_OpName[269]; +PyAPI_DATA(const char) *_PyOpcode_OpName[268]; #ifdef NEED_OPCODE_METADATA -const char *_PyOpcode_OpName[269] = { +const char *_PyOpcode_OpName[268] = { [ANNOTATIONS_PLACEHOLDER] = "ANNOTATIONS_PLACEHOLDER", [BINARY_OP] = "BINARY_OP", [BINARY_OP_ADD_FLOAT] = "BINARY_OP_ADD_FLOAT", @@ -1713,7 +1707,6 @@ const char *_PyOpcode_OpName[269] = { [LOAD_ATTR_WITH_HINT] = "LOAD_ATTR_WITH_HINT", [LOAD_BUILD_CLASS] = "LOAD_BUILD_CLASS", [LOAD_CLOSURE] = "LOAD_CLOSURE", - [LOAD_CLOSURE_AND_CLEAR] = "LOAD_CLOSURE_AND_CLEAR", [LOAD_COMMON_CONSTANT] = "LOAD_COMMON_CONSTANT", [LOAD_CONST] = "LOAD_CONST", [LOAD_DEREF] = "LOAD_DEREF", @@ -2133,11 +2126,10 @@ struct pseudo_targets { uint8_t as_sequence; uint8_t targets[4]; }; -extern const struct pseudo_targets _PyOpcode_PseudoTargets[13]; +extern const struct pseudo_targets _PyOpcode_PseudoTargets[12]; #ifdef NEED_OPCODE_METADATA -const struct pseudo_targets _PyOpcode_PseudoTargets[13] = { +const struct pseudo_targets _PyOpcode_PseudoTargets[12] = { [LOAD_CLOSURE-256] = { 0, { LOAD_FAST, 0, 0, 0 } }, - [LOAD_CLOSURE_AND_CLEAR-256] = { 0, { LOAD_FAST_AND_CLEAR, 0, 0, 0 } }, [STORE_CLOSURE-256] = { 0, { STORE_FAST, 0, 0, 0 } }, [STORE_FAST_MAYBE_NULL-256] = { 0, { STORE_FAST, 0, 0, 0 } }, [ANNOTATIONS_PLACEHOLDER-256] = { 0, { NOP, 0, 0, 0 } }, @@ -2154,7 +2146,7 @@ const struct pseudo_targets _PyOpcode_PseudoTargets[13] = { #endif // NEED_OPCODE_METADATA static inline bool is_pseudo_target(int pseudo, int target) { - if (pseudo < 256 || pseudo >= 269) { + if (pseudo < 256 || pseudo >= 268) { return false; } for (int i = 0; _PyOpcode_PseudoTargets[pseudo-256].targets[i]; i++) { diff --git a/Include/opcode_ids.h b/Include/opcode_ids.h index 3a2929eef8a35e..b7df17eb74490f 100644 --- a/Include/opcode_ids.h +++ b/Include/opcode_ids.h @@ -247,13 +247,12 @@ extern "C" { #define JUMP_IF_TRUE 259 #define JUMP_NO_INTERRUPT 260 #define LOAD_CLOSURE 261 -#define LOAD_CLOSURE_AND_CLEAR 262 -#define POP_BLOCK 263 -#define SETUP_CLEANUP 264 -#define SETUP_FINALLY 265 -#define SETUP_WITH 266 -#define STORE_CLOSURE 267 -#define STORE_FAST_MAYBE_NULL 268 +#define POP_BLOCK 262 +#define SETUP_CLEANUP 263 +#define SETUP_FINALLY 264 +#define SETUP_WITH 265 +#define STORE_CLOSURE 266 +#define STORE_FAST_MAYBE_NULL 267 #define HAVE_ARGUMENT 41 #define MIN_SPECIALIZED_OPCODE 129 diff --git a/InternalDocs/inlined_comprehensions.md b/InternalDocs/inlined_comprehensions.md index be50c04b310403..3a285db71270ee 100644 --- a/InternalDocs/inlined_comprehensions.md +++ b/InternalDocs/inlined_comprehensions.md @@ -87,10 +87,12 @@ Class-closure names that would otherwise be free through a class become `GLOBAL_IMPLICIT`. If the inlined name is `LOCAL` or `CELL` but the nearest non-inlined -enclosing table has it as `FREE`, resolve it as `FREE` so the -comprehension reuses that localsplus slot. `compiler_cellvars()` also -skips adding those child cells, which would otherwise create a second -same-named entry. +enclosing table has it as `FREE` (or `DEF_FREE_CLASS`), resolve it as +`FREE` so the comprehension reuses that localsplus slot. +`compiler_cellvars()` also skips adding those child cells, which would +otherwise create a second same-named entry. Class-closure names +(`__class__` and friends) are never reused: zero-arg `super()` and +class-cell bookkeeping need the real free cell. ### Isolating iteration variables @@ -100,14 +102,12 @@ the comprehension on one of two paths: Reuse an enclosing free (`_PyCompile_GetRefType()` is `FREE`): -* `LOAD_CLOSURE_AND_CLEAR` saves the enclosing cell. -* `MAKE_CELL` always runs, so the slot holds a fresh empty cell. - The comprehension then uses `DEREF`; it must not store into the - enclosing cell. -* Restore uses `STORE_CLOSURE`. -* Those pseudo instructions carry a cell/free index so - `fix_cell_offsets` can remap them; they become `LOAD_FAST_AND_CLEAR` - / `STORE_FAST` in the assembler. +* `LOAD_CLOSURE` saves the enclosing cell without clearing the slot + (so free-slot introspection never sees a NULL). +* `MAKE_CELL` on a `CO_FAST_FREE` slot always installs a fresh empty + cell, replacing the saved one. The comprehension then uses `DEREF`. +* Restore uses `STORE_CLOSURE` (a cell/free-index pseudo that becomes + `STORE_FAST` after `fix_cell_offsets`). Own fast-local slot (everything else): @@ -133,6 +133,12 @@ seen by existing closures; lambdas that capture the iteration variable share the temporary cell. After the comprehension, the original cell is restored. +While the temporary cell is installed, `_PyFrame_IsInlinedCompTempFree()` +is true (frame cell differs from `func_closure`). That drives +class/module `locals()` to use `FrameLocalsProxy` even without a +`CO_FAST_HIDDEN` slot, and makes an empty temporary cell raise +`UnboundLocalError` rather than `NameError`. + Source ------ diff --git a/Lib/_opcode_metadata.py b/Lib/_opcode_metadata.py index efe394d40cf898..8de01b19fbcaaf 100644 --- a/Lib/_opcode_metadata.py +++ b/Lib/_opcode_metadata.py @@ -373,13 +373,12 @@ JUMP_IF_TRUE=259, JUMP_NO_INTERRUPT=260, LOAD_CLOSURE=261, - LOAD_CLOSURE_AND_CLEAR=262, - POP_BLOCK=263, - SETUP_CLEANUP=264, - SETUP_FINALLY=265, - SETUP_WITH=266, - STORE_CLOSURE=267, - STORE_FAST_MAYBE_NULL=268, + POP_BLOCK=262, + SETUP_CLEANUP=263, + SETUP_FINALLY=264, + SETUP_WITH=265, + STORE_CLOSURE=266, + STORE_FAST_MAYBE_NULL=267, ) HAVE_ARGUMENT = 41 diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index d278452cd726d0..214310d1cd9592 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -1,11 +1,12 @@ import doctest +import sys import textwrap import traceback import types import unittest from test import support -from test.support import BrokenIter +from test.support import BrokenIter, import_helper doctests = """ @@ -359,6 +360,133 @@ def inner(): return inner() self.assertEqual(outer([1, 2]), [1, 2]) + def test_reuse_class_closure_name_as_param_with_lambda(self): + # Class-closure names are not reused. Lambdas share the last + # iteration value; the enclosing binding is unchanged. + def outer(__class__): + class C: + result = [lambda: __class__ for __class__ in __class__] + return [f() for f in C.result], __class__ + self.assertEqual(outer([1, 2]), ([2, 2], [1, 2])) + + def outer(__classdict__): + class C: + result = [lambda: __classdict__ for __classdict__ in __classdict__] + return [f() for f in C.result], __classdict__ + self.assertEqual(outer([1, 2]), ([2, 2], [1, 2])) + + def test_reuse_class_closure_name_as_param_preserves_enclosing(self): + def outer(__class__): + class C: + result = [__class__ for __class__ in __class__] + return C.result, __class__ + self.assertEqual(outer([1, 2]), ([1, 2], [1, 2])) + + def outer(__classdict__): + class C: + result = [__classdict__ for __classdict__ in __classdict__] + return C.result, __classdict__ + self.assertEqual(outer([1, 2]), ([1, 2], [1, 2])) + + def test_reuse_free_slot_visible_to_opcode_trace_getvar(self): + # Isolation must not leave a free slot temporarily NULL where + # PyFrame_GetVar() (and similar) assume a cell is always present. + _testcapi = import_helper.import_module("_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) + try: + self.assertEqual(f(), [1]) + finally: + sys.settrace(None) + + def test_reuse_does_not_break_zero_arg_super(self): + # Temporary reuse of the __class__ free must not change what + # zero-argument super() observes during the comprehension. + class C: + def method(self): + __class__ + return [super() for __class__ in (int,)] + + self.assertIs(C().method()[0].__thisclass__, C) + + def test_reuse_class_body_locals_sees_iteration_var(self): + # Class-body locals() must still expose the comprehension target + # when that name reuses an enclosing free (no separate hidden slot). + def outer(x): + class C: + values = [locals()["x"] for x in x] + return C.values + self.assertEqual(outer([1, 2]), [1, 2]) + + def outer_eval(x): + class C: + values = [eval("x") for x in x] + return C.values + self.assertEqual(outer_eval([1, 2]), [1, 2]) + + def test_reuse_uninitialized_comp_local_is_unbound_local(self): + # Reading the comprehension target before it is assigned must remain + # UnboundLocalError, not NameError from LOAD_DEREF on a free slot. + def outer(x): + def inner(): + return [x for y in x for x in x] + return inner() + + with self.assertRaises(UnboundLocalError): + outer([1, 2]) + + def outer_lambda(x): + def inner(): + return [lambda: x for y in x for x in x] + return inner() + + with self.assertRaises(UnboundLocalError): + outer_lambda([1, 2]) + + def test_reuse_def_free_class_avoids_duplicate_f_locals(self): + # Class-local names with DEF_FREE_CLASS are in freevars but scoped + # LOCAL; reuse must still apply so f_locals has a unique 'x'. + def outer(): + x = 1 + class C: + x = 2 + vals = [( + dict(**sys._getframe().f_locals), + len(sys._getframe().f_locals), + list(sys._getframe().f_locals.keys()), + list(sys._getframe().f_locals.values()), + list(sys._getframe().f_locals.items()), + ) for x in [3]] + def m(): + return x + return C + + d, n, ks, vs, it = outer().vals[0] + self.assertEqual(d["x"], 3) + self.assertEqual(n, len(ks)) + self.assertEqual(n, len(vs)) + self.assertEqual(n, len(it)) + self.assertEqual(ks.count("x"), 1) + self.assertEqual(d, dict(zip(ks, vs))) + self.assertEqual(d, dict(it)) + def test_free_inner_cell_outer(self): code = """ g = 2 diff --git a/Modules/_testinternalcapi/test_cases.c.h b/Modules/_testinternalcapi/test_cases.c.h index 3bdc16437e2bc6..a5a537b8e4cfed 100644 --- a/Modules/_testinternalcapi/test_cases.c.h +++ b/Modules/_testinternalcapi/test_cases.c.h @@ -9744,12 +9744,21 @@ value = _PyCell_GetStackRef(cell); _PyFrame_StackPointerInvalidate(frame); if (PyStackRef_IsNull(value)) { + PyCodeObject *co = _PyFrame_GetCode(frame); stack_pointer[0] = value; stack_pointer += 1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); _PyFrame_SetStackPointer(frame, stack_pointer); _PyFrame_StackPointerValidate(frame); - _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); + int unbound_local = (oparg < PyUnstable_Code_GetFirstFree(co) || + _PyFrame_IsInlinedCompTempFree(frame, oparg)); + _PyFrame_StackPointerInvalidate(frame); + assert(stack_pointer == _PyFrame_GetStackPointer(frame)); + _PyFrame_StackPointerValidate(frame); + _PyEval_FormatExcCheckArg(tstate, + unbound_local ? PyExc_UnboundLocalError : PyExc_NameError, + unbound_local ? UNBOUNDLOCAL_ERROR_MSG : UNBOUNDFREE_ERROR_MSG, + PyTuple_GET_ITEM(co->co_localsplusnames, oparg)); _PyFrame_StackPointerInvalidate(frame); JUMP_TO_LABEL(error); } @@ -10652,7 +10661,11 @@ frame->instr_ptr = next_instr; next_instr += 1; INSTRUCTION_STATS(MAKE_CELL); - PyObject *initial = PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); + PyObject *initial = NULL; + PyCodeObject *co = _PyFrame_GetCode(frame); + if (!(_PyLocals_GetKind(co->co_localspluskinds, oparg) & CO_FAST_FREE)) { + initial = PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); + } PyObject *cell = PyCell_New(initial); if (cell == NULL) { JUMP_TO_LABEL(error); diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 5872ff6606bd5a..303a288a0d7c60 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -2246,6 +2246,35 @@ frame_get_var(_PyInterpreterFrame *frame, PyCodeObject *co, int i, } +bool +_PyFrame_IsInlinedCompTempFree(_PyInterpreterFrame *frame, int oparg) +{ + PyCodeObject *co = _PyFrame_GetCode(frame); + if (oparg < 0 || oparg >= co->co_nlocalsplus) { + return false; + } + if (!(_PyLocals_GetKind(co->co_localspluskinds, oparg) & CO_FAST_FREE)) { + return false; + } + if (!PyStackRef_FunctionCheck(frame->f_funcobj)) { + return false; + } + PyFunctionObject *func = + (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj); + PyObject *closure = func->func_closure; + if (closure == NULL) { + return false; + } + int free_index = oparg - (co->co_nlocalsplus - co->co_nfreevars); + if (free_index < 0 || free_index >= co->co_nfreevars) { + return false; + } + assert(free_index < PyTuple_GET_SIZE(closure)); + PyObject *closure_cell = PyTuple_GET_ITEM(closure, free_index); + PyObject *frame_cell = PyStackRef_AsPyObjectBorrow(frame->localsplus[oparg]); + return frame_cell != NULL && frame_cell != closure_cell; +} + bool _PyFrame_HasHiddenLocals(_PyInterpreterFrame *frame) { @@ -2263,6 +2292,14 @@ _PyFrame_HasHiddenLocals(_PyInterpreterFrame *frame) return true; } } + else if (kind & CO_FAST_FREE) { + /* A free slot whose cell was swapped for an inlined + * comprehension temporary must also force FrameLocalsProxy + * in class/module scopes (no separate HIDDEN slot). */ + if (_PyFrame_IsInlinedCompTempFree(frame, i)) { + return true; + } + } } return false; diff --git a/Python/bytecodes.c b/Python/bytecodes.c index e2ff87739d8b3d..5dcecc0d348ea8 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -270,16 +270,9 @@ dummy_func( LOAD_FAST, }; - /* Like LOAD_FAST_AND_CLEAR, but the oparg is a cell/free index - * remapped in fix_cell_offsets. Used to isolate an inlined - * comprehension local that reuses an enclosing free slot. */ - pseudo(LOAD_CLOSURE_AND_CLEAR, (-- unused)) = { - LOAD_FAST_AND_CLEAR, - }; - /* Like STORE_FAST, but the oparg is a cell/free index remapped - * in fix_cell_offsets. Restores the cell saved by - * LOAD_CLOSURE_AND_CLEAR. */ + * in fix_cell_offsets. Restores the cell saved by LOAD_CLOSURE + * when isolating an inlined comprehension that reuses a free. */ pseudo(STORE_CLOSURE, (unused --)) = { STORE_FAST, }; @@ -2353,7 +2346,14 @@ dummy_func( inst(MAKE_CELL, (--)) { // "initial" is probably NULL but not if it's an arg (or set // via the f_locals proxy before MAKE_CELL has run). - PyObject *initial = PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); + // For CO_FAST_FREE, always start empty: used to isolate an + // inlined comprehension that reuses a free slot after + // LOAD_CLOSURE saved the enclosing cell on the stack. + PyObject *initial = NULL; + PyCodeObject *co = _PyFrame_GetCode(frame); + if (!(_PyLocals_GetKind(co->co_localspluskinds, oparg) & CO_FAST_FREE)) { + initial = PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); + } PyObject *cell = PyCell_New(initial); if (cell == NULL) { ERROR_NO_POP(); @@ -2410,7 +2410,15 @@ dummy_func( PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); value = _PyCell_GetStackRef(cell); if (PyStackRef_IsNull(value)) { - _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); + /* Same choice as _PyEval_FormatExcUnbound, plus: a free slot + * reused as an inlined-comprehension temporary is unbound local. */ + PyCodeObject *co = _PyFrame_GetCode(frame); + int unbound_local = (oparg < PyUnstable_Code_GetFirstFree(co) || + _PyFrame_IsInlinedCompTempFree(frame, oparg)); + _PyEval_FormatExcCheckArg(tstate, + unbound_local ? PyExc_UnboundLocalError : PyExc_NameError, + unbound_local ? UNBOUNDLOCAL_ERROR_MSG : UNBOUNDFREE_ERROR_MSG, + PyTuple_GET_ITEM(co->co_localsplusnames, oparg)); ERROR_IF(true); } } diff --git a/Python/codegen.c b/Python/codegen.c index 257a948948fee2..84e8d3c5fded8a 100644 --- a/Python/codegen.c +++ b/Python/codegen.c @@ -4918,9 +4918,11 @@ codegen_push_inlined_comprehension_locals(compiler *c, location loc, int reftype = _PyCompile_GetRefType(c, k); RETURN_IF_ERROR(reftype); 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); + // Reuse the enclosing free slot. Save that cell without + // clearing (avoids a NULL free slot), then MAKE_CELL + // replaces it with a fresh empty cell (see MAKE_CELL on + // CO_FAST_FREE). + ADDOP_NAME(c, loc, LOAD_CLOSURE, k, freevars); ADDOP_NAME(c, loc, MAKE_CELL, k, freevars); } else { diff --git a/Python/compile.c b/Python/compile.c index 11088e51a5e6a4..cc29cdd094bd1f 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -595,6 +595,9 @@ dictbytype(PyObject *src, int scope_type, int flag, Py_ssize_t offset) return dest; } +static int +compiler_should_reuse_enclosing_free(PySTEntryObject *enclosing, PyObject *name); + static int add_cell_names_from_symbols(PyObject *symbols, PyObject *names, PySTEntryObject *skip_if_free) @@ -612,9 +615,9 @@ add_cell_names_from_symbols(PyObject *symbols, PyObject *names, /* Inlined cells that collide with an enclosing free reuse that * slot instead of adding a second same-named localsplus entry. */ if (skip_if_free != NULL) { - int enclosing = _PyST_GetScope(skip_if_free, k); - RETURN_IF_ERROR(enclosing); - if (enclosing == FREE) { + int reuse = compiler_should_reuse_enclosing_free(skip_if_free, k); + RETURN_IF_ERROR(reuse); + if (reuse) { continue; } } @@ -1000,6 +1003,30 @@ enclosing_non_inlined_ste(PySTEntryObject *ste) return ste; } +/* True if an inlined LOCAL/CELL should reuse enclosing's freevars slot. + * Class-closure names are excluded: zero-arg super() and class-cell + * bookkeeping depend on the real free cell. DEF_FREE_CLASS counts as a + * free slot even though _PyST_GetScope() returns LOCAL. */ +static int +compiler_should_reuse_enclosing_free(PySTEntryObject *enclosing, PyObject *name) +{ + if (_PyST_IsClassClosureName(name)) { + return 0; + } + PyObject *v = PyDict_GetItemWithError(enclosing->ste_symbols, name); + if (v == NULL) { + return PyErr_Occurred() ? ERROR : 0; + } + long flags = PyLong_AsLong(v); + if (flags == -1 && PyErr_Occurred()) { + return ERROR; + } + if (SYMBOL_TO_SCOPE(flags) == FREE || (flags & DEF_FREE_CLASS)) { + return 1; + } + return 0; +} + /* Inlined comprehensions are compiled in the enclosing unit. If a name is * FREE in the comprehension, or is absent from its table (scope 0), resolve * it in enclosing tables until it is bound. Stop if the next table is a class: @@ -1007,9 +1034,9 @@ enclosing_non_inlined_ste(PySTEntryObject *ste) * the name stays FREE. __class__ and friends are not allowed to be free * through a class; treat those loads as implicit globals. * - * A LOCAL or CELL on the inlined table that is FREE in the nearest - * non-inlined enclosing table reuses that free slot, so the compilation - * unit does not grow a second same-named localsplus entry. + * A LOCAL or CELL on the inlined table that collides with an enclosing free + * (or DEF_FREE_CLASS) reuses that free slot, so the compilation unit does not + * grow a second same-named localsplus entry. * * Names with no entry (scope 0) include loads synthesized by codegen, such as * the implicit receiver for zero-arg super(). */ @@ -1040,9 +1067,9 @@ compiler_resolve_inlined_free(PySTEntryObject **ste, PyObject *name) { PySTEntryObject *enclosing = enclosing_non_inlined_ste(*ste); if (enclosing != NULL) { - int enclosing_scope = _PyST_GetScope(enclosing, name); - RETURN_IF_ERROR(enclosing_scope); - if (enclosing_scope == FREE) { + int reuse = compiler_should_reuse_enclosing_free(enclosing, name); + RETURN_IF_ERROR(reuse); + if (reuse) { *ste = enclosing; return FREE; } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index c5b2dfcf5f618f..7fff8697ff0e9f 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -10608,7 +10608,11 @@ CHECK_CURRENT_CACHED_VALUES(0); ASSERT_WITHIN_STACK_BOUNDS_IGNORING_CACHE(__FILE__, __LINE__); oparg = CURRENT_OPARG(); - PyObject *initial = PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); + PyObject *initial = NULL; + PyCodeObject *co = _PyFrame_GetCode(frame); + if (!(_PyLocals_GetKind(co->co_localspluskinds, oparg) & CO_FAST_FREE)) { + initial = PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); + } PyObject *cell = PyCell_New(initial); if (cell == NULL) { SET_CURRENT_CACHED_VALUES(0); @@ -10728,12 +10732,21 @@ value = _PyCell_GetStackRef(cell); _PyFrame_StackPointerInvalidate(frame); if (PyStackRef_IsNull(value)) { + PyCodeObject *co = _PyFrame_GetCode(frame); stack_pointer[0] = value; stack_pointer += 1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); _PyFrame_SetStackPointer(frame, stack_pointer); _PyFrame_StackPointerValidate(frame); - _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); + int unbound_local = (oparg < PyUnstable_Code_GetFirstFree(co) || + _PyFrame_IsInlinedCompTempFree(frame, oparg)); + _PyFrame_StackPointerInvalidate(frame); + assert(stack_pointer == _PyFrame_GetStackPointer(frame)); + _PyFrame_StackPointerValidate(frame); + _PyEval_FormatExcCheckArg(tstate, + unbound_local ? PyExc_UnboundLocalError : PyExc_NameError, + unbound_local ? UNBOUNDLOCAL_ERROR_MSG : UNBOUNDFREE_ERROR_MSG, + PyTuple_GET_ITEM(co->co_localsplusnames, oparg)); _PyFrame_StackPointerInvalidate(frame); SET_CURRENT_CACHED_VALUES(0); JUMP_TO_ERROR(); diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 6ce89ccfd4e590..e8d8a13055dd53 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -3654,10 +3654,6 @@ convert_pseudo_ops(cfg_builder *g) assert(is_pseudo_target(LOAD_CLOSURE, LOAD_FAST)); instr->i_opcode = LOAD_FAST; } - else if (instr->i_opcode == LOAD_CLOSURE_AND_CLEAR) { - assert(is_pseudo_target(LOAD_CLOSURE_AND_CLEAR, LOAD_FAST_AND_CLEAR)); - instr->i_opcode = LOAD_FAST_AND_CLEAR; - } else if (instr->i_opcode == STORE_CLOSURE) { assert(is_pseudo_target(STORE_CLOSURE, STORE_FAST)); instr->i_opcode = STORE_FAST; @@ -3983,7 +3979,6 @@ fix_cell_offsets(_PyCompile_CodeUnitMetadata *umd, basicblock *entryblock, int * switch(inst->i_opcode) { case MAKE_CELL: case LOAD_CLOSURE: - case LOAD_CLOSURE_AND_CLEAR: case STORE_CLOSURE: case LOAD_DEREF: case STORE_DEREF: diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index dd0ce41e4b06b4..eac9a96cc3fece 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -9742,12 +9742,21 @@ value = _PyCell_GetStackRef(cell); _PyFrame_StackPointerInvalidate(frame); if (PyStackRef_IsNull(value)) { + PyCodeObject *co = _PyFrame_GetCode(frame); stack_pointer[0] = value; stack_pointer += 1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); _PyFrame_SetStackPointer(frame, stack_pointer); _PyFrame_StackPointerValidate(frame); - _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); + int unbound_local = (oparg < PyUnstable_Code_GetFirstFree(co) || + _PyFrame_IsInlinedCompTempFree(frame, oparg)); + _PyFrame_StackPointerInvalidate(frame); + assert(stack_pointer == _PyFrame_GetStackPointer(frame)); + _PyFrame_StackPointerValidate(frame); + _PyEval_FormatExcCheckArg(tstate, + unbound_local ? PyExc_UnboundLocalError : PyExc_NameError, + unbound_local ? UNBOUNDLOCAL_ERROR_MSG : UNBOUNDFREE_ERROR_MSG, + PyTuple_GET_ITEM(co->co_localsplusnames, oparg)); _PyFrame_StackPointerInvalidate(frame); JUMP_TO_LABEL(error); } @@ -10650,7 +10659,11 @@ frame->instr_ptr = next_instr; next_instr += 1; INSTRUCTION_STATS(MAKE_CELL); - PyObject *initial = PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); + PyObject *initial = NULL; + PyCodeObject *co = _PyFrame_GetCode(frame); + if (!(_PyLocals_GetKind(co->co_localspluskinds, oparg) & CO_FAST_FREE)) { + initial = PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); + } PyObject *cell = PyCell_New(initial); if (cell == NULL) { JUMP_TO_LABEL(error); From bd9b32358dd709608a571a0b6f668040e1fd088b Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 7 Oct 2026 18:17:07 +0100 Subject: [PATCH 4/4] deduplicate class closure names --- Lib/test/test_listcomps.py | 29 ++++++++++ Objects/frameobject.c | 109 ++++++++++++++++++++++++++++++++++--- 2 files changed, 129 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index 214310d1cd9592..a2f8ba4d40fe79 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -426,6 +426,35 @@ def method(self): self.assertIs(C().method()[0].__thisclass__, C) + def test_class_closure_name_not_duplicated_in_f_locals(self): + # Class-closure names are not slot-reused, so localsplus can hold + # both a hidden comprehension local and the free. FrameLocalsProxy + # must still present a unique key (first wins). + class C: + def method(self): + __class__ + return [( + dict(**sys._getframe().f_locals), + len(sys._getframe().f_locals), + list(sys._getframe().f_locals.keys()), + list(sys._getframe().f_locals.values()), + list(sys._getframe().f_locals.items()), + ) for __class__ in (int,)] + + d, n, ks, vs, it = C().method()[0] + self.assertEqual(d["__class__"], int) + self.assertEqual(ks.count("__class__"), 1) + self.assertEqual(n, len(ks)) + self.assertEqual(n, len(vs)) + self.assertEqual(n, len(it)) + self.assertEqual(d, dict(zip(ks, vs))) + self.assertEqual(d, dict(it)) + # Duplicate slots are still present in the code object. + code = C.method.__code__ + self.assertEqual(code.co_varnames.count("__class__") + + code.co_cellvars.count("__class__") + + code.co_freevars.count("__class__"), 2) + def test_reuse_class_body_locals_sees_iteration_var(self): # Class-body locals() must still expose the comprehension target # when that name reuses an enclosing free (no separate hidden slot). diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 303a288a0d7c60..93a1116a2f5f0b 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -14,6 +14,7 @@ #include "pycore_object.h" // _PyObject_GC_UNTRACK() #include "pycore_opcode_metadata.h" // _PyOpcode_Caches #include "pycore_optimizer.h" // _Py_Executors_InvalidateDependency() +#include "pycore_symtable.h" // _PyST_IsClassClosureName() #include "pycore_tuple.h" // _PyTuple_FromPair #include "pycore_unicodeobject.h" // _PyUnicode_Equal() #include "pycore_weakref.h" // FT_CLEAR_WEAKREFS() @@ -94,6 +95,28 @@ framelocalsproxy_hasval(_PyInterpreterFrame *frame, PyCodeObject *co, int i) return true; } +/* 1 = include, 0 = skip duplicate, -1 = error. + * Class-closure names are not slot-reused, so localsplus can hold both a + * comprehension local and a free; track only those names in seen. */ +static int +framelocalsproxy_include_name(PyObject *seen, PyObject *name) +{ + if (!_PyST_IsClassClosureName(name)) { + return 1; + } + int found = PySet_Contains(seen, name); + if (found < 0) { + return -1; + } + if (found) { + return 0; + } + if (PySet_Add(seen, name) < 0) { + return -1; + } + return 1; +} + static int framelocalsproxy_getkeyindex(PyFrameObject *frame, PyObject *key, bool read, PyObject **value_ptr) { @@ -380,13 +403,24 @@ framelocalsproxy_keys(PyObject *self, PyObject *Py_UNUSED(ignored)) if (names == NULL) { return NULL; } + PyObject *seen = PySet_New(NULL); + if (seen == NULL) { + Py_DECREF(names); + return NULL; + } for (int i = 0; i < co->co_nlocalsplus; i++) { if (framelocalsproxy_hasval(frame->f_frame, co, i)) { PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); + int include = framelocalsproxy_include_name(seen, name); + if (include < 0) { + goto error; + } + if (!include) { + continue; + } if (PyList_Append(names, name) < 0) { - Py_DECREF(names); - return NULL; + goto error; } } } @@ -401,15 +435,21 @@ framelocalsproxy_keys(PyObject *self, PyObject *Py_UNUSED(ignored)) while (PyDict_Next(frame->f_extra_locals, &i, &key, &value)) { if (PyList_Append(names, key) < 0) { - Py_DECREF(names); - return NULL; + goto error; } } } + Py_DECREF(seen); return names; + +error: + Py_DECREF(seen); + Py_DECREF(names); + return NULL; } + static void framelocalsproxy_dealloc(PyObject *self) { @@ -589,14 +629,28 @@ framelocalsproxy_values(PyObject *self, PyObject *Py_UNUSED(ignored)) if (values == NULL) { return NULL; } + PyObject *seen = PySet_New(NULL); + if (seen == NULL) { + Py_DECREF(values); + return NULL; + } for (int i = 0; i < co->co_nlocalsplus; i++) { PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i); if (value) { + PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); + int include = framelocalsproxy_include_name(seen, name); + if (include < 0) { + Py_DECREF(value); + goto error; + } + if (!include) { + Py_DECREF(value); + continue; + } if (PyList_Append(values, value) < 0) { Py_DECREF(value); - Py_DECREF(values); - return NULL; + goto error; } Py_DECREF(value); } @@ -609,15 +663,21 @@ framelocalsproxy_values(PyObject *self, PyObject *Py_UNUSED(ignored)) PyObject *value = NULL; while (PyDict_Next(frame->f_extra_locals, &j, &key, &value)) { if (PyList_Append(values, value) < 0) { - Py_DECREF(values); - return NULL; + goto error; } } } + Py_DECREF(seen); return values; + +error: + Py_DECREF(seen); + Py_DECREF(values); + return NULL; } + static PyObject * framelocalsproxy_items(PyObject *self, PyObject *Py_UNUSED(ignored)) { @@ -627,12 +687,26 @@ framelocalsproxy_items(PyObject *self, PyObject *Py_UNUSED(ignored)) if (items == NULL) { return NULL; } + PyObject *seen = PySet_New(NULL); + if (seen == NULL) { + Py_DECREF(items); + return NULL; + } for (int i = 0; i < co->co_nlocalsplus; i++) { PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i); if (value) { + int include = framelocalsproxy_include_name(seen, name); + if (include < 0) { + Py_DECREF(value); + goto error; + } + if (!include) { + Py_DECREF(value); + continue; + } PyObject *pair = _PyTuple_FromPairSteal(Py_NewRef(name), value); if (pair == NULL) { goto error; @@ -660,13 +734,16 @@ framelocalsproxy_items(PyObject *self, PyObject *Py_UNUSED(ignored)) } } + Py_DECREF(seen); return items; error: + Py_DECREF(seen); Py_DECREF(items); return NULL; } + static Py_ssize_t framelocalsproxy_length(PyObject *self) { @@ -679,14 +756,28 @@ framelocalsproxy_length(PyObject *self) size += PyDict_Size(frame->f_extra_locals); } + PyObject *seen = PySet_New(NULL); + if (seen == NULL) { + return -1; + } for (int i = 0; i < co->co_nlocalsplus; i++) { if (framelocalsproxy_hasval(frame->f_frame, co, i)) { - size++; + PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); + int include = framelocalsproxy_include_name(seen, name); + if (include < 0) { + Py_DECREF(seen); + return -1; + } + if (include) { + size++; + } } } + Py_DECREF(seen); return size; } + static int framelocalsproxy_contains(PyObject *self, PyObject *key) {