From 546cefcda75d7150b55c8bc1724bea35a1e12890 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Salgado Date: Fri, 19 Nov 2021 23:11:57 +0000 Subject: [PATCH] bpo-45727: Make the syntax error for missing comma more consistent (GH-29427) --- Grammar/python.gram | 4 ++-- Lib/test/test_exceptions.py | 1 + Parser/parser.c | 42 ++++++++++++++++++------------------- Parser/pegen.c | 4 +++- 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/Grammar/python.gram b/Grammar/python.gram index d1901a5a9a2..168bb3c05e6 100644 --- a/Grammar/python.gram +++ b/Grammar/python.gram @@ -618,6 +618,7 @@ expressions[expr_ty]: expression[expr_ty] (memo): | invalid_expression + | invalid_legacy_expression | a=disjunction 'if' b=disjunction 'else' c=expression { _PyAST_IfExp(b, a, c, EXTRA) } | disjunction | lambdef @@ -1080,11 +1081,10 @@ invalid_legacy_expression: "Missing parentheses in call to '%U'. Did you mean %U(...)?", a->v.Name.id, a->v.Name.id) : NULL} invalid_expression: - | invalid_legacy_expression # !(NAME STRING) is not matched so we don't show this error with some invalid string prefixes like: kf"dsfsdf" # Soft keywords need to also be ignored because they can be parsed as NAME NAME | !(NAME STRING | SOFT_KEYWORD) a=disjunction b=expression_without_invalid { - RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "invalid syntax. Perhaps you forgot a comma?") } + _PyPegen_check_legacy_stmt(p, a) ? NULL : RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "invalid syntax. Perhaps you forgot a comma?") } | a=disjunction 'if' b=disjunction !('else'|':') { RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "expected 'else' after 'if' expression") } invalid_named_expression: diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 098804fad5e..1341f77ac45 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -233,6 +233,7 @@ class ExceptionTests(unittest.TestCase): check('[file for\n str(file) in []]', 2, 2) check("ages = {'Alice'=22, 'Bob'=23}", 1, 16) check('match ...:\n case {**rest, "key": value}:\n ...', 2, 19) + check("a b c d e f", 1, 1) # Errors thrown by compile.c check('class foo:return 1', 1, 11) diff --git a/Parser/parser.c b/Parser/parser.c index b508c1ddec1..87f492d50c6 100644 --- a/Parser/parser.c +++ b/Parser/parser.c @@ -9719,6 +9719,7 @@ expressions_rule(Parser *p) // expression: // | invalid_expression +// | invalid_legacy_expression // | disjunction 'if' disjunction 'else' expression // | disjunction // | lambdef @@ -9764,6 +9765,25 @@ expression_rule(Parser *p) D(fprintf(stderr, "%*c%s expression[%d-%d]: %s failed!\n", p->level, ' ', p->error_indicator ? "ERROR!" : "-", _mark, p->mark, "invalid_expression")); } + if (p->call_invalid_rules) { // invalid_legacy_expression + if (p->error_indicator) { + D(p->level--); + return NULL; + } + D(fprintf(stderr, "%*c> expression[%d-%d]: %s\n", p->level, ' ', _mark, p->mark, "invalid_legacy_expression")); + void *invalid_legacy_expression_var; + if ( + (invalid_legacy_expression_var = invalid_legacy_expression_rule(p)) // invalid_legacy_expression + ) + { + D(fprintf(stderr, "%*c+ expression[%d-%d]: %s succeeded!\n", p->level, ' ', _mark, p->mark, "invalid_legacy_expression")); + _res = invalid_legacy_expression_var; + goto done; + } + p->mark = _mark; + D(fprintf(stderr, "%*c%s expression[%d-%d]: %s failed!\n", p->level, ' ', + p->error_indicator ? "ERROR!" : "-", _mark, p->mark, "invalid_legacy_expression")); + } { // disjunction 'if' disjunction 'else' expression if (p->error_indicator) { D(p->level--); @@ -18239,7 +18259,6 @@ invalid_legacy_expression_rule(Parser *p) } // invalid_expression: -// | invalid_legacy_expression // | !(NAME STRING | SOFT_KEYWORD) disjunction expression_without_invalid // | disjunction 'if' disjunction !('else' | ':') static void * @@ -18252,25 +18271,6 @@ invalid_expression_rule(Parser *p) } void * _res = NULL; int _mark = p->mark; - if (p->call_invalid_rules) { // invalid_legacy_expression - if (p->error_indicator) { - D(p->level--); - return NULL; - } - D(fprintf(stderr, "%*c> invalid_expression[%d-%d]: %s\n", p->level, ' ', _mark, p->mark, "invalid_legacy_expression")); - void *invalid_legacy_expression_var; - if ( - (invalid_legacy_expression_var = invalid_legacy_expression_rule(p)) // invalid_legacy_expression - ) - { - D(fprintf(stderr, "%*c+ invalid_expression[%d-%d]: %s succeeded!\n", p->level, ' ', _mark, p->mark, "invalid_legacy_expression")); - _res = invalid_legacy_expression_var; - goto done; - } - p->mark = _mark; - D(fprintf(stderr, "%*c%s invalid_expression[%d-%d]: %s failed!\n", p->level, ' ', - p->error_indicator ? "ERROR!" : "-", _mark, p->mark, "invalid_legacy_expression")); - } { // !(NAME STRING | SOFT_KEYWORD) disjunction expression_without_invalid if (p->error_indicator) { D(p->level--); @@ -18288,7 +18288,7 @@ invalid_expression_rule(Parser *p) ) { D(fprintf(stderr, "%*c+ invalid_expression[%d-%d]: %s succeeded!\n", p->level, ' ', _mark, p->mark, "!(NAME STRING | SOFT_KEYWORD) disjunction expression_without_invalid")); - _res = RAISE_SYNTAX_ERROR_KNOWN_RANGE ( a , b , "invalid syntax. Perhaps you forgot a comma?" ); + _res = _PyPegen_check_legacy_stmt ( p , a ) ? NULL : RAISE_SYNTAX_ERROR_KNOWN_RANGE ( a , b , "invalid syntax. Perhaps you forgot a comma?" ); if (_res == NULL && PyErr_Occurred()) { p->error_indicator = 1; D(p->level--); diff --git a/Parser/pegen.c b/Parser/pegen.c index 15879a64f27..09c1a19a793 100644 --- a/Parser/pegen.c +++ b/Parser/pegen.c @@ -79,7 +79,9 @@ _PyPegen_check_barry_as_flufl(Parser *p, Token* t) { int _PyPegen_check_legacy_stmt(Parser *p, expr_ty name) { - assert(name->kind == Name_kind); + if (name->kind != Name_kind) { + return 0; + } const char* candidates[2] = {"print", "exec"}; for (int i=0; i<2; i++) { if (PyUnicode_CompareWithASCIIString(name->v.Name.id, candidates[i]) == 0) {