From 66b212d7bbe86c7b272ad9df649c9e2b57584901 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Tue, 11 Sep 2018 23:13:04 -0700 Subject: [PATCH 1/5] bpo-34641: Further restrict the LHS of keyword argument function call syntax. --- Lib/test/test_syntax.py | 3 + .../2018-09-11-23-12-33.bpo-34641.gFBCc9.rst | 2 + Python/ast.c | 56 ++++++++++++++----- 3 files changed, 46 insertions(+), 15 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-09-11-23-12-33.bpo-34641.gFBCc9.rst diff --git a/Lib/test/test_syntax.py b/Lib/test/test_syntax.py index fa1e7aa5d4f27d..c5b2496e010d18 100644 --- 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 new file mode 100644 index 00000000000000..a123167714f066 --- /dev/null +++ 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((arg)=default)`` is now disallowed. diff --git a/Python/ast.c b/Python/ast.c index 94962e00c7d33e..aa467de7158e71 100644 --- a/Python/ast.c +++ b/Python/ast.c @@ -2815,29 +2815,55 @@ 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)) { From f609faa51936f1e6d3b53a50a40e2d4b03f46a7e Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Wed, 12 Sep 2018 06:53:24 -0700 Subject: [PATCH 2/5] fix bracing --- Python/ast.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Python/ast.c b/Python/ast.c index aa467de7158e71..b93eb88dae7fce 100644 --- a/Python/ast.c +++ b/Python/ast.c @@ -2852,7 +2852,8 @@ ast_for_call(struct compiling *c, const node *n, expr_ty func, bool allowgen) ast_error(c, chch, "lambda cannot contain assignment"); return NULL; - } else if (TYPE(expr_node) != NAME) { + } + else if (TYPE(expr_node) != NAME) { ast_error(c, chch, "keyword can't be an expression"); return NULL; From 6fee37d9c89c1033ed35ccdc3cacc5847b788ac1 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Wed, 12 Sep 2018 06:53:36 -0700 Subject: [PATCH 3/5] Improve example message. --- .../Core and Builtins/2018-09-11-23-12-33.bpo-34641.gFBCc9.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index a123167714f066..9b6eb24c138c48 100644 --- 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 @@ -1,2 +1,2 @@ Further restrict the syntax of the left-hand side of keyword arguments in -function calls. In particular, ``f((arg)=default)`` is now disallowed. +function calls. In particular, ``f((keyword)=arg)`` is now disallowed. From 8f9c904850b3e7e52629ebd09f7afed6551798b5 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Wed, 12 Sep 2018 06:57:55 -0700 Subject: [PATCH 4/5] update whatsnew --- Doc/whatsnew/3.8.rst | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 259dcf65f2e048..1da9ccfe4ab57b 100644 --- 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 `. (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 =========== @@ -102,7 +106,7 @@ New Modules Improved Modules -================ +===============r= Optimizations ============= From d097ea58753bfd8d24c3c7eb6b4942199afc55f6 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Wed, 12 Sep 2018 15:02:59 -0700 Subject: [PATCH 5/5] fix stray character --- Doc/whatsnew/3.8.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 1da9ccfe4ab57b..b2475c7df33e06 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -106,7 +106,7 @@ New Modules Improved Modules -===============r= +================ Optimizations =============