-
-
Notifications
You must be signed in to change notification settings - Fork 30.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-35975: Support parsing earlier minor versions of Python 3 #12086
Conversation
I could use some help finding the cause of the one remaining test failure -- test_parser.py computes the size of tree nodes differently somehow. [UPDATE: Fixed it, I think.] |
The main thing left to do is adding tests. There are some tests in typed_ast that I can migrate over. But first I think we should complete the debate over on bpo about whether this is desirable at all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This looks very clean, I just have few minor suggestions.
(I also there is a merge conflict now.)
Lib/ast.py
Outdated
return compile(source, filename, mode, flags) | ||
return compile(source, filename, mode, flags, | ||
dont_inherit=False, | ||
optimize=-1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we now need to pass these two extra arguments? Maybe add a comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, those are the default values that I somehow copied when cleaning this up. I'll remove them -- we just need feature_version
.
Python/ast.c
Outdated
/* Async comprehensions only allowed in Python 3.6 and greater */ | ||
if (is_async && c->c_feature_version < 6) { | ||
ast_error(c, n, | ||
"Async comprehensions are only supported in Python 3.6 and greater"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation here and in several other places is a bit unusual, I would rather justify it after (
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, fixed. My guess is that at some point in the past that function had had a name that was 2 characters shorter. :-)
@@ -1746,6 +1758,12 @@ ast_for_funcdef_impl(struct compiling *c, const node *n0, | |||
node *tc; | |||
string type_comment = NULL; | |||
|
|||
if (is_async && c->c_feature_version < 5) { | |||
ast_error(c, n, | |||
"Async functions are only supported in Python 3.5 and greater"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here indentation looks good.
Visible behavior: - Issue error for `X @ Y` (matrix multiply) if c_feature_version < 5 - Add optional feature_version kw arg to ast.parse() (default -1 which implies PY_MINOR_VERSION) - Add feature_version: int = -1 to compile() (via Argument Clinic); this sets cf_feature_version to the given value if >= 0, else defaults to PY_MINOR_VERSION Implementation: - Add PyAST_obj2mod_ex(): like PyAST_obj2mod() but with feature_version arg; the latter calls the former with PY_MINOR_VERSION - Add cf_feature_version to PyCompilerFlags structure; initialized to PY_MINOR_VERSION everywhere - Add c_feature_version to struct compiling; initialize from cf_feature_version - Add 'c' argument to get_operator() - In builtin eval() and exec(), default to PY_MINOR_VERSION TODO: - Put version-dependent ASYNC/AWAIT keyword scanning back - Reject async functions, await expressions, and async for/with in minor versions < 5 - Reject async comprehensions in minor versions < 6 - Reject underscores in numeric literals in minor versions < 6 - Reject variable annotations in minor versions < 6 - Reject `X @= Y` in minor versions < 5
This is everything currently in typeshed except await expressions (but it does reject async functions etc.): - Reject async functions and async for/with in minor versions < 5 - Reject async comprehensions in minor versions < 6 - Reject underscores in numeric literals in minor versions < 6 - Reject variable annotations in minor versions < 6 - Reject `X @= Y` in minor versions < 5
This adds: - Add ASYNC/AWAIT tokens back to Grammar and regenerate - Recognize async/await keywords conditionally if feature_version < 7 - Reject await expressions if feature_version < 5 - Docs for ASYNC/AWAIT tokens and for ast.parse(..., feature_version=N)
The PyST_Object header in parsermodule.c became one int larger because it contains a PyCompilerFlags struct, which grew extra space for the st_feature_version field. Took me long enough!
… not regular keywords
87c80c1
to
ae36b7b
Compare
OK, this is ready for final review and merge. (Sorry that the rebase lost some of the review history, I'm used to different tooling.) |
(Well, I promised tests. Upcoming. Docs are already done.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates! All looks good, I have few optional suggestions.
@@ -145,13 +145,18 @@ and classes for traversing abstract syntax trees: | |||
modified to correspond to :pep:`484` "signature type comments", | |||
e.g. ``(str, int) -> List[str]``. | |||
|
|||
Also, setting ``feature_version`` to the minor version of an | |||
earlier Python 3 version will attempt to parse using that version's | |||
grammar. For example, setting ``feature_version=4`` will allow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add that 4
is the lowest supported value?
Lib/test/test_type_comments.py
Outdated
return ast.parse(source, type_comments=True, | ||
feature_version=feature_version) | ||
|
||
def parses(self, source, minver=lowest, maxver=highest, expected_regex=""): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parses
is not very descriptive and is easy to confuse with parse
, maybe parse_all
, or parse_all_versions
?
fstring = """\ | ||
a = 42 | ||
f"{a}" | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add underscores in numeric literals for completeness?
This adds a
feature_version
flag toast.parse()
(documented) andcompile()
(hidden) that allow tweaking the parser to support older versions of the grammar. In particular iffeature_version
is 5 or 6, the hacks for theasync
andawait
keyword from PEP 492 are reinstated. (For 7 or higher, these are unconditionally treated as keywords, but they are still special tokens rather thanNAME
tokens that the parser driver recognizes.)https://bugs.python.org/issue35975