review comments

python · carljm · May 9, 2023 · Jan 30, 2023 · Jan 31, 2023 · Jan 31, 2023
commit 43722b4aadcdddde1b09cba8161dffdd78f8e5fa
diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst
@@ -153,6 +153,30 @@ New Features
  In Python 3.14, the default will switch to ``'data'``.
  (Contributed by Petr Viktorin in :pep:`706`.)

+.. _whatsnew312-pep709:
+
+PEP 709: Comprehension inlining
+-------------------------------
+
+Dictionary, list, and set comprehensions are now inlined, rather than creating a
+new single-use function object for each execution of the comprehension. This
+speeds up execution of a comprehension by up to 2x.
+
+Comprehension iteration variables remain isolated; they don't overwrite a
+variable of the same name in the outer scope, nor are they visible after the
+comprehension. This isolation is now maintained via stack/locals manipulation,
+not via separate function scope.
+
+Inlining does result in a few visible behavior changes:
+
+* There is no longer a separate frame for the comprehension in tracebacks,
+  and tracing/profiling no longer shows the comprehension as a function call.
+* Calling :func:`locals` inside a comprehension now includes variables
+  from outside the comprehension, and no longer includes the synthetic `.0`
+  variable for the comprehension "argument".
+
+Contributed by Carl Meyer and Vladimir Matveev in :pep:`709`.
+
 PEP 688: Making the buffer protocol accessible in Python
 --------------------------------------------------------


diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py
@@ -92,7 +92,7 @@


 class ListComprehensionTest(unittest.TestCase):
-    def _check_in_scopes(self, code, outputs, ns=None, scopes=None):
+    def _check_in_scopes(self, code, outputs=None, ns=None, scopes=None, raises=()):
        code = textwrap.dedent(code)
        scopes = scopes or ["module", "class", "function"]
        for scope in scopes:
@@ -118,9 +118,14 @@ def get_output(moddict, name):
                    def get_output(moddict, name):
                        return moddict[name]
                ns = ns or {}
-                exec(newcode, ns)
-                for k, v in outputs.items():
-                    self.assertEqual(get_output(ns, k), v)
+                try:
+                    exec(newcode, ns)
+                except raises as e:
+                    # We care about e.g. NameError vs UnboundLocalError
+                    self.assertIs(type(e), raises)
+                else:
+                    for k, v in (outputs or {}).items():
+                        self.assertEqual(get_output(ns, k), v)

    def test_lambdas_with_iteration_var_as_default(self):
        code = """
@@ -236,6 +241,29 @@ def test_introspecting_frame_locals(self):
        outputs = {"i": 20}
        self._check_in_scopes(code, outputs)

+    def test_nested(self):
+        code = """
+            l = [1, 2, 3]
+            y = [x*2 for x in [x for x in l]]
+        """
+        outputs = {"y": [2, 4, 6]}
+        self._check_in_scopes(code, outputs)
+
+    def test_nameerror(self):
+        code = """
+            [x for x in [1]]
+            x
+        """
+
+        self._check_in_scopes(code, raises=NameError)
+
+    def test_dunder_name(self):
+        code = """
+            y = [__x for __x in [1]]
+        """
+        outputs = {"y": [1]}
+        self._check_in_scopes(code, outputs)
+
    def test_unbound_local_after_comprehension(self):
        def f():
            if False:
@@ -246,18 +274,6 @@ def f():
        with self.assertRaises(UnboundLocalError):
            f()

-    def test_nameerror(self):
-        def f():
-            [x for x in [1]]
-            return x
-
-        with self.assertRaises(NameError) as ctx:
-            f()
-
-        # UnboundLocalError is a subtype of NameError; in this case we want to
-        # check that it is actually NameError
-        self.assertIs(type(ctx.exception), NameError)
-
    def test_unbound_local_inside_comprehension(self):
        def f():
            l = [None]

diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst b/Misc/NEWS.d/next/Core and Builtins/2023-01-30-15-40-29.gh-issue-97933.nUlp3r.rst
@@ -1,2 +1,2 @@
-Inline list, dict and set comprehensions to improve performance
+:pep:`709`: inline list, dict and set comprehensions to improve performance
 and reduce bytecode size.
@@ -5011,7 +5011,9 @@ push_inlined_comprehension_state(struct compiler *c, location loc,
        if (symbol & DEF_LOCAL && ~symbol & DEF_NONLOCAL) {
            if (c->u->u_ste->ste_type != FunctionBlock) {
                // non-function scope: override this name to use fast locals
-                PyDict_SetItem(c->u->u_metadata.u_fasthidden, k, Py_True);
+                if (PyDict_SetItem(c->u->u_metadata.u_fasthidden, k, Py_True) < 0) {
+                    return ERROR;
+                }
            }
            long scope = (symbol >> SCOPE_OFFSET) & SCOPE_MASK;
            PyObject *outv = PyDict_GetItemWithError(c->u->u_ste->ste_symbols, k);
@@ -5034,11 +5036,11 @@ push_inlined_comprehension_state(struct compiler *c, location loc,
                // the outer version; we'll restore it after running the
                // comprehension
                Py_INCREF(outv);
-                if (PyDict_SetItem(c->u->u_ste->ste_symbols, k, v)) {
+                if (PyDict_SetItem(c->u->u_ste->ste_symbols, k, v) < 0) {
                    Py_DECREF(outv);
                    return ERROR;
                }
-                if (PyDict_SetItem(state->temp_symbols, k, outv)) {
+                if (PyDict_SetItem(state->temp_symbols, k, outv) < 0) {
                    Py_DECREF(outv);
                    return ERROR;
                }
@@ -5061,7 +5063,7 @@ push_inlined_comprehension_state(struct compiler *c, location loc,
                if (scope == CELL) {
                    ADDOP_NAME(c, loc, MAKE_CELL, k, cellvars);
                }
-                if (PyList_Append(state->pushed_locals, k)) {
+                if (PyList_Append(state->pushed_locals, k) < 0) {
                    return ERROR;
                }
            }