bpo-35766: Merge typed_ast back into CPython#11645

gvanrossum

[UPDATE: I've rewritten this comment to match the latest state of the PR.]

See the bpo issue for motivation. This is the minimal code I hope to merge into CPython. The PR here is complete except for docs. To test this, use ast.parse(source, type_comments=True).

(Speaking of backwards compatibility, my bpo issue suggests that I'd also like some features supporting older grammar versions. The only thing that I would really appreciate is being able to parse code that uses async or await as a non-keyword. But I could structure this as a follow-up PR, much smaller in size.)

https://bugs.python.org/issue35766

…anything

Demo: `compile(sample, "<sample>", "exec", PyCF_ONLY_AST|0x1000)` This will crash if type comments are present. It doesn't *work* yet -- type comments cause crashes in ast.c and type ignore comments aren't passed on to Module yet.

This works, it just needs tests

gvanrossum

I updated the PR to be merged with master now #11605 has been merged. Everything still works, and there are no more "ghost commits" from #11605.

gvanrossum

I'll try to fix all failing tests before trying again. In my local branch I have test_ast and test_asdl_parser working; the rest is more mysterious (it seems bad indents are parsed differently somehow).

test_ast.py
test_asdl_parser.py
test_syntax.py
test_exceptions.py
test_parser.py

gvanrossum

I may have to put this off for a while (certainly I won't get it into 3.8a1). The main problem at this point is the two failing tests, test_syntax and test_exceptions. Both are failing because the grammar changes have made it so that

if 1:
foo()

reports SyntaxError instead of IndentationError at the start of line 2. The reason for that is this definition in Grammar:

# the TYPE_COMMENT in suites is only parsed for funcdefs, but can't go elsewhere due to ambiguity
suite: simple_stmt | NEWLINE [TYPE_COMMENT NEWLINE] INDENT stmt+ DEDENT

Previously this was

suite: simple_stmt | NEWLINE INDENT stmt+ DEDENT

and there is code in Python/pythonrun.c::err_input that depends on INDENT being the only possibility after the NEWLINE.

I haven't figured out how to fix this yet.

This fixes if 1: foo() but does nothing about def f(): foo() The latter still reports 'SyntaxError: invalid syntax'. So this is not a real solution, just a stop-gap measure until I find a better solution.

Also: - remove redundant 'c' argument from new_type_comment() - double-check that TYPE_COMMENT in a suite is followed by NL

gvanrossum

Sorry for the flood of comments. I believe I've done or responded to every code review comment. Hopefully tests will pass now.

ilevkivskyi

Thanks for updates! This looks ready, I left a bunch of really minor comments.

gvanrossum

@ambv Should I just merge this, or do you want to review it?

ambv

Unless you're in a hurry now, I'll review and stamp it tomorrow morning my time. Mostly to familiarize myself with the changes.

gvanrossum

That's fine!

gvanrossum

Unsure why the Appveyor build isn't running?

zooba

      Unsure why the Appveyor build isn't running?

It looks it decided to skip the build, maybe because the last commit is a merge?

ambv

Let's merge this. It's a bit of a bigger change than promised but such is life 😉

I left one comment, @gvanrossum, that you might want to respond to.

pablogsal

@ambv @gvanrossum I have detected some refleaks in the buildbots after this PR and I started https://bugs.python.org/issue35881 to track this problem. I already started debugging, but I would like so input in case you can see something obvious right away.

pythonGH-11645 added the fourth option for `mode` argument; updating the comment accordingly.

tyomitch

This PR added a fourth option for mode argument of builtin_compile_impl(), but didn't update its documentation; please see GH-11762

This is mostly cribbed from python/cpython#11645, though it also adds a new error check to new_type_comment itself.

gvanrossum requested review from ambv and ilevkivskyi January 22, 2019 02:26

bedevere-bot added the awaiting merge label Jan 22, 2019

gvanrossum added 13 commits January 22, 2019 08:58

Add TYPE_IGNORE and TYPE_COMMENT without regenerating anything

77de614

Regenerated files (make regen-token)

8e0fd8a

Tentative parsetok.c changes to handle TYPE_IGNORE

cb93ba5

Unconditionally recognize type comments in tokenizer.c

232f2b6

Changes to Grammar without regenerating anything

c80197e

Regenerated files (make regen-grammar)

b440842

Add type_comment fields to various asdl classes without regenerating … …

9db5e34

…anything

Regenerated files (make regen-ast)

6cc5a55

Make ast.c compile by adding NULL for all type_comment arguments

e7f207d

Make recognition of type comments conditional on a flag (off by default)

8b426b9

Define flags to turn on parsing type comments …

72e4f4d

Demo: `compile(sample, "<sample>", "exec", PyCF_ONLY_AST|0x1000)` This will crash if type comments are present. It doesn't *work* yet -- type comments cause crashes in ast.c and type ignore comments aren't passed on to Module yet.

Support passing type ignores to Module object

3d19dbb

Support TYPE_COMMENT in ast.c for all nodes that need it …

d273d67

This works, it just needs tests

gvanrossum force-pushed the ast-type-comments branch from e1bf919 to d273d67 Compare January 22, 2019 17:21

gvanrossum added 2 commits January 22, 2019 09:32

Regenerate Lib/symbol.py

830d63a

Make growable_int_array functions static

71aecd5

blurb-it Bot and others added 3 commits January 22, 2019 11:21

News blurb (thank you blurb-it)

53ceee6

Fix test_ast.py

6ddca66

Fix test_asdl_parser.py

36c212b

gvanrossum force-pushed the ast-type-comments branch from a40849b to 36c212b Compare January 22, 2019 20:56

gvanrossum mentioned this pull request Jan 23, 2019

[DO NOT MERGE] Fix def line numbers for decorated nodes python/typed_ast#61

Closed

gvanrossum added 3 commits January 22, 2019 20:30

Hacky way to get tests to pass …

d2091ed

This fixes if 1: foo() but does nothing about def f(): foo() The latter still reports 'SyntaxError: invalid syntax'. So this is not a real solution, just a stop-gap measure until I find a better solution.

Regen Lib/symbol.py (again)

6d1f1de

Fix assert in ast_for_suite() -- n can be a func_body_suite too now

567f341

gvanrossum added 3 commits January 28, 2019 11:31

Add comment clarifying the func_body_suite switcheroo

98ee20a

Check for errors from _Py_asdl_seq_new()

4abedd3

Check for NULL from NEW_TYPE_COMMENT() …

551f2a6

Also: - remove redundant 'c' argument from new_type_comment() - double-check that TYPE_COMMENT in a suite is followed by NL

Fix markup bug

b68270c

ilevkivskyi approved these changes Jan 30, 2019

View reviewed changes

gvanrossum added 4 commits January 30, 2019 09:18

Brace block in parsetok.c

dcf86e8

Brace a block and fix indentation of another block

87812ad

Brace block, update grammar comment, fix indents of many ast_error calls

5633403

Merge branch 'master' into new-ast-type-comments

2062e27

Merge remote-tracking branch 'origin/master' into ast-type-comments

03310fc

Break up long comment to trigger Appveyor build

229874c

ambv approved these changes Jan 31, 2019

View reviewed changes

ambv merged commit dcfcd14 into python:master Jan 31, 2019

bedevere-bot removed the awaiting merge label Jan 31, 2019

gvanrossum mentioned this pull request Feb 1, 2019

bpo-35879: Fix type comment leaks #11728

Merged

gvanrossum deleted the ast-type-comments branch February 1, 2019 23:29

tyomitch added a commit to tyomitch/cpython that referenced this pull request Feb 5, 2019

Update comment for builtin_compile_impl …

31327c8

pythonGH-11645 added the fourth option for `mode` argument; updating the comment accordingly.

tyomitch mentioned this pull request Feb 5, 2019

Update comment for builtin_compile_impl #11762

Closed

msullivan added a commit to python/typed_ast that referenced this pull request Feb 6, 2019

Check NEW_TYPE_COMMENT calls for failure …

76511c0

This is mostly cribbed from python/cpython#11645, though it also adds a new error check to new_type_comment itself.

msullivan mentioned this pull request Feb 6, 2019

Check NEW_TYPE_COMMENT calls for failure python/typed_ast#93

Merged

gvanrossum pushed a commit to python/typed_ast that referenced this pull request Feb 6, 2019

Check NEW_TYPE_COMMENT calls for failure (#93) …

f2acb2b

This is mostly cribbed from python/cpython#11645, though it also adds a new error check to new_type_comment itself.

Conversation

gvanrossum commented Jan 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gvanrossum commented Jan 22, 2019

Uh oh!

gvanrossum commented Jan 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gvanrossum commented Jan 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gvanrossum commented Jan 28, 2019

Uh oh!

ilevkivskyi left a comment

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Jan 30, 2019

Uh oh!

ambv commented Jan 30, 2019

Uh oh!

gvanrossum commented Jan 30, 2019

Uh oh!

gvanrossum commented Jan 30, 2019

Uh oh!

zooba commented Jan 30, 2019

Uh oh!

ambv left a comment

Choose a reason for hiding this comment

Uh oh!

pablogsal commented Feb 1, 2019

Uh oh!

tyomitch commented Feb 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

gvanrossum commented Jan 22, 2019 •

edited

Loading

gvanrossum commented Jan 22, 2019 •

edited

Loading

gvanrossum commented Jan 23, 2019 •

edited

Loading