gh-124697: avoid duplicate names in the same frame

Author: iritkatriel

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:
 

Include/internal/pycore_opcode_metadata.h

@@ -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

Include/opcode_ids.h

@@ -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.

InternalDocs/inlined_comprehensions.md

@@ -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)) {