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

serhiy-storchaka · 2017-11-12T22:43:27Z

https://mail.python.org/pipermail/python-dev/2017-November/150481.html

https://bugs.python.org/issue32012

ncoghlan

The test and compiler updates look good to me, just some minor grammar suggestions for the NEWS entry.

ncoghlan · 2017-11-13T00:53:41Z

Misc/NEWS.d/next/Core and Builtins/2017-11-13-00-37-11.bpo-32012.Kprjqe.rst

@@ -0,0 +1,4 @@
+SyntaxError now is raised when a generator expression without parenthesis is


Minor grammar fix: "now is" -> "is now"

ncoghlan · 2017-11-13T00:54:31Z

Misc/NEWS.d/next/Core and Builtins/2017-11-13-00-37-11.bpo-32012.Kprjqe.rst

@@ -0,0 +1,4 @@
+SyntaxError now is raised when a generator expression without parenthesis is
+passed as argument but followed by trailing comma. A generator expression


"... as an argument, but followed by a trailing ..."

serhiy-storchaka · 2017-11-13T07:30:31Z

Thank you @ncoghlan for your fixes.

ncoghlan

I think this version goes too far - decorator factories that accept generator expressions don't break any syntactic design principles, and we don't check for any other "definitely not a valid base class" expressions in base class lists.

(The latter change might be an interesting one to make, but it should be a separate enhancement request that checks for all the constructs with a guaranteed non-class return type, and explicitly considers the question of metaclasses that might be doing something unusual with the base class list)

ncoghlan · 2017-11-14T00:11:54Z

Doc/whatsnew/3.7.rst

@@ -638,10 +638,18 @@ Changes in Python behavior

      f(1 for x in [1],)

+      @deco(1 for x in [1])


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".

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.

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.

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.

ncoghlan · 2017-11-14T00:14:10Z

Doc/whatsnew/3.7.rst

+      def f():
+          pass
+
+      class C(1 for x in [1]):


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

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.

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.

bedevere-bot · 2017-11-14T00:17:44Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

benjaminp

This seems to disallow a lot more than trailing commas in call genexps.

benjaminp · 2017-11-14T07:15:32Z

Python/ast.c

@@ -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)


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

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’?

You need to #include <stdbool.h>

serhiy-storchaka · 2017-11-14T07:39:57Z

This seems to disallow a lot more than trailing commas in call genexps.

Only makes the implementation more conforming to the specification.

This reverts commit 355fea6.

This reverts commit 06b0abd.

serhiy-storchaka · 2017-11-14T16:16:41Z

I have made the requested changes; please review again.

bedevere-bot · 2017-11-14T16:16:44Z

Thanks for making the requested changes!

@ncoghlan: please review the changes made to this pull request.

ncoghlan

Thanks! We can follow up on the others on the issue tracker, and potentially take them to python-dev if Guido thinks that's appropriate.

serhiy-storchaka · 2017-11-15T07:12:03Z

Thank you for your review @ncoghlan and @benjaminp!

serhiy-storchaka · 2017-11-15T07:13:14Z

Created #4400 for fixing class definitions.

This expression is broken by: bpo-32012: Disallow trailing comma after genexpr without parenthesis. python/cpython#4382 python/cpython@9165f77

serhiy-storchaka added the type-feature A feature request or enhancement label Nov 12, 2017

the-knights-who-say-ni added the CLA signed label Nov 12, 2017

bedevere-bot added the awaiting merge label Nov 12, 2017

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

db4aafc

serhiy-storchaka force-pushed the gen-expr-argument-and-trailing-comma branch from b0bfcb7 to db4aafc Compare November 12, 2017 22:44

ncoghlan approved these changes Nov 13, 2017

View reviewed changes

Fix grammar of a news entry.

3d8c9bc

Disallow other improper syntax with generator expressions.

06b0abd

serhiy-storchaka requested review from benjaminp and ncoghlan November 13, 2017 15:12

ncoghlan requested changes Nov 14, 2017

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting merge labels Nov 14, 2017

