bpo-37757: Disallow PEP 572 cases that expose implementation details (GH-15131)

- drop TargetScopeError in favour of raising SyntaxError directly
  as per the updated PEP 572
- comprehension iteration variables are explicitly local, but
  named expression targets in comprehensions are nonlocal or
  global. Raise SyntaxError as specified in PEP 572
- named expression targets in the outermost iterable of a
  comprehension have an ambiguous target scope. Avoid resolving
  that question now by raising SyntaxError. PEP 572
  originally required this only for cases where the bound name
  conflicts with the iteration variable in the comprehension,
  but CPython can't easily restrict the exception to that case
  (as it doesn't know the target variable names when visiting
  the outermost iterator expression)

1 files changed, 97 insertions, 24 deletions

diff --git a/Python/symtable.c b/Python/symtable.c
index 668cc21..e48baa8 100644
--- a/Python/symtable.c
+++ b/Python/symtable.c
@@ -32,7 +32,16 @@
 #define IMPORT_STAR_WARNING "import * only allowed at module level"
 
 #define NAMED_EXPR_COMP_IN_CLASS \
-"named expression within a comprehension cannot be used in a class body"
+"assignment expression within a comprehension cannot be used in a class body"
+
+#define NAMED_EXPR_COMP_CONFLICT \
+"assignment expression cannot rebind comprehension iteration variable '%U'"
+
+#define NAMED_EXPR_COMP_INNER_LOOP_CONFLICT \
+"comprehension inner loop cannot rebind assignment expression target '%U'"
+
+#define NAMED_EXPR_COMP_ITER_EXPR \
+"assignment expression cannot be used in a comprehension iterable expression"
 
 static PySTEntryObject *
 ste_new(struct symtable *st, identifier name, _Py_block_ty block,
@@ -81,6 +90,8 @@ ste_new(struct symtable *st, identifier name, _Py_block_ty block,
     ste->ste_comprehension = 0;
     ste->ste_returns_value = 0;
     ste->ste_needs_class_closure = 0;
+    ste->ste_comp_iter_target = 0;
+    ste->ste_comp_iter_expr = 0;
 
     ste->ste_symbols = PyDict_New();
     ste->ste_varnames = PyList_New(0);
@@ -373,14 +384,21 @@ PySymtable_Lookup(struct symtable *st, void *key)
     return (PySTEntryObject *)v;
 }
 
-int
-PyST_GetScope(PySTEntryObject *ste, PyObject *name)
+static long
+_PyST_GetSymbol(PySTEntryObject *ste, PyObject *name)
 {
     PyObject *v = PyDict_GetItem(ste->ste_symbols, name);
     if (!v)
         return 0;
     assert(PyLong_Check(v));
-    return (PyLong_AS_LONG(v) >> SCOPE_OFFSET) & SCOPE_MASK;
+    return PyLong_AS_LONG(v);
+}
+
+int
+PyST_GetScope(PySTEntryObject *ste, PyObject *name)
+{
+    long symbol = _PyST_GetSymbol(ste, name);
+    return (symbol >> SCOPE_OFFSET) & SCOPE_MASK;
 }
 
 static int
@@ -955,6 +973,13 @@ symtable_enter_block(struct symtable *st, identifier name, _Py_block_ty block,
         return 0;
     }
     prev = st->st_cur;
+    /* bpo-37757: For now, disallow *all* assignment expressions in the
+     * outermost iterator expression of a comprehension, even those inside
+     * a nested comprehension or a lambda expression.
+     */
+    if (prev) {
+        ste->ste_comp_iter_expr = prev->ste_comp_iter_expr;
+    }
     /* The entry is owned by the stack. Borrow it for st_cur. */
     Py_DECREF(ste);
     st->st_cur = ste;
@@ -971,15 +996,10 @@ symtable_enter_block(struct symtable *st, identifier name, _Py_block_ty block,
 static long
 symtable_lookup(struct symtable *st, PyObject *name)
 {
-    PyObject *o;
     PyObject *mangled = _Py_Mangle(st->st_private, name);
     if (!mangled)
         return 0;
-    o = PyDict_GetItem(st->st_cur->ste_symbols, mangled);
-    Py_DECREF(mangled);
-    if (!o)
-        return 0;
-    return PyLong_AsLong(o);
+    return _PyST_GetSymbol(st->st_cur, mangled);
 }
 
 static int
@@ -1012,6 +1032,22 @@ symtable_add_def_helper(struct symtable *st, PyObject *name, int flag, struct _s
     else {
         val = flag;
     }
+    if (ste->ste_comp_iter_target) {
+        /* This name is an iteration variable in a comprehension,
+         * so check for a binding conflict with any named expressions.
+         * Otherwise, mark it as an iteration variable so subsequent
+         * named expressions can check for conflicts.
+         */
+        if (val & (DEF_GLOBAL | DEF_NONLOCAL)) {
+            PyErr_Format(PyExc_SyntaxError,
+                NAMED_EXPR_COMP_INNER_LOOP_CONFLICT, name);
+            PyErr_SyntaxLocationObject(st->st_filename,
+                                       ste->ste_lineno,
+                                       ste->ste_col_offset + 1);
+            goto error;
+        }
+        val |= DEF_COMP_ITER;
+    }
     o = PyLong_FromLong(val);
     if (o == NULL)
         goto error;
@@ -1392,7 +1428,9 @@ static int
 symtable_extend_namedexpr_scope(struct symtable *st, expr_ty e)
 {
     assert(st->st_stack);
+    assert(e->kind == Name_kind);
 
+    PyObject *target_name = e->v.Name.id;
     Py_ssize_t i, size;
     struct _symtable_entry *ste;
     size = PyList_GET_SIZE(st->st_stack);
@@ -1402,32 +1440,42 @@ symtable_extend_namedexpr_scope(struct symtable *st, expr_ty e)
     for (i = size - 1; i >= 0; i--) {
         ste = (struct _symtable_entry *) PyList_GET_ITEM(st->st_stack, i);
 
-        /* If our current entry is a comprehension, skip it */
+        /* If we find a comprehension scope, check for a target
+         * binding conflict with iteration variables, otherwise skip it
+         */
         if (ste->ste_comprehension) {
+            long target_in_scope = _PyST_GetSymbol(ste, target_name);
+            if (target_in_scope & DEF_COMP_ITER) {
+                PyErr_Format(PyExc_SyntaxError, NAMED_EXPR_COMP_CONFLICT, target_name);
+                PyErr_SyntaxLocationObject(st->st_filename,
+                                            e->lineno,
+                                            e->col_offset);
+                VISIT_QUIT(st, 0);
+            }
             continue;
         }
 
         /* If we find a FunctionBlock entry, add as NONLOCAL/LOCAL */
         if (ste->ste_type == FunctionBlock) {
-            if (!symtable_add_def(st, e->v.Name.id, DEF_NONLOCAL))
+            if (!symtable_add_def(st, target_name, DEF_NONLOCAL))
                 VISIT_QUIT(st, 0);
-            if (!symtable_record_directive(st, e->v.Name.id, e->lineno, e->col_offset))
+            if (!symtable_record_directive(st, target_name, e->lineno, e->col_offset))
                 VISIT_QUIT(st, 0);
 
-            return symtable_add_def_helper(st, e->v.Name.id, DEF_LOCAL, ste);
+            return symtable_add_def_helper(st, target_name, DEF_LOCAL, ste);
         }
         /* If we find a ModuleBlock entry, add as GLOBAL */
         if (ste->ste_type == ModuleBlock) {
-            if (!symtable_add_def(st, e->v.Name.id, DEF_GLOBAL))
+            if (!symtable_add_def(st, target_name, DEF_GLOBAL))
                 VISIT_QUIT(st, 0);
-            if (!symtable_record_directive(st, e->v.Name.id, e->lineno, e->col_offset))
+            if (!symtable_record_directive(st, target_name, e->lineno, e->col_offset))
                 VISIT_QUIT(st, 0);
 
-            return symtable_add_def_helper(st, e->v.Name.id, DEF_GLOBAL, ste);
+            return symtable_add_def_helper(st, target_name, DEF_GLOBAL, ste);
         }
         /* Disallow usage in ClassBlock */
         if (ste->ste_type == ClassBlock) {
-            PyErr_Format(PyExc_TargetScopeError, NAMED_EXPR_COMP_IN_CLASS, e->v.Name.id);
+            PyErr_Format(PyExc_SyntaxError, NAMED_EXPR_COMP_IN_CLASS);
             PyErr_SyntaxLocationObject(st->st_filename,
                                         e->lineno,
                                         e->col_offset);
@@ -1443,6 +1491,26 @@ symtable_extend_namedexpr_scope(struct symtable *st, expr_ty e)
 }
 
 static int
+symtable_handle_namedexpr(struct symtable *st, expr_ty e)
+{
+    if (st->st_cur->ste_comp_iter_expr > 0) {
+        /* Assignment isn't allowed in a comprehension iterable expression */
+        PyErr_Format(PyExc_SyntaxError, NAMED_EXPR_COMP_ITER_EXPR);
+        PyErr_SyntaxLocationObject(st->st_filename,
+                                    e->lineno,
+                                    e->col_offset);
+        VISIT_QUIT(st, 0);
+    }
+    if (st->st_cur->ste_comprehension) {
+        /* Inside a comprehension body, so find the right target scope */
+        if (!symtable_extend_namedexpr_scope(st, e->v.NamedExpr.target))
+            VISIT_QUIT(st, 0);
+    }
+    VISIT(st, expr, e->v.NamedExpr.value);
+    VISIT(st, expr, e->v.NamedExpr.target);
+}
+
+static int
 symtable_visit_expr(struct symtable *st, expr_ty e)
 {
     if (++st->recursion_depth > st->recursion_limit) {
@@ -1452,12 +1520,7 @@ symtable_visit_expr(struct symtable *st, expr_ty e)
     }
     switch (e->kind) {
     case NamedExpr_kind:
-        if (st->st_cur->ste_comprehension) {
-            if (!symtable_extend_namedexpr_scope(st, e->v.NamedExpr.target))
-                VISIT_QUIT(st, 0);
-        }
-        VISIT(st, expr, e->v.NamedExpr.value);
-        VISIT(st, expr, e->v.NamedExpr.target);
+        symtable_handle_namedexpr(st, e);
         break;
     case BoolOp_kind:
         VISIT_SEQ(st, expr, e->v.BoolOp.values);
@@ -1739,8 +1802,12 @@ symtable_visit_alias(struct symtable *st, alias_ty a)
 static int
 symtable_visit_comprehension(struct symtable *st, comprehension_ty lc)
 {
+    st->st_cur->ste_comp_iter_target = 1;
     VISIT(st, expr, lc->target);
+    st->st_cur->ste_comp_iter_target = 0;
+    st->st_cur->ste_comp_iter_expr++;
     VISIT(st, expr, lc->iter);
+    st->st_cur->ste_comp_iter_expr--;
     VISIT_SEQ(st, expr, lc->ifs);
     if (lc->is_async) {
         st->st_cur->ste_coroutine = 1;
@@ -1788,7 +1855,9 @@ symtable_handle_comprehension(struct symtable *st, expr_ty e,
     comprehension_ty outermost = ((comprehension_ty)
                                     asdl_seq_GET(generators, 0));
     /* Outermost iterator is evaluated in current scope */
+    st->st_cur->ste_comp_iter_expr++;
     VISIT(st, expr, outermost->iter);
+    st->st_cur->ste_comp_iter_expr--;
     /* Create comprehension scope for the rest */
     if (!scope_name ||
         !symtable_enter_block(st, scope_name, FunctionBlock, (void *)e,
@@ -1805,7 +1874,11 @@ symtable_handle_comprehension(struct symtable *st, expr_ty e,
         symtable_exit_block(st, (void *)e);
         return 0;
     }
+    /* Visit iteration variable target, and mark them as such */
+    st->st_cur->ste_comp_iter_target = 1;
     VISIT(st, expr, outermost->target);
+    st->st_cur->ste_comp_iter_target = 0;
+    /* Visit the rest of the comprehension body */
     VISIT_SEQ(st, expr, outermost->ifs);
     VISIT_SEQ_TAIL(st, comprehension, generators, 1);
     if (value)