msg306132 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-11-12 22:29 |
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.
|
msg306145 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-11-13 07:35 |
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 issue27494 should be reverted.
|
msg306149 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-11-13 08:24 |
It is easy to forbid the above cases, but I don't know what error message is appropriate. General "invalid syntax"?
|
msg306159 - (view) |
Author: Henk-Jaap Wagenaar (cryvate) * |
Date: 2017-11-13 15:19 |
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.
|
msg306160 - (view) |
Author: Henk-Jaap Wagenaar (cryvate) * |
Date: 2017-11-13 15:19 |
[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.
]
|
msg306161 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-11-13 16:10 |
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.
|
msg306179 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2017-11-14 00:26 |
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.
|
msg306185 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-11-14 06:07 |
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.
|
msg306198 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2017-11-14 10:28 |
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
|
msg306199 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-11-14 10:43 |
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.
|
msg306200 - (view) |
Author: Henk-Jaap Wagenaar (cryvate) * |
Date: 2017-11-14 11:40 |
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.
|
msg306204 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-11-14 13:03 |
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.
|
msg306206 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2017-11-14 13:12 |
OK, I've filed https://bugs.python.org/issue32024 to cover the decorator syntax discrepancy.
So I'd still prefer to restrict the patch for *this* issue to just the genuinely ambiguous case, and leave the unambiguous-but-inconsistent-with-the-language-spec cases to their respective issues.
|
msg306207 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-11-14 13:17 |
I would prefer to do nothing about the subject of this issue. I still
don't see any ambiguity, except in a very broad colloquial sense.
Why introduce another special case?
|
msg306208 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2017-11-14 13:31 |
If limited to the original scope, this isn't a new special case, it's fixing a bug in the implementation of the existing special case (where it's ignoring the trailing comma when it shouldn't be).
If it hadn't been for the scope creep to include a couple of other cases where the implementation is arguably more permissive than the language spec says it should, it would have already been merged.
|
msg306209 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-11-14 13:43 |
On Tue, Nov 14, 2017 at 01:31:52PM +0000, Nick Coghlan wrote:
> If limited to the original scope, this isn't a new special case, it's fixing a bug in the implementation of the existing special case (where it's ignoring the trailing comma when it shouldn't be).
This ignores the trailing comma:
f([1,2,3],)
And this:
f(x for x in [1,2,3],)
Seems logical to me.
Do you want to allow the 1,2 to be read as a tuple?
f(x for x in 1,2)
|
msg306211 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-11-14 13:55 |
I would prefer to fix all related cases in one issue, for having all examples in one place and having only one reference. All this cases are caused by the limitation of the parser used in CPython, and using different grammar rules. This If you want to change the language specification for decorator expression and class definition, it should be discussed before merging PR 4382, and I would make corresponding changes in it. In any case it is harder to fix issue32023 without fixing the original issue.
|
msg306212 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-11-14 13:59 |
Stefan, `[1,2,3]` is an expression, but `x for x in [1,2,3]` is not. If you want to change the Python language specification, please open a topic on Python-Dev and provide the rationale.
|
msg306213 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-11-14 14:02 |
Yes Sir!
|
msg306233 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2017-11-14 18:00 |
It's a (small) mistake that we didn't make the syntax for argument lists in decorators the same as argument lists everywhere else, and that should be fixed to allow exactly what's allowed in regular calls. (That syntax is weird because we don't want e.g. `@foo().bar` but we do want e.g. `@foo.bar()`.)
I am honestly not sure that we should change anything here, since the meaning is not actually ambiguous: the syntax for generator expressions doesn't allow e.g. `x for x in 1, 2, 3` -- you have to write `x for x in (1, 2, 3)`. (A regular for-loop *does* allow this, but there the context makes it unambiguous -- that's why genexprs are different.)
But I'm fine with changing it, as long as we do it consistently.
|
msg306255 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-11-15 06:49 |
New changeset 9165f77d5f93a2c12aa0e90853e3ae7212800d3c by Serhiy Storchaka in branch 'master':
bpo-32012: Disallow trailing comma after genexpr without parenthesis. (#4382)
https://github.com/python/cpython/commit/9165f77d5f93a2c12aa0e90853e3ae7212800d3c
|
msg306256 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2017-11-15 08:16 |
With issue 32023 (base class lists) and 32034 (fixing the documentation for decorator factory function calls) covering the other refinements, this particular issue is done now.
The most recent PR is the one for issue 32023.
|
msg322730 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2018-07-31 06:34 |
New changeset 4b8a7f51da224d1a0ad8159935f78ba4e6e16037 by Serhiy Storchaka in branch 'master':
Revert "closes bpo-27494: Fix 2to3 handling of trailing comma after a generator expression (GH-3771)" (#8241)
https://github.com/python/cpython/commit/4b8a7f51da224d1a0ad8159935f78ba4e6e16037
|
msg322733 - (view) |
Author: miss-islington (miss-islington) |
Date: 2018-07-31 06:52 |
New changeset 9ecbe3321f7bb3726017a053e583ca507d4453fc by Miss Islington (bot) in branch '3.7':
Revert "closes bpo-27494: Fix 2to3 handling of trailing comma after a generator expression (GH-3771)" (GH-8241)
https://github.com/python/cpython/commit/9ecbe3321f7bb3726017a053e583ca507d4453fc
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:54 | admin | set | github: 76193 |
2018-07-31 06:52:51 | miss-islington | set | nosy:
+ miss-islington messages:
+ msg322733
|
2018-07-31 06:34:46 | miss-islington | set | pull_requests:
+ pull_request8089 |
2018-07-31 06:34:33 | serhiy.storchaka | set | messages:
+ msg322730 |
2017-11-15 08:16:39 | ncoghlan | set | status: open -> closed resolution: fixed messages:
+ msg306256
stage: patch review -> resolved |
2017-11-15 07:09:15 | serhiy.storchaka | set | pull_requests:
+ pull_request4348 |
2017-11-15 06:49:48 | serhiy.storchaka | set | messages:
+ msg306255 |
2017-11-14 18:44:28 | serhiy.storchaka | link | issue32023 dependencies |
2017-11-14 18:25:26 | serhiy.storchaka | link | issue32024 dependencies |
2017-11-14 18:00:03 | gvanrossum | set | messages:
+ msg306233 |
2017-11-14 14:03:11 | yselivanov | set | nosy:
- yselivanov
|
2017-11-14 14:02:43 | skrah | set | nosy:
- skrah
|
2017-11-14 14:02:28 | skrah | set | nosy:
+ skrah messages:
+ msg306213
|
2017-11-14 13:59:52 | serhiy.storchaka | set | messages:
+ msg306212 |
2017-11-14 13:57:40 | skrah | set | nosy:
- skrah
|
2017-11-14 13:55:40 | serhiy.storchaka | set | messages:
+ msg306211 |
2017-11-14 13:43:30 | skrah | set | messages:
+ msg306209 |
2017-11-14 13:31:52 | ncoghlan | set | messages:
+ msg306208 |
2017-11-14 13:17:58 | skrah | set | messages:
+ msg306207 |
2017-11-14 13:12:52 | ncoghlan | set | messages:
+ msg306206 |
2017-11-14 13:03:05 | skrah | set | nosy:
+ skrah messages:
+ msg306204
|
2017-11-14 11:40:15 | cryvate | set | messages:
+ msg306200 |
2017-11-14 10:43:13 | serhiy.storchaka | set | messages:
+ msg306199 |
2017-11-14 10:28:28 | ncoghlan | set | messages:
+ msg306198 |
2017-11-14 06:07:43 | serhiy.storchaka | set | nosy:
+ gvanrossum messages:
+ msg306185
|
2017-11-14 00:26:30 | ncoghlan | set | messages:
+ msg306179 |
2017-11-13 16:10:41 | serhiy.storchaka | set | messages:
+ msg306161 |
2017-11-13 15:19:50 | cryvate | set | messages:
+ msg306160 |
2017-11-13 15:19:35 | cryvate | set | nosy:
+ cryvate messages:
+ msg306159
|
2017-11-13 08:24:12 | serhiy.storchaka | set | messages:
+ msg306149 |
2017-11-13 07:35:39 | serhiy.storchaka | set | messages:
+ msg306145 |
2017-11-12 22:43:28 | serhiy.storchaka | set | keywords:
+ patch stage: patch review pull_requests:
+ pull_request4330 |
2017-11-12 22:29:05 | serhiy.storchaka | create | |