Skip to content

Commit

Permalink
bpo-40334: Produce better error messages for non-parenthesized genexps (
Browse files Browse the repository at this point in the history
pythonGH-20153)

The error message, generated for a non-parenthesized generator expression
in function calls, was still the generic `invalid syntax`, when the generator expression wasn't appearing as the first argument in the call. With this patch, even on input like `f(a, b, c for c in d, e)`, the correct error message gets produced.
  • Loading branch information
lysnikolaou authored May 22, 2020
1 parent 7864f11 commit ae14583
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 6 deletions.
3 changes: 3 additions & 0 deletions Grammar/python.gram
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -11670,6 +11670,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 @@ -11731,6 +11733,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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

0 comments on commit ae14583

Please sign in to comment.