Skip to content

Conversation

isidentical
Copy link
Member

@isidentical isidentical commented Oct 9, 2020

@pablogsal pablogsal added the needs backport to 3.9 only security fixes label Oct 9, 2020
@pablogsal pablogsal self-assigned this Oct 9, 2020
@lysnikolaou
Copy link
Member

FWIW there are some differences here in the SyntaxErrors displayed.

@pablogsal
Copy link
Member

FWIW there are some differences here in the SyntaxErrors displayed.

What syntax error are you referring to?

@lysnikolaou
Copy link
Member

lysnikolaou commented Oct 9, 2020

➜  cpython git:([bpo-41979](https://bugs.python.org/issue41979)) python3.8
Python 3.8.6 (default, Oct  3 2020, 16:26:47) 
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> with a as *b:
  File "<stdin>", line 1
    with a as *b:
              ^
SyntaxError: invalid syntax
>>> with a as (*b):
...     pass
... 
  File "<stdin>", line 1
SyntaxError: starred assignment target must be in a list or tuple
>>> 
➜  cpython git:([bpo-41979](https://bugs.python.org/issue41979)) ./python
Python 3.10.0a1+ (heads/[bpo-41979](https://bugs.python.org/issue41979):bc2fd55f74, Oct  9 2020, 11:30:38) 
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> with a as *b:
...     pass
... 
  File "<stdin>", line 1
SyntaxError: starred assignment target must be in a list or tuple
>>> with a as (*b):
  File "<stdin>", line 1
    with a as (*b):
               ^
SyntaxError: can't use starred expression here

I'm not saying we should change something here. I'm just mentioning the differences, so that we know about them. Thoughts?

@pablogsal
Copy link
Member

I'm not saying we should change something here. I'm just mentioning the differences, so that we know about them. Thoughts?

Oh, thanks for the example! I think the second one may be a bit confusing to users (you can use star expressions there but as the first error mentions, must be in a list or a tuple). I will try to create a PR today for this, but I think we can treat it separately from this one, what do you think?

@isidentical
Copy link
Member Author

FWIW, this behavior is the exact same thing with other assignments;

(3.10)
>>> *a = [1,2]
  File "<stdin>", line 1
SyntaxError: starred assignment target must be in a list or tuple
>>> (*a) = [1,2]
  File "<stdin>", line 1
    (*a) = [1,2]
     ^
SyntaxError: can't use starred expression here

@pablogsal
Copy link
Member

FWIW, this behavior is the exact same thing with other assignments;

Yeah, that's why I proposed to treat this independently of this PR. I will create a patch for this syntax error today if I find the time

@lysnikolaou
Copy link
Member

FWIW, this behavior is the exact same thing with other assignments;

Yeah, that's why I proposed to treat this independently of this PR. I will create a patch for this syntax error today if I find the time

Agreed.

What bugs me most is that in the with a as *b example, I'm not immediately getting an error like in the old parser. I must complete the whole with statement before I get to the SyntaxError.

@pablogsal
Copy link
Member

pablogsal commented Oct 9, 2020

I must complete the whole with statement before I get to the SyntaxError.

But that only makes sense in the REPL no? We can look together later how to make this fail before if you wish. Likely is a matter of restricting some of that rule or checking the order. But I would be cautious of this as it probably will raise the complexity of the grammar a bit.

@pablogsal pablogsal merged commit 48f305f into python:master Oct 9, 2020
@miss-islington
Copy link
Contributor

Thanks @isidentical for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @isidentical and @pablogsal, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 48f305fd122080a9181cbda33bdb42ea36a0136f 3.9

@pablogsal
Copy link
Member

@isidentical Can you do the backport, please?

@lysnikolaou
Copy link
Member

Agreed, let's merge this and come back to it later.

isidentical added a commit to isidentical/cpython that referenced this pull request Oct 9, 2020
…-22611)

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>.
(cherry picked from commit 48f305f)

Co-authored-by: Batuhan Taskaya <batuhanosmantaskaya@gmail.com>
@bedevere-bot
Copy link

GH-22612 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Oct 9, 2020
miss-islington pushed a commit that referenced this pull request Oct 9, 2020
GH-22612)

Co-authored-by: Batuhan Taskaya <batuhanosmantaskaya@gmail.com>

Automerge-Triggered-By: @pablogsal
xzy3 pushed a commit to xzy3/cpython that referenced this pull request Oct 18, 2020
Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants