Add checks for some 3.5+ only syntax (async/await and the @ operator) #41
Conversation
Python 3.5 introduced three new forms of syntax: - Coroutines with async/await (PEP 492) - The '@' operator (PEP 465) - Additional unpacking generalizations (PEP 448) This commit adds a version check for the first two, but not the last item. This commit was originally motivated by python/mypy#2401. I decided to also add a check for '@' because it looked like an easy fix, but didn't add a check for the unpacking generalizations because that seemed harder.
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
There was a problem hiding this comment.
Thanks for the PR, Michael! This looks great! I just had a couple of tiny nits (see comments).
I agree that properly catching the PEP 448 additional unpacking generalizations will be a bit tricky, so I'm fine with it not being part of this PR.
I think it's fine to have checks for the async things that can only appear inside async functions. My understanding is that the parsing of those will change in 3.7 (async is becoming a keyword, I think), and it will be good to have reminders to give appropriate error messages when they do. (Now that I think about it, this might be a significant problem backwards-compatibility-wise, but that's unrelated to this PR.)
ast3/Python/ast.c
Outdated
| if (c->c_feature_version < 5) { | ||
| ast_error(c, n, | ||
| "Await expressions are only supported in Python 3.5 and greater"); | ||
| return 0; |
There was a problem hiding this comment.
Nit: this should be return NULL;.
There was a problem hiding this comment.
Whoops, I think I must have forgotten to change this after copy-pasting. In any case, fixed.
typed_ast/ast3.py
Outdated
|
|
||
| If feature_version=4, the parser will currently forbid the use of the | ||
| async/await keywords and the '@' operator, but will not forbid the use of | ||
| PEP 448 additional unpacking generalizations, which was added in Python 3.5. |
There was a problem hiding this comment.
Nit: I'd consider some minor wording tweaks here. I think "When" is more commonly used than "If", in this context. I think you can get rid of "currently" -- docs by default describe current behavior (and there aren't any concrete plans to change this). Also, "additional unpacking generalizations" is plural, so I think the final "was" should be a "were".
There was a problem hiding this comment.
Mkay, sounds good -- hopefully the tweaked version flows better.
|
(That said, I think that catching the PEP 448 generalizations might not be too terrible -- it looks to me like it could be done in |
Changes made: - Return 'NULL' instead of 0 for await check - Reword doc comment in typed_ast.ast3.parse
|
Thanks! |
This incorporates python/typed_ast#41: 'Add checks for some 3.5+ only syntax (async/await and the @ operator)'
This incorporates python/typed_ast#41: 'Add checks for some 3.5+ only syntax (async/await and the @ operator)'
Merge development branch
Python 3.5 introduced three new forms of syntax:
This commit adds a version check for the first two, but not the last item.
This commit was originally motivated by python/mypy#2401. I decided to also add a check for the '@' operator because it looked like an easy fix, but didn't add a check for the unpacking generalizations because that seemed harder.
The checks for
async for,async with, andawaitare probably unnecessary because they can only be used insideasync defanyways, which will always be checked first, but I figured I might as well add them just in case. I can remove them if you want, though, since it probably does incur a slight performance hit.