classification
Title: async/await parser issues
Type: Stage: resolved
Components: Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: yselivanov Nosy List: benjamin.peterson, gvanrossum, martin.panter, ncoghlan, python-dev, skrah, vstinner, yselivanov
Priority: release blocker Keywords: patch

Created on 2015-07-12 18:49 by skrah, last changed 2015-07-26 09:37 by yselivanov. This issue is now closed.

Files
File name Uploaded Description Edit
issue24619.1.patch yselivanov, 2015-07-21 13:33 review
issue24619.2.patch yselivanov, 2015-07-21 14:30 review
issue24619.3.patch yselivanov, 2015-07-22 22:02 review
issue24619.4.patch yselivanov, 2015-07-23 06:20 review
Messages (13)
msg246660 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2015-07-12 18:49
If I understand the reference manual correctly, these should probably
be rejected by the compiler:

>>> async def f():
...     def g(): pass
...     async = 10
... 
>>> async def f():
...     def async():
...         pass
... 
>>> async def f(): async = 10
... 
>>> async def f():
...     def await(): pass
... 
>>> 


And this should perhaps be accepted:

>>> async def f():
...     return lambda await: await
  File "<stdin>", line 2
    return lambda await: await
                      ^
SyntaxError: invalid syntax


This, too:

>>> async def f():
...     async def g(): pass
...     await z
  File "<stdin>", line 3
    await z
          ^
SyntaxError: invalid syntax
msg247034 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-07-21 13:33
Sorry for not responding earlier, I'm on vacation and don't check email too often.

While investigating what can be done in tokenizer.c to fix some of the bugs that Stefan pointed out, I discovered a simpler approach: instead of checking what kind of function we have on top of the stack, I now have a counter of nested async functions.  This aligns nicely with what Nick said on python-dev once: we essentially treat 'async def' as a future import, everything inside it parses differently.

In other words, before this patch:

   async def foo():
       def bar():
           async = 1

was legal, now it's a syntax error, as 'async' will be parsed as 'ASYNC' token.

This simplifies the implementation of tokenizer.c hacks, fixes all of the bugs that Stefan posted here, *and* enables one-line 'async' functions:

   async def foo(): await bar() # legal with the patch

It will also help with migrating the code to Python 3.7 when async/await will become proper keywords.

And, with this patch we can effectively remove the shortcomings section from the PEP 492 (https://www.python.org/dev/peps/pep-0492/#transition-period-shortcomings).

Please review the attached patch, it would be great if we can commit it before 3.5beta4.
msg247035 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2015-07-21 13:54
Haven't reviewed the patch, but this approach sounds great (in fact I had
assumed you were doing this already, and I was a bit surprised by some of
the problems you encountered :-).
msg247037 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-07-21 14:00
Good news :) I guess this means we can also remove the sentence I added at <https://docs.python.org/3.5/reference/compound_stmts.html#coroutine-function-definition>.
msg247038 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-07-21 14:30
An updated patch is attached.  I had to implement a little bit more sophisticated tracking of one-line functions to fix parsing of things like

    def foo():
        async def f(): pass
        async def f(): pass
        async = 1

I hope that test_coroutine.py now covers all possible legal and illegal async/await syntax.

> Haven't reviewed the patch, but this approach sounds great (in fact I had
> assumed you were doing this already, and I was a bit surprised by some of
> the problems you encountered :-).

Yes, I'm a bit surprised myself ;)  I guess one of the reasons why I tried to do more in tokenizer is that at the time of me hacking the tokenizer, compile.c wasn't quite ready (specifically, catching async for/async with/await outside of async functions).

> Good news :) I guess this means we can also remove the sentence I added at [..]

Right!
msg247078 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-07-22 01:58
Patch & test cases look good to me.

I'm so used to thinking of the tokenisation phase as a linear token stream
that it never occurred to me to just count the function nesting directly to
determine if the "async def" tokenisation rules are in effect - it's a very
nice simplifying improvement.
msg247106 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-07-22 10:38
New changeset 9da080ecadb2 by Yury Selivanov in branch '3.5':
Issue #24619: New approach for tokenizing async/await.
https://hg.python.org/cpython/rev/9da080ecadb2

New changeset 987b72921a0c by Yury Selivanov in branch 'default':
Merge 3.5 (Issue #24619)
https://hg.python.org/cpython/rev/987b72921a0c
msg247107 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-07-22 10:40
Thanks Nick!  I've committed the patch with a few more unittests and a couple of additional comments in tokenizer.(c|h).
msg247110 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-07-22 11:49
New changeset e4e01488afff by Yury Selivanov in branch '3.5':
Issue #24619: More tests; fix nits in compiler.c
https://hg.python.org/cpython/rev/e4e01488afff

New changeset 6f4e0c462daf by Yury Selivanov in branch 'default':
Merge 3.5 (Issue #24619)
https://hg.python.org/cpython/rev/6f4e0c462daf
msg247123 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2015-07-22 14:47
This is a very nice solution!

I'm just curious if the 'ctx' is still needed:  It looks like
the outermost "async def" dominates all further nested scopes
w.r.t the tokenizer mode, no matter whether they're "def" or
"async def" scopes.

IOW, a single indent_level variable that follows all INDENTs/DEDENTs
once the outermost "async def" scope is entered might be sufficient.


[This is in no way urgent, please do not feel obliged to respond
during your holiday.]
msg247152 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-07-22 22:02
> I'm just curious if the 'ctx' is still needed:  It looks like
> the outermost "async def" dominates all further nested scopes
> w.r.t the tokenizer mode, no matter whether they're "def" or
> "async def" scopes.

This is a wonderful idea, that's the way it should be done in
both tokenizer.c and tokenize.py.

Please see the new patch.  I love the simplicity of it, no 
more stacks or hard to follow code.
msg247175 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-07-23 06:20
Updated patch attached (rebased; minor optimization).  I'll commit this patch in a few hours to make sure it lands in 3.5beta4.
msg247188 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-07-23 12:02
New changeset d03f86e41066 by Yury Selivanov in branch '3.5':
Issue #24619: Simplify async/await tokenization.
https://hg.python.org/cpython/rev/d03f86e41066

New changeset 3f8048926690 by Yury Selivanov in branch 'default':
Merge 3.5 (Issue #24619)
https://hg.python.org/cpython/rev/3f8048926690
History
Date User Action Args
2015-07-26 09:37:11yselivanovsetstage: commit review -> resolved
2015-07-25 21:07:11larrysetstatus: open -> closed
2015-07-23 12:04:39yselivanovsetresolution: fixed
stage: patch review -> commit review
2015-07-23 12:02:47python-devsetmessages: + msg247188
2015-07-23 06:21:00yselivanovsetfiles: + issue24619.4.patch

messages: + msg247175
2015-07-23 06:15:39yselivanovsetnosy: + benjamin.peterson
2015-07-22 22:02:09yselivanovsetstatus: closed -> open
files: + issue24619.3.patch
messages: + msg247152

resolution: fixed -> (no value)
stage: resolved -> patch review
2015-07-22 14:47:28skrahsetmessages: + msg247123
2015-07-22 11:49:24python-devsetmessages: + msg247110
2015-07-22 10:43:58yselivanovsetstage: patch review -> resolved
2015-07-22 10:43:51yselivanovsetstatus: open -> closed
2015-07-22 10:40:43yselivanovsetresolution: fixed
messages: + msg247107
2015-07-22 10:38:47python-devsetnosy: + python-dev
messages: + msg247106
2015-07-22 01:58:05ncoghlansetmessages: + msg247078
2015-07-21 14:30:25yselivanovsetfiles: + issue24619.2.patch

messages: + msg247038
2015-07-21 14:00:12martin.pantersetnosy: + martin.panter
messages: + msg247037
2015-07-21 13:54:51gvanrossumsetmessages: + msg247035
2015-07-21 13:33:10yselivanovsetfiles: + issue24619.1.patch
priority: normal -> release blocker

assignee: yselivanov
versions: + Python 3.5, Python 3.6
keywords: + patch
nosy: + ncoghlan, vstinner

messages: + msg247034
stage: patch review
2015-07-12 18:49:13skrahcreate