closes bpo-34641: Further restrict the LHS of keyword argument function call syntax. (GH-9212)

benjaminp · web-flow · commit c9a71dd223c4 · 2018-09-12T17:14:39.000-07:00
diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst
@@ -94,6 +94,10 @@ Other Language Changes
 * Added support of ``\N{name}`` escapes in :mod:`regular expressions <re>`.
   (Contributed by Jonathan Eunice and Serhiy Storchaka in :issue:`30688`.)
 
+* The syntax allowed for keyword names in function calls was further
+  restricted. In particular, ``f((keyword)=arg)`` is no longer allowed. It was
+  never intended to permit more than a bare name on the left-hand side of a
+  keyword argument assignment term. See :issue:`34641`.
 
 New Modules
 ===========
diff --git a/Lib/test/test_syntax.py b/Lib/test/test_syntax.py
@@ -269,6 +269,9 @@
 >>> f(x.y=1)
 Traceback (most recent call last):
 SyntaxError: keyword can't be an expression
+>>> f((x)=2)
+Traceback (most recent call last):
+SyntaxError: keyword can't be an expression
 
 
 More set_context():
diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-09-11-23-12-33.bpo-34641.gFBCc9.rst b/Misc/NEWS.d/next/Core and Builtins/2018-09-11-23-12-33.bpo-34641.gFBCc9.rst
@@ -0,0 +1,2 @@
+Further restrict the syntax of the left-hand side of keyword arguments in
+function calls. In particular, ``f((keyword)=arg)`` is now disallowed.
diff --git a/Python/ast.c b/Python/ast.c
@@ -2815,29 +2815,56 @@ ast_for_call(struct compiling *c, const node *n, expr_ty func, bool allowgen)
                 identifier key, tmp;
                 int k;
 
-                /* chch is test, but must be an identifier? */
-                e = ast_for_expr(c, chch);
-                if (!e)
-                    return NULL;
-                /* f(lambda x: x[0] = 3) ends up getting parsed with
-                 * LHS test = lambda x: x[0], and RHS test = 3.
-                 * SF bug 132313 points out that complaining about a keyword
-                 * then is very confusing.
-                 */
-                if (e->kind == Lambda_kind) {
+                // To remain LL(1), the grammar accepts any test (basically, any
+                // expression) in the keyword slot of a call site.  So, we need
+                // to manually enforce that the keyword is a NAME here.
+                static const int name_tree[] = {
+                    test,
+                    or_test,
+                    and_test,
+                    not_test,
+                    comparison,
+                    expr,
+                    xor_expr,
+                    and_expr,
+                    shift_expr,
+                    arith_expr,
+                    term,
+                    factor,
+                    power,
+                    atom_expr,
+                    atom,
+                    0,
+                };
+                node *expr_node = chch;
+                for (int i = 0; name_tree[i]; i++) {
+                    if (TYPE(expr_node) != name_tree[i])
+                        break;
+                    if (NCH(expr_node) != 1)
+                        break;
+                    expr_node = CHILD(expr_node, 0);
+                }
+                if (TYPE(expr_node) == lambdef) {
+                    // f(lambda x: x[0] = 3) ends up getting parsed with LHS
+                    // test = lambda x: x[0], and RHS test = 3.  Issue #132313
+                    // points out that complaining about a keyword then is very
+                    // confusing.
                     ast_error(c, chch,
                             "lambda cannot contain assignment");
                     return NULL;
                 }
-                else if (e->kind != Name_kind) {
+                else if (TYPE(expr_node) != NAME) {
                     ast_error(c, chch,
-                            "keyword can't be an expression");
+                              "keyword can't be an expression");
+                    return NULL;
+                }
+                key = new_identifier(STR(expr_node), c);
+                if (key == NULL) {
                     return NULL;
                 }
-                else if (forbidden_name(c, e->v.Name.id, ch, 1)) {
+                if (forbidden_name(c, key, chch, 1)) {
                     return NULL;
                 }
-                key = e->v.Name.id;
                 for (k = 0; k < nkeywords; k++) {
                     tmp = ((keyword_ty)asdl_seq_GET(keywords, k))->arg;
                     if (tmp && !PyUnicode_Compare(tmp, key)) {

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Further restrict the syntax of the left-hand side of keyword arguments in`
	`2`	+function calls. In particular, ``f((keyword)=arg)`` is now disallowed.