Disallow ambiguous syntax f(x for x in [1],)

[serhiy-storchaka]

BPO	32012
Nosy	@gvanrossum, @brettcannon, @ncoghlan, @benjaminp, @serhiy-storchaka, @cryvate, @miss-islington
PRs	bpo-32012: Disallow trailing comma after genexpr without parenthesis. #4382 bpo-32023: Disallow genexprs without parenthesis in class definitions. #4400 [3.7] Revert "closes bpo-27494: Fix 2to3 handling of trailing comma after a generator expression (GH-3771)" (GH-8241) #8580

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-11-15.08:16:39.988>
created_at = <Date 2017-11-12.22:29:02.776>
labels = ['interpreter-core', 'type-feature', '3.7']
title = 'Disallow ambiguous syntax f(x for x in [1],)'
updated_at = <Date 2018-07-31.06:52:51.418>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2018-07-31.06:52:51.418>
actor = 'miss-islington'
assignee = 'none'
closed = True
closed_date = <Date 2017-11-15.08:16:39.988>
closer = 'ncoghlan'
components = ['Interpreter Core']
creation = <Date 2017-11-12.22:29:02.776>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 32012
keywords = ['patch']
message_count = 24.0
messages = ['306132', '306145', '306149', '306159', '306160', '306161', '306179', '306185', '306198', '306199', '306200', '306204', '306206', '306207', '306208', '306209', '306211', '306212', '306213', '306233', '306255', '306256', '322730', '322733']
nosy_count = 7.0
nosy_names = ['gvanrossum', 'brett.cannon', 'ncoghlan', 'benjamin.peterson', 'serhiy.storchaka', 'cryvate', 'miss-islington']
pr_nums = ['4382', '4400', '8580']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue32012'
versions = ['Python 3.7']

[serhiy-storchaka]

The syntax f(x for x in [1],) is ambiguous and reasons that allow it (omitting parenthesis in generator expression and using trailing comma in call expression) are not applicable in this case. Rationales see on Python-Dev: https://mail.python.org/pipermail/python-dev/2017-November/150481.html.

[serhiy-storchaka]

PR 4382 doesn't change the grammar, it changes only checks in the CST to AST transformer. Maybe it would be better to change the grammar. Currently it doesn't match the language specification and allows the following constructions:

@deco(x for x in [1])
def f(): ...

    class C(x for x in [1]): ...

And I think the part of bpo-27494 should be reverted.

[serhiy-storchaka]

It is easy to forbid the above cases, but I don't know what error message is appropriate. General "invalid syntax"?

[Cryvate]

Currently,

Class C(*some_classes): ...

works 'as expected' and is within grammar and language specification whereas

Class C(x for x in [object]): ...

does not work but does not cause a syntax error. I can see a use case for both in dynamic class factories. I was going to do this, but was thwarted by another issue (doc cannot be assigned after creation, nor can it be defined as anything but a pure string, any work around or reason that is the case? Not true for e.g. functions).

I think having one of these within the language specification and not the other is odd.

[Cryvate]

[As a follow-on, should I open a new issue/discuss on python-dev? Willing to help out with a solution on way or another! I know https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence, "In my head" <> "Should be the case" etc. very much applies.]

In my head

    @...
    def foo(): pass

is equivalent to

    def _foo(): pass
    foo = ...()
    del _foo

However the following shows this is not the case:

    @0
    def foo(): pass

throws a syntax error, whereas

    def _foo(): pass
    foo = 0(_foo)

throws a type error. This might seem silly, but it is still unexpected.

https://docs.python.org/3/reference/compound_stmts.html#grammar-token-decorator has

decorator ::= "@" dotted_name ["(" [argument_list [","]] ")"] NEWLINE

which in my head is

decorator ::= "@" atom NEWLINE

Similarly for classes: https://docs.python.org/3/reference/compound_stmts.html#class-definitions

inheritance ::= "(" [argument_list] ")"

which allows for keyword arguments (does that make any sense!?). In my head it is (compare with call: https://docs.python.org/3/reference/expressions.html#calls)

inheritance ::= "(" [positional_arguments [","] | comprehension] ")"

[Tangentially related, this is how I originally got onto the mailing lists, my unhappiness with the definition of the for statement (https://docs.python.org/3/reference/compound_stmts.html#the-for-statement):

for_stmt ::= "for" target_list "in" expression_list ":" suite ["else" ":" suite]

Which I would expect to be:

for_stmt ::= comp_for ":" suite ["else" ":" suite]

so you could e.g. have if statements.
]

[serhiy-storchaka]

I think this issue is not the best way for answering your question, but I will make a try.

The fact that "class C(x for x in [object]): ..." does not cause a syntax error is a bug. This issue fixes it. The fact that corrected "class C((x for x in [object])): ..." doesn't work is expected, because a generator instance is not a class.

The equivalence between a decorator expression and explicit calling a decorator function is true only in one direction and only for valid Python syntax. Saying about equivalence of syntactically incorrect Python code doesn't make sense.

Yes, an inheritance list can contain keyword arguments. They are passed to a metaclass constructor as well as positional arguments.

The syntaxes of the for statement and comprehensions are different.

[ncoghlan]

In a function call, f(x for x in iterable) is roughly equivalent to f(iter(iterable)), not f(*iterable) (the genexp based equivalent of the latter would be ``f(*(x for x in iterable))`).

Thus the base class list is no different from any other argument list in this case - it's just that generator objects aren't valid base classes.

Getting back on topic for this particular bug fix though: as noted in my last PR review, I think the latest version goes too far by disallowing `@deco(x for x in iterable)` and `class C(x for x in iterable):`. While semantically questionable, there's nothing *syntactically* invalid about those - they pass a single generator expression, and that generator expression is correctly surrounded by parentheses. There's no more reason to prohibit a genexp in either of those situations at compile time than there is to prohibit a list comprehension.

[serhiy-storchaka]

The problem with these constructions is that they are not allowed by the Python language specification. It should be explicitly changed for allowing them. And this change should be accepted by Guido.

[ncoghlan]

I created https://bugs.python.org/issue32023 to explicitly cover the base class list case, and after checking the language spec, I agree that case should be a syntax error.

However, @deco(x for x in []) should not be a syntax error, as:

• it's a call with one argument, so the genexp parentheses can be omitted as described in https://docs.python.org/3/reference/expressions.html#generator-expressions
• it matches the "@dotted_name(arg_list)" pattern permitted by https://docs.python.org/3/reference/compound_stmts.html#function-definitions

[serhiy-storchaka]

No, it doesn't match the "@dotted_name(arg_list)" pattern.

decorator: "@" dotted_name ["(" [argument_list [","]] ")"] NEWLINE
call: primary "(" [argument_list [","] | comprehension] ")"
argument_list: positional_arguments ["," starred_and_keywords]
: ["," keywords_arguments]
: | starred_and_keywords ["," keywords_arguments]
: | keywords_arguments

The call syntax contains a special case for generator expression. The decorator expression syntax dosn't contain it. You should change the grammar rule to

decorator: "@" dotted_name ["(" [argument_list [","] | comprehension] ")"] NEWLINE

for supporting this syntax. Please open a separate topic on Python-Dev for discussing this language change.

[Cryvate]

I think this showcases how difficult it is to get this right, requires carefully reading the EBNF language spec, not just the text, and the behaviour is unexpected.

[skrah]

I think "ambiguous" is not the right word. If a single argument
can be a non-parenthesized generator and all arguments can be
followed by a trailing comma, it's clear.

The language spec is often behind in my experience.