benjaminp reviewed Nov 14, 2017

View reviewed changes

serhiy-storchaka added 2 commits November 14, 2017 09:37

Use booleans.

355fea6

Merge branch 'master' into gen-expr-argument-and-trailing-comma

8d81722

serhiy-storchaka added 3 commits November 14, 2017 17:04

06b0abd Revert "Use booleans."

19a8013

This reverts commit 355fea6.

Revert "Disallow other improper syntax with generator expressions."

691dfbf

This reverts commit 06b0abd.

Minor documentation fixes.

74439e4

bedevere-bot added awaiting change review and removed awaiting changes labels Nov 14, 2017

ncoghlan approved these changes Nov 15, 2017

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Nov 15, 2017

serhiy-storchaka merged commit 9165f77 into python:master Nov 15, 2017

bedevere-bot removed the awaiting merge label Nov 15, 2017

serhiy-storchaka mentioned this pull request Nov 15, 2017

bpo-32023: Disallow genexprs without parenthesis in class definitions. #4400

Merged

tacaswell added a commit to tacaswell/toolz that referenced this pull request Nov 16, 2017

FIX: compatibility with python3.7

77ac86f

This expression is broken by: bpo-32012: Disallow trailing comma after genexpr without parenthesis. python/cpython#4382 python/cpython@9165f77

tacaswell mentioned this pull request Nov 16, 2017

FIX: compatibility with python3.7 pytoolz/toolz#386

Merged

tacaswell added a commit to tacaswell/toolz that referenced this pull request Nov 16, 2017

FIX: compatibility with python3.7

5eb9fcb

This expression is broken by: bpo-32012: Disallow trailing comma after genexpr without parenthesis. python/cpython#4382 python/cpython@9165f77

serhiy-storchaka deleted the gen-expr-argument-and-trailing-comma branch September 22, 2018 14:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

serhiy-storchaka commented Nov 12, 2017 •

edited by bedevere-bot

Loading

ncoghlan left a comment

ncoghlan Nov 13, 2017

ncoghlan Nov 13, 2017

serhiy-storchaka commented Nov 13, 2017

ncoghlan left a comment

ncoghlan Nov 14, 2017

serhiy-storchaka Nov 14, 2017

ncoghlan Nov 14, 2017

serhiy-storchaka Nov 14, 2017

ncoghlan Nov 14, 2017

serhiy-storchaka Nov 14, 2017

ncoghlan Nov 14, 2017

bedevere-bot commented Nov 14, 2017

benjaminp left a comment

benjaminp Nov 14, 2017

serhiy-storchaka Nov 14, 2017

benjaminp Nov 14, 2017

serhiy-storchaka commented Nov 14, 2017

serhiy-storchaka commented Nov 14, 2017

bedevere-bot commented Nov 14, 2017

ncoghlan left a comment

serhiy-storchaka commented Nov 15, 2017

serhiy-storchaka commented Nov 15, 2017

		@@ -0,0 +1,4 @@
		SyntaxError now is raised when a generator expression without parenthesis is

		@@ -0,0 +1,4 @@
		SyntaxError now is raised when a generator expression without parenthesis is
		passed as argument but followed by trailing comma. A generator expression

		@@ -638,10 +638,18 @@ Changes in Python behavior

		f(1 for x in [1],)

		@deco(1 for x in [1])

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

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

Conversation

serhiy-storchaka commented Nov 12, 2017 • edited by bedevere-bot Loading

ncoghlan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhiy-storchaka commented Nov 13, 2017

ncoghlan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bedevere-bot commented Nov 14, 2017

benjaminp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhiy-storchaka commented Nov 14, 2017

serhiy-storchaka commented Nov 14, 2017

bedevere-bot commented Nov 14, 2017

ncoghlan left a comment

Choose a reason for hiding this comment

serhiy-storchaka commented Nov 15, 2017

serhiy-storchaka commented Nov 15, 2017

serhiy-storchaka commented Nov 12, 2017 •

edited by bedevere-bot

Loading