remove DEF_COMP_CELL

Author: iritkatriel

Doc/whatsnew/3.16.rst

@@ -500,11 +500,10 @@ symtable
 
 * Inlined list, set and dict comprehensions (:pep:`709`) are now represented
   as their own symbol table entries, of type
-  :attr:`~symtable.SymbolTableType.INLINED_COMPREHENSION`. This entry type
-  represents a sub-scope, and holds information only on the symbols whose
-  scopes are different in the comprehension and the enclosing scope.
-  Sub-scopes are a new mechanism that can be used when a symbol's scope
-  changes within the same compilation unit.
+  :attr:`~symtable.SymbolTableType.INLINED_COMPREHENSION`. Each such entry is
+  a lexical child of the enclosing scope and records the comprehension's own
+  locals, cells, and free names. It does not correspond to a separate
+  compilation unit.
   (Contributed by Irit Katriel in :gh:`124697`.)
 
 

Include/internal/pycore_symtable.h

@@ -177,7 +177,6 @@ _Py_IsPrivateName(PyObject *);
 #define DEF_ANNOT (2<<7)         /* this name is annotated */
 #define DEF_COMP_ITER (2<<8)     /* this name is a comprehension iteration variable */
 #define DEF_TYPE_PARAM (2<<9)    /* this name is a type parameter */
-#define DEF_COMP_CELL (2<<10)    /* this name is a cell in an inlined comprehension */
 
 #define DEF_BOUND (DEF_LOCAL | DEF_PARAM | DEF_IMPORT)
 

Lib/symtable.py

@@ -7,7 +7,7 @@
     DEF_NONLOCAL, DEF_LOCAL,
     DEF_PARAM, DEF_TYPE_PARAM, DEF_FREE_CLASS,
     DEF_IMPORT, DEF_BOUND, DEF_ANNOT,
-    DEF_COMP_ITER, DEF_COMP_CELL,
+    DEF_COMP_ITER,
     SCOPE_OFF, SCOPE_MASK,
     FREE, LOCAL, GLOBAL_IMPLICIT, GLOBAL_EXPLICIT, CELL
 )
@@ -154,8 +154,10 @@ def lookup(self, name):
             flags = self._table.symbols[name]
             namespaces = self.__check_children(name)
             module_scope = (self._table.name == "top")
+            inlined = (self._table.type == _symtable.TYPE_INLINED_COMPREHENSION)
             sym = self._symbols[name] = Symbol(name, flags, namespaces,
-                                               module_scope=module_scope)
+                                               module_scope=module_scope,
+                                               inlined_comprehension=inlined)
         return sym
 
     def get_symbols(self):
@@ -249,12 +251,14 @@ class Class(SymbolTable):
 
 class Symbol:
 
-    def __init__(self, name, flags, namespaces=None, *, module_scope=False):
+    def __init__(self, name, flags, namespaces=None, *, module_scope=False,
+                 inlined_comprehension=False):
         self.__name = name
         self.__flags = flags
         self.__scope = _get_scope(flags)
         self.__namespaces = namespaces or ()
         self.__module_scope = module_scope
+        self.__inlined_comprehension = inlined_comprehension
 
     def __repr__(self):
         flags_str = '|'.join(self._flags_str())
@@ -348,7 +352,7 @@ def is_comp_iter(self):
     def is_comp_cell(self):
         """Return *True* if the symbol is a cell in an inlined comprehension.
         """
-        return bool(self.__flags & DEF_COMP_CELL)
+        return self.is_cell() and self.__inlined_comprehension
 
     def is_namespace(self):
         """Returns *True* if name binding introduces new namespace.

Lib/test/test_listcomps.py

@@ -277,6 +277,20 @@ def f():
         outputs = {"y": [1]}
         self._check_in_scopes(code, outputs, scopes=["module", "function"])
 
+    def test_inlined_comp_cell_with_enclosing_free(self):
+        # The listcomp cell and the enclosing free must not share an index.
+        code = """
+            def outer(y):
+                def inner():
+                    return [lambda: x for x in (1, 2)], y
+                return inner()
+            funcs, val = outer(99)
+            z = [f() for f in funcs]
+            w = val
+        """
+        outputs = {"z": [2, 2], "w": 99}
+        self._check_in_scopes(code, outputs)
+
     def test_free_inner_cell_outer(self):
         code = """
             g = 2

Lib/test/test_symtable.py

@@ -431,7 +431,7 @@ def test_symbol_repr(self):
 
         st2 = symtable.symtable("[(lambda: x) for x in [1]]", "?", "exec")
         self.assertEqual(repr(st2.get_children()[0].lookup("x")),
-                         "<symbol 'x': CELL, DEF_LOCAL|DEF_COMP_ITER|DEF_COMP_CELL>")
+                         "<symbol 'x': CELL, DEF_LOCAL|DEF_COMP_ITER>")
 
         st3 = symtable.symtable("def f():\n"
                                 "   x = 1\n"
@@ -556,6 +556,20 @@ def test_inlined_comprehension_use_of_enclosing_free_in_function(self):
         self.assertTrue(comp.lookup("x").is_free())
         self.assertTrue(comp.lookup("x").is_referenced())
 
+    def test_inlined_comprehension_comp_cell_not_on_enclosing(self):
+        st = symtable.symtable(
+            "def f():\n"
+            "    x = 1\n"
+            "    return [(lambda: x) for x in [1]]",
+            "?", "exec")
+        f = find_block(st, "f")
+        self.assertTrue(f.lookup("x").is_cell())
+        self.assertFalse(f.lookup("x").is_comp_cell())
+        comp, = (c for c in f.get_children()
+                 if c.get_type() is symtable.SymbolTableType.INLINED_COMPREHENSION)
+        self.assertTrue(comp.lookup("x").is_cell())
+        self.assertTrue(comp.lookup("x").is_comp_cell())
+
     def test_inlined_comprehension_use_of_enclosing_free_in_class(self):
         st = symtable.symtable(
             "def f():\n"
@@ -600,8 +614,10 @@ def test_inlined_nested_comprehension_class_iter_var(self):
             children[0], ["x"], ["_", "x"], nested=False)
         self.assertFalse(C.lookup("x").is_free())
         self.assertTrue(C.lookup("x").is_local())
+        self.assertFalse(C.lookup("x").is_comp_cell())
         self.assertFalse(children[0].lookup("x").is_free())
         self.assertTrue(children[0].lookup("x").is_cell())
+        self.assertTrue(children[0].lookup("x").is_comp_cell())
         self.assertFalse(inner.lookup("_").is_free())
         self.assertTrue(inner.lookup("x").is_free())
         self.assertTrue(inner.lookup("x").is_referenced())

Modules/symtablemodule.c

@@ -128,7 +128,6 @@ symtable_init_constants(PyObject *m)
     if (PyModule_AddIntMacro(m, DEF_BOUND) < 0) return -1;
     if (PyModule_AddIntMacro(m, DEF_ANNOT) < 0) return -1;
     if (PyModule_AddIntMacro(m, DEF_COMP_ITER) < 0) return -1;
-    if (PyModule_AddIntMacro(m, DEF_COMP_CELL) < 0) return -1;
 
     if (PyModule_AddIntConstant(m, "TYPE_FUNCTION", FunctionBlock) < 0)
         return -1;

Python/compile.c

@@ -595,6 +595,75 @@ dictbytype(PyObject *src, int scope_type, int flag, Py_ssize_t offset)
     return dest;
 }
 
+static int
+add_cell_names_from_symbols(PyObject *symbols, PyObject *names)
+{
+    Py_ssize_t pos = 0;
+    PyObject *k, *v;
+    while (PyDict_Next(symbols, &pos, &k, &v)) {
+        long flags = PyLong_AsLong(v);
+        if (flags == -1 && PyErr_Occurred()) {
+            return ERROR;
+        }
+        if (SYMBOL_TO_SCOPE(flags) == CELL) {
+            if (PySet_Add(names, k) < 0) {
+                return ERROR;
+            }
+        }
+    }
+    return SUCCESS;
+}
+
+static int
+add_inlined_comprehension_cell_names(PySTEntryObject *ste, PyObject *names)
+{
+    for (Py_ssize_t i = 0; i < PyList_GET_SIZE(ste->ste_children); i++) {
+        PySTEntryObject *child =
+            (PySTEntryObject *)PyList_GET_ITEM(ste->ste_children, i);
+        if (child->ste_type != InlinedComprehensionBlock) {
+            continue;
+        }
+        if (add_cell_names_from_symbols(child->ste_symbols, names) < 0) {
+            return ERROR;
+        }
+        if (add_inlined_comprehension_cell_names(child, names) < 0) {
+            return ERROR;
+        }
+    }
+    return SUCCESS;
+}
+
+/* Cells of the shared unit: this table's CELL names, plus cells that live
+ * only on inlined comprehension children. */
+static PyObject *
+compiler_cellvars(PySTEntryObject *ste)
+{
+    PyObject *names = PySet_New(NULL);
+    if (names == NULL) {
+        return NULL;
+    }
+    if (add_cell_names_from_symbols(ste->ste_symbols, names) < 0) {
+        Py_DECREF(names);
+        return NULL;
+    }
+    if (add_inlined_comprehension_cell_names(ste, names) < 0) {
+        Py_DECREF(names);
+        return NULL;
+    }
+    PyObject *sorted = PySequence_List(names);
+    Py_DECREF(names);
+    if (sorted == NULL) {
+        return NULL;
+    }
+    if (PyList_Sort(sorted) < 0) {
+        Py_DECREF(sorted);
+        return NULL;
+    }
+    PyObject *cellvars = list2dict(sorted);
+    Py_DECREF(sorted);
+    return cellvars;
+}
+
 int
 _PyCompile_EnterScope(compiler *c, identifier name, int scope_type,
                        void *key, int lineno, PyObject *private,
@@ -626,7 +695,7 @@ _PyCompile_EnterScope(compiler *c, identifier name, int scope_type,
         compiler_unit_free(u);
         return ERROR;
     }
-    u->u_metadata.u_cellvars = dictbytype(u->u_ste->ste_symbols, CELL, DEF_COMP_CELL, 0);
+    u->u_metadata.u_cellvars = compiler_cellvars(u->u_ste);
     if (!u->u_metadata.u_cellvars) {
         compiler_unit_free(u);
         return ERROR;

Python/symtable.c

@@ -361,7 +361,6 @@ static void _dump_symtable(PySTEntryObject* ste, PyObject* prefix)
         if (flags & DEF_ANNOT) printf(" DEF_ANNOT");
         if (flags & DEF_COMP_ITER) printf(" DEF_COMP_ITER");
         if (flags & DEF_TYPE_PARAM) printf(" DEF_TYPE_PARAM");
-        if (flags & DEF_COMP_CELL) printf(" DEF_COMP_CELL");
         switch (scope) {
             case LOCAL: printf(" LOCAL"); break;
             case GLOBAL_EXPLICIT: printf(" GLOBAL_EXPLICIT"); break;
@@ -860,23 +859,10 @@ finalize_inlined_comprehension(PySTEntryObject *ste, PySTEntryObject *comp,
             goto error;
         }
         int scope = SYMBOL_TO_SCOPE(comp_flags);
-        int only_flags = comp_flags & ((1 << SCOPE_OFFSET) - 1);
-        if (scope == CELL || only_flags & DEF_COMP_CELL) {
+        if (scope == CELL) {
             if (PySet_Add(inlined_cells, k) < 0) {
                 goto error;
             }
-            if (!(only_flags & DEF_COMP_CELL)) {
-                comp_flags |= DEF_COMP_CELL;
-                PyObject *newv = PyLong_FromLong(comp_flags);
-                if (newv == NULL) {
-                    goto error;
-                }
-                if (PyDict_SetItem(comp->ste_symbols, k, newv) < 0) {
-                    Py_DECREF(newv);
-                    goto error;
-                }
-                Py_DECREF(newv);
-            }
         }
         PyObject *existing = PyDict_GetItemWithError(ste->ste_symbols, k);
         if (existing == NULL && PyErr_Occurred()) {
@@ -1037,7 +1023,7 @@ drop_class_free(PySTEntryObject *ste, PyObject *free)
 static int
 update_symbols(PyObject *symbols, PyObject *scopes,
                PyObject *bound, PyObject *free,
-               PyObject *inlined_cells, int classflag)
+               int classflag)
 {
     PyObject *name = NULL, *itr = NULL;
     PyObject *v = NULL, *v_scope = NULL, *v_new = NULL, *v_free = NULL;
@@ -1049,13 +1035,6 @@ update_symbols(PyObject *symbols, PyObject *scopes,
         if (flags == -1 && PyErr_Occurred()) {
             return 0;
         }
-        int contains = PySet_Contains(inlined_cells, name);
-        if (contains < 0) {
-            return 0;
-        }
-        if (contains) {
-            flags |= DEF_COMP_CELL;
-        }
         if (PyDict_GetItemRef(scopes, name, &v_scope) < 0) {
             return 0;
         }
@@ -1336,7 +1315,7 @@ analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free,
         goto error;
     }
     /* Records the results of the analysis in the symbol table entry */
-    if (!update_symbols(ste->ste_symbols, scopes, bound, newfree, inlined_cells,
+    if (!update_symbols(ste->ste_symbols, scopes, bound, newfree,
                         (ste->ste_type == ClassBlock) || ste->ste_can_see_class_scope))
         goto error;
     temp = PyNumber_InPlaceOr(free, newfree);