Skip to content
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

Merged
merged 14 commits into from
Mar 7, 2019

Conversation

gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Feb 28, 2019

This adds a feature_version flag to ast.parse() (documented) and compile() (hidden) that allow tweaking the parser to support older versions of the grammar. In particular if feature_version is 5 or 6, the hacks for the async and await keyword from PEP 492 are reinstated. (For 7 or higher, these are unconditionally treated as keywords, but they are still special tokens rather than NAME tokens that the parser driver recognizes.)

https://bugs.python.org/issue35975

@gvanrossum
Copy link
Member Author

gvanrossum commented Feb 28, 2019

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.]

@gvanrossum
Copy link
Member Author

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.

Copy link
Member

@ilevkivskyi ilevkivskyi left a 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,
Copy link
Member

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?

Copy link
Member Author

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");
Copy link
Member

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 (.

Copy link
Member Author

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");
Copy link
Member

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!
@gvanrossum gvanrossum changed the title bpo-35975: [WIP] Support parsing earlier minor versions of Python 3 bpo-35975: Support parsing earlier minor versions of Python 3 Mar 5, 2019
@gvanrossum
Copy link
Member Author

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.)

@gvanrossum
Copy link
Member Author

(Well, I promised tests. Upcoming. Docs are already done.)

Copy link
Member

@ilevkivskyi ilevkivskyi left a 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
Copy link
Member

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?

return ast.parse(source, type_comments=True,
feature_version=feature_version)

def parses(self, source, minver=lowest, maxver=highest, expected_regex=""):
Copy link
Member

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}"
"""
Copy link
Member

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?

@zware zware deleted the feature-version branch April 10, 2019 02:26
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.

5 participants