Skip to content

bpo-32012: Disallow trailing comma after genexpr without parenthesis. #4382

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Nov 15, 2017
Merged

bpo-32012: Disallow trailing comma after genexpr without parenthesis. #4382

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
db4aafc
bpo-32012: Disallow trailing comma after genexpr without parenthesis.
serhiy-storchaka Nov 12, 2017
3d8c9bc
Fix grammar of a news entry.
serhiy-storchaka Nov 13, 2017
06b0abd
Disallow other improper syntax with generator expressions.
serhiy-storchaka Nov 13, 2017
355fea6
Use booleans.
serhiy-storchaka Nov 14, 2017
8d81722
Merge branch 'master' into gen-expr-argument-and-trailing-comma
serhiy-storchaka Nov 14, 2017
19a8013
06b0abd Revert "Use booleans."
serhiy-storchaka Nov 14, 2017
691dfbf
Revert "Disallow other improper syntax with generator expressions."
serhiy-storchaka Nov 14, 2017
74439e4
Minor documentation fixes.
serhiy-storchaka Nov 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions Doc/whatsnew/3.7.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,28 @@ This section lists previously described changes and other bugfixes
that may require changes to your code.


Changes in Python behavior
--------------------------

* Due to an oversight, earlier Python versions erroneously accepted the
following syntax::

f(1 for x in [1],)

@deco(1 for x in [1])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with this one? While we've restricted the form that decorator expressions take to being a name (dotted or otherwise) plus an optional function call, we've never restricted the arguments passed to that function call.

At the generator expression level, it also still meets the "must be surrounded by parentheses rule".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as with

@deco[0](1)

or

@deco(0)(1)

or

@(a+b*c)

This looks like valid Python expression syntax, but the syntax of decorator expression is intentionally more limited.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This aspect of the change is more akin to prohibiting @deco(a+b*c) - once the top level expression is in the form of a function call, the fact that it's part of a decorator expression shouldn't be altering what's permitted in the argument list.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case these syntax changes should be discussed and officially approved. Could you please open a topic on Python-Dev about omitting parenthesis in a generator expression in these two cases? I have arguments for these changes and against them, but I think you will formulate them better. I'll add my comments.

Despite the fact that currently the CPython parser allows such syntax, it is illegal, and allowing it officially is a changing of Python language specification.

def f():
pass

class C(1 for x in [1]):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one seems a bit odd to me as well, as we don't disallow other expressions in base class lists at compile time just because we know they can't produce a valid base class:

>>> class C([]): pass
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: list() takes at most 1 argument (3 given)
>>> class C({}): pass
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: dict expected at most 1 arguments, got 3

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still can use

>>> class C((1 for x in [1])): pass
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot create 'generator' instances

Omitting parenthesis in a generator expression is a special case for improving readability when pass a sole generator expression argument. The syntax of class definition doesn't have such special case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I think there's merit to that argument, I don't think it belongs in the same PR (or issue) as the one that addresses the problem with mishandling trailing commas in function calls. Unlike the trailing comma case, there's no syntactic ambiguity being addressed.

pass

Python 3.7 now correctly raises a :exc:`SyntaxError`, as a generator
expression always needs to be directly inside a set of parentheses
and cannot have a comma on either side, and the duplication of the
parentheses can be omitted only on calls.
(Contributed by Serhiy Storchaka in :issue:`32012`.)


Changes in the Python API
-------------------------

Expand Down
32 changes: 29 additions & 3 deletions Lib/test/test_syntax.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,19 +125,45 @@

From ast_for_call():

>>> def f(it, *varargs):
>>> def f(it, *varargs, **kwargs):
... return list(it)
>>> L = range(10)
>>> f(x for x in L)
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
>>> f(x for x in L, 1)
Traceback (most recent call last):
SyntaxError: Generator expression must be parenthesized if not sole argument
SyntaxError: Generator expression must be parenthesized
>>> f(x for x in L, y=1)
Traceback (most recent call last):
SyntaxError: Generator expression must be parenthesized
>>> f(x for x in L, *[])
Traceback (most recent call last):
SyntaxError: Generator expression must be parenthesized
>>> 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(x for x in L, y for y in L)
Traceback (most recent call last):
SyntaxError: Generator expression must be parenthesized if not sole argument
SyntaxError: Generator expression must be parenthesized
>>> f(x for x in L,)
Traceback (most recent call last):
SyntaxError: Generator expression must be parenthesized
>>> f((x for x in L), 1)
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
>>> def deco(*args):
... return lambda f: f
>>> @deco(x for x in L)
... def g(*args, **kwargs):
... pass
Traceback (most recent call last):
SyntaxError: invalid syntax
>>> class C(x for x in L):
... pass
Traceback (most recent call last):
SyntaxError: invalid syntax

>>> def g(*args, **kwargs):
... print(args, sorted(kwargs.items()))
Expand Down
7 changes: 7 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2017-11-13-00-37-11.bpo-32012.Kprjqe.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
SyntaxError is now correctly raised when a generator expression without
parenthesis is passed as an argument, but followed by a trailing comma, or
it is used instead of an argument list in a decorator expression, or
instead of an inheritance list in a class definition. A generator expression
always needs to be directly inside a set of parentheses and cannot have a
comma on either side. The duplication of the parentheses can be omitted
only on calls.
33 changes: 18 additions & 15 deletions Python/ast.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ static stmt_ty ast_for_with_stmt(struct compiling *, const node *, int);
static stmt_ty ast_for_for_stmt(struct compiling *, const node *, int);

/* Note different signature for ast_for_call */
static expr_ty ast_for_call(struct compiling *, const node *, expr_ty);
static expr_ty ast_for_call(struct compiling *, const node *, expr_ty, int);

static PyObject *parsenumber(struct compiling *, const char *);
static expr_ty parsestrplus(struct compiling *, const node *n);
Expand Down Expand Up @@ -1545,7 +1545,7 @@ ast_for_decorator(struct compiling *c, const node *n)
name_expr = NULL;
}
else {
d = ast_for_call(c, CHILD(n, 3), name_expr);
d = ast_for_call(c, CHILD(n, 3), name_expr, 0);
if (!d)
return NULL;
name_expr = NULL;
Expand Down Expand Up @@ -2368,7 +2368,7 @@ ast_for_trailer(struct compiling *c, const node *n, expr_ty left_expr)
return Call(left_expr, NULL, NULL, LINENO(n),
n->n_col_offset, c->c_arena);
else
return ast_for_call(c, CHILD(n, 1), left_expr);
return ast_for_call(c, CHILD(n, 1), left_expr, 1);
}
else if (TYPE(CHILD(n, 0)) == DOT) {
PyObject *attr_id = NEW_IDENTIFIER(CHILD(n, 1));
Expand Down Expand Up @@ -2705,14 +2705,14 @@ ast_for_expr(struct compiling *c, const node *n)
}

static expr_ty
ast_for_call(struct compiling *c, const node *n, expr_ty func)
ast_for_call(struct compiling *c, const node *n, expr_ty func, int allowgen)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allowgen should be bool parameter and the callers should pass true or false.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

Python/ast.c:614:72: error: unknown type name ‘bool’; did you mean ‘_Bool’?
Python/ast.c:1548:53: error: ‘false’ undeclared (first use in this function); did you mean ‘fabsl’?
Python/ast.c:2371:60: error: ‘true’ undeclared (first use in this function); did you mean ‘time’?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to #include <stdbool.h>

{
/*
arglist: argument (',' argument)* [',']
argument: ( test [comp_for] | '*' test | test '=' test | '**' test )
*/

int i, nargs, nkeywords, ngens;
int i, nargs, nkeywords;
int ndoublestars;
asdl_seq *args;
asdl_seq *keywords;
Expand All @@ -2721,28 +2721,31 @@ ast_for_call(struct compiling *c, const node *n, expr_ty func)

nargs = 0;
nkeywords = 0;
ngens = 0;
for (i = 0; i < NCH(n); i++) {
node *ch = CHILD(n, i);
if (TYPE(ch) == argument) {
if (NCH(ch) == 1)
nargs++;
else if (TYPE(CHILD(ch, 1)) == comp_for)
ngens++;
else if (TYPE(CHILD(ch, 1)) == comp_for) {
nargs++;
if (!allowgen) {
ast_error(c, ch, "invalid syntax");
return NULL;
}
if (NCH(n) > 1) {
ast_error(c, ch, "Generator expression must be parenthesized");
return NULL;
}
}
else if (TYPE(CHILD(ch, 0)) == STAR)
nargs++;
else
/* TYPE(CHILD(ch, 0)) == DOUBLESTAR or keyword argument */
nkeywords++;
}
}
if (ngens > 1 || (ngens && (nargs || nkeywords))) {
ast_error(c, n, "Generator expression must be parenthesized "
"if not sole argument");
return NULL;
}

args = _Py_asdl_seq_new(nargs + ngens, c->c_arena);
args = _Py_asdl_seq_new(nargs, c->c_arena);
if (!args)
return NULL;
keywords = _Py_asdl_seq_new(nkeywords, c->c_arena);
Expand Down Expand Up @@ -3974,7 +3977,7 @@ ast_for_classdef(struct compiling *c, const node *n, asdl_seq *decorator_seq)
if (!dummy_name)
return NULL;
dummy = Name(dummy_name, Load, LINENO(n), n->n_col_offset, c->c_arena);
call = ast_for_call(c, CHILD(n, 3), dummy);
call = ast_for_call(c, CHILD(n, 3), dummy, 0);
if (!call)
return NULL;
}
Expand Down