bpo-43224: Implement PEP 646 grammar changes

[mrahtz]

These are the parts of the old PR (#30398) relevant to only the grammar changes. To summarise, this adds support for:

1. Starring within squared brackets, e.g. Tensor[*Ts], Tensor[T, *Ts] (where Ts is a TypeVarTuple and T is a TypeVar).
   • But not within an actual slice - e.g. Tensor[0:*Ts] should not be valid.
2. Starring the annotation given to *args, e.g. *args: *Ts.

For 1, our intention is to call __iter__ on Ts and add the items from the resulting iterator to the tuple of arguments sent to Tensor.__cls_getitem__. For example, with Tensor[*Ts], if Ts.__iter__ returns an iterator yielding Ts.unpacked, Tensor.__cls_getitem__ would receive (Ts.unpacked,). With Tensor[T, *Ts], Tensor.__cls_getitem__ would receive (T, Ts.unpacked).

For 2, our intention is to call __iter__ on Ts, get a single item from the iterator, verify that the iterator is exhausted, and set that item as the annotation for *args.

I'm guessing the person I should ask for review on this is @pablogsal?

https://bugs.python.org/issue43224

[pablogsal]

@mrahtz Could you please add some expected syntax errors to Lib/test/test_syntax.py? Things like def f(**kwargs: *a) and the like.

[mrahtz]

@pablogsal

Could you please add some expected syntax errors to Lib/test/test_syntax.py? Things like def f(**kwargs: *a) and the like.

Sure, will do. Two questions:

• Is there a convention for which tests should be implemented as doctests vs test cases in test_syntax.py?
• Should I move the uses of PEP 646 syntax that don't cause failure to test_grammar.py, or can they be kept in a test module of their own?

[pablogsal]

• Is there a convention for which tests should be implemented as doctests vs test cases in test_syntax.py?

If you want to check that something is a Syntax error and just want to assert the message and can do they with a simple input, in th doctest.

If you required special compilation modes, some gigantic or dynamic string or check the location of the errors, then a specific test case is needed.

• Should I move the uses of PEP 646 syntax that don't cause failure to test_grammar.py, or can they be kept in a test module of their own?

In general we have one test file for testing the runtime implications of the grammar, and things go to test_grammar to check grammatical relates aspects or the shape of the AST. Normally you can do both if you want/need

[mrahtz]

Sorry for the slow reply - I'm only just now finding time again to work on this.

Looking through test_grammar.py and test_syntax.py, it seemed like the most appropriate thing was just to move the failure cases from test_pep646_syntax.py to test_syntax.py. This is what I've done in the latest commit, but let me know if you'd like more doing.

[mrahtz]

@pablogsal Friendly poke :)

[mrahtz]

@pablogsal Another friendly poke :) I'm getting a bit nervous we might miss the merge window for 3.11 on this - if you're busy at the moment, would you be able to recommend anyone else who'd be a good fit for a review of this? (Or @JelleZijlstra do you know of anyone?)

[JelleZijlstra]

Maybe @isidentical if you'd like to take a look?

Seems like Pablo already looked at the diff to some extent, so if he doesn't have time for a full re-review, I can also do a review and help get this in. I'd of course prefer if someone familiar with the parser can do a review, though.

[JelleZijlstra]

I noticed that this is allowed by the grammar: s[1:1, *x]. It's not meaningful for typing, but I don't think we need to go out of our way to disallow it. Could you add some tests that combine slicing with unpacking?

[pablogsal]

Maybe @isidentical if you'd like to take a look?

Seems like Pablo already looked at the diff to some extent, so if he doesn't have time for a full re-review, I can also do a review and help get this in. I'd of course prefer if someone familiar with the parser can do a review, though.

Apologies, unfortunately I was sick with COVID these past weeks and I couldn't dedicate much time to open source :(

I will do another pass today or tomorrow.

Thanks for your understanding @mrahtz

[isidentical]

Seems like ast.unparse is broken on some scenerious:

importasttree_1=ast.parse("A[1:2, *l]")
tree_2=ast.parse(ast.unparse(ast.parse(tree_1)))
assertast.dump(tree_1) ==ast.dump(tree_2)

I assume it this is related to parenthesizing logic in here, which needs to be adapted to PEP646:

cpython/Lib/ast.py

Lines 1479 to 1487 in ef1327e

	defis_simple_tuple(slice_value):
	# when unparsing a non-empty tuple, the parentheses can be safely
	# omitted if there aren't any elements that explicitly requires
	# parentheses (such as starred expressions).
	return (
	isinstance(slice_value, Tuple)
	andslice_value.elts
	andnotany(isinstance(elt, Starred) foreltinslice_value.elts)
	)

[JelleZijlstra]

I just fixed a merge conflict and a whitespace issue found by make patchcheck. I think we can go ahead and merge, but I noticed a few things we still need to address:

(1) The ast.py changes aren't tested. They should be tested with a call to ast.unparse.

(2) There is also a C version of unparse in Python/ast_unparse.c, which is used for from __future__ import annotations. It likely needs changes too.

(3) If you use from __future__ import annotations and call get_type_hints() on a function that takes *args: *Ts, it fails because it tries to parse *Ts as an expression. I guess to solve this we'll have to detect an initial * and remove it before trying to parse the code.

Here's an example that demonstrates these issues:

Details

from __future__ importannotationsimporttypingimportastTs=typing.TypeVarTuple("Ts")
deff(x: tuple[*Ts], *args: *Ts):
passprint(f.__annotations__)
print(ast.unparse(ast.parse("tuple[*Ts]")))
print(typing.get_type_hints(f))

% ./python.exe Doc/tools/annotvt.py
{'x': 'tuple[(*Ts,)]', 'args': '*Ts'}
tuple[*Ts,]
Traceback (most recent call last):
File "/Users/jelle/py/cpython/Lib/typing.py", line 738, in __init__
code = compile(arg, '<string>', 'eval')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<string>", line 1
*Ts
^
SyntaxError: invalid syntax
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/Users/jelle/py/cpython/Doc/tools/annotvt.py", line 12, in <module>
print(typing.get_type_hints(f))
^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/jelle/py/cpython/Lib/typing.py", line 2199, in get_type_hints
value = ForwardRef(
^^^^^^^^^^^
File "/Users/jelle/py/cpython/Lib/typing.py", line 740, in __init__
raise SyntaxError(f"Forward reference must be an expression -- got {arg!r}")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
SyntaxError: Forward reference must be an expression -- got '*Ts'

[JelleZijlstra]

Also, there were a few buildbot failures when Pablo triggered builds on 1543785, but apart from the whitespace issue I fixed, they looked like random failures on unrelated tests.

[gvanrossum]

Thanks, go ahead, but do add the needed tests and fix the issues. Agreed on the parsing in get_type_hints.

[JelleZijlstra]

Thanks @mrahtz for the fixes! I'll do another review today or maybe tomorrow and then hopefully merge.

I think we can leave the get_type_hints() fix for another PR; no need to worry about that here.

[mrahtz]

@JelleZijlstra Oops, I only just saw your message 😅 Let me know if you'd prefer to take the get_type_hints() fix out of this PR.

[JelleZijlstra]

If you already implemented it, that's fine too :) Thanks!

[mrahtz]

I removed this because if both globals and locals are passed to exec, it treats the code as it if were executed in a class definition, which makes the new test I've added harder than it needs to be (we can't access c so easily). Removing this doesn't seem to break any of the rest of the tests, so I hope this is fine?

[mrahtz]

Whoop whoop! :D