Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-40334: Produce better error messages for non-parenthesized genexps #20153

Merged
merged 5 commits into from May 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions Grammar/python.gram
Expand Up @@ -624,6 +624,9 @@ incorrect_arguments:
| args ',' '*' { RAISE_SYNTAX_ERROR("iterable argument unpacking follows keyword argument unpacking") }
| a=expression for_if_clauses ',' [args | expression for_if_clauses] {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "Generator expression must be parenthesized") }
| a=args for_if_clauses { _PyPegen_nonparen_genexp_in_call(p, a) }
| args ',' a=expression for_if_clauses {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "Generator expression must be parenthesized") }
| a=args ',' args { _PyPegen_arguments_parsing_error(p, a) }
invalid_kwarg:
| a=expression '=' {
Expand Down
8 changes: 3 additions & 5 deletions Lib/test/test_syntax.py
Expand Up @@ -216,11 +216,9 @@
>>> f(x for x in L, **{})
Traceback (most recent call last):
SyntaxError: Generator expression must be parenthesized

# >>> f(L, x for x in L)
# Traceback (most recent call last):
# SyntaxError: Generator expression must be parenthesized

>>> f(L, x for x in L)
Traceback (most recent call last):
SyntaxError: Generator expression must be parenthesized
>>> f(x for x in L, y for y in L)
Traceback (most recent call last):
SyntaxError: Generator expression must be parenthesized
Expand Down
50 changes: 50 additions & 0 deletions Parser/pegen/parse.c
Expand Up @@ -11602,6 +11602,8 @@ t_atom_rule(Parser *p)
// incorrect_arguments:
// | args ',' '*'
// | expression for_if_clauses ',' [args | expression for_if_clauses]
// | args for_if_clauses
// | args ',' expression for_if_clauses
// | args ',' args
static void *
incorrect_arguments_rule(Parser *p)
Expand Down Expand Up @@ -11663,6 +11665,54 @@ incorrect_arguments_rule(Parser *p)
}
p->mark = _mark;
}
{ // args for_if_clauses
if (p->error_indicator) {
return NULL;
}
expr_ty a;
asdl_seq* for_if_clauses_var;
if (
(a = args_rule(p)) // args
&&
(for_if_clauses_var = for_if_clauses_rule(p)) // for_if_clauses
)
{
_res = _PyPegen_nonparen_genexp_in_call ( p , a );
if (_res == NULL && PyErr_Occurred()) {
p->error_indicator = 1;
return NULL;
}
goto done;
}
p->mark = _mark;
}
{ // args ',' expression for_if_clauses
if (p->error_indicator) {
return NULL;
}
Token * _literal;
expr_ty a;
expr_ty args_var;
asdl_seq* for_if_clauses_var;
if (
(args_var = args_rule(p)) // args
&&
(_literal = _PyPegen_expect_token(p, 12)) // token=','
&&
(a = expression_rule(p)) // expression
&&
(for_if_clauses_var = for_if_clauses_rule(p)) // for_if_clauses
)
{
_res = RAISE_SYNTAX_ERROR_KNOWN_LOCATION ( a , "Generator expression must be parenthesized" );
if (_res == NULL && PyErr_Occurred()) {
p->error_indicator = 1;
return NULL;
}
goto done;
}
p->mark = _mark;
}
{ // args ',' args
if (p->error_indicator) {
return NULL;
Expand Down
21 changes: 21 additions & 0 deletions Parser/pegen/pegen.c
Expand Up @@ -2100,3 +2100,24 @@ void *_PyPegen_arguments_parsing_error(Parser *p, expr_ty e) {

return RAISE_SYNTAX_ERROR(msg);
}

void *
_PyPegen_nonparen_genexp_in_call(Parser *p, expr_ty args)
{
/* The rule that calls this function is 'args for_if_clauses'.
For the input f(L, x for x in y), L and x are in args and
the for is parsed as a for_if_clause. We have to check if
len <= 1, so that input like dict((a, b) for a, b in x)
gets successfully parsed and then we pass the last
argument (x in the above example) as the location of the
error */
Py_ssize_t len = asdl_seq_LEN(args->v.Call.args);
if (len <= 1) {
return NULL;
}

return RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
(expr_ty) asdl_seq_GET(args->v.Call.args, len - 1),
"Generator expression must be parenthesized"
);
}
3 changes: 2 additions & 1 deletion Parser/pegen/pegen.h
Expand Up @@ -152,7 +152,7 @@ RAISE_ERROR_KNOWN_LOCATION(Parser *p, PyObject *errtype, int lineno,
#define RAISE_SYNTAX_ERROR(msg, ...) _PyPegen_raise_error(p, PyExc_SyntaxError, msg, ##__VA_ARGS__)
#define RAISE_INDENTATION_ERROR(msg, ...) _PyPegen_raise_error(p, PyExc_IndentationError, msg, ##__VA_ARGS__)
#define RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, msg, ...) \
RAISE_ERROR_KNOWN_LOCATION(p, PyExc_SyntaxError, a->lineno, a->col_offset, msg, ##__VA_ARGS__)
RAISE_ERROR_KNOWN_LOCATION(p, PyExc_SyntaxError, (a)->lineno, (a)->col_offset, msg, ##__VA_ARGS__)

Py_LOCAL_INLINE(void *)
CHECK_CALL(Parser *p, void *result)
Expand Down Expand Up @@ -262,6 +262,7 @@ mod_ty _PyPegen_make_module(Parser *, asdl_seq *);
// Error reporting helpers
expr_ty _PyPegen_get_invalid_target(expr_ty e);
void *_PyPegen_arguments_parsing_error(Parser *, expr_ty);
void *_PyPegen_nonparen_genexp_in_call(Parser *p, expr_ty args);


void *_PyPegen_parse(Parser *);
Expand Down