Skip to content

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


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.


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.

6 participants