I'm guessing there are still crasher bugs in here... E.g.

>>> from __future__ import string_annotations>>> def f(a: list[str]): pass... Segmentation fault: 11

If unparse the AST I would move the code into a separate file. There will be a lot of code, comparable with the size ofcompile.c.

Alright, I'm going to:

• change the future name back to annotations (yay!)
• stick to AST unparsing
• move this work to a separate file
• change concatenation to use the accumulator pattern (reusing_PyBytesWriter would be great)
• finish the functionality

Sounds great! I am pretty happy to see where this is going, I'd like to see it in a solid state by the time beta 1 comes around.

All comments from Serhiy's review acted upon, the import renamed back to "annotations" like commented above. The only missing piece in the implementation is f-string support but my battery will die soon so I wanted to push this out for you to look at.

Shouldn't segfault anymore, trying to use f-strings raises an exception instead.

Hm, the clang Travis CI build is failing due to invalid whitespace.make patchcheck is complaining aboutInclude/code.h, suggesting the following diff:
https://gist.github.com/ambv/9f0874d56c7cf0d5ce98dfef38de0ce6

This diff is suggesting a lot of changes but not on the single line that I modified¯\_(ツ)_/¯

I added a commit that fixes the whitespace according to the generated patch above so that I can see Travis CI passing. We can decide what to do with it later.

AppVeyor is failing because we need to modify PCbuild/* but I don't have access to a Windows box.

NEWS entry is not there yet since Blurb is tied to BPO issues and I'm wondering whether creating dummy issues for PEP work isn't redundant? I asked@larryhastings, we can also deal with this later.

Changes:

• Rebased on top of latest master
• Added a NEWS entry (manually since this is a PEP; the bot doesn't recognize this, therefore theskip news label has to stay)
• Moved the changes to PCbuild to the respective implementation commit ("Implement unparsing...")

This is ready for another review pass,@serhiy-storchaka. The only bit left is f-strings which is going to be a bit tedious so I'm waiting with it after a new round of feedback :-)

How, do we organize the updates totyping.get_type_hints() to work with "doubly quoted" strings? I mean this should still work with the__future__ import:

deff()->List['int']:    ...assertget_type_hints(f)['return']==List[int]

I suppose this can be part of the same PR, since this is not necessary in the backported version on PyPI. Also my updates totyping following PEP 560 should not have merge conflicts.

@ilevkivskyi, I want to fixget_type_hints() separately since there's really no reason why accepting"List['int']" shouldn't be supported even today. Same with fixing the self-class reference as you suggested on python-dev (I think I'd do it with a ChainMap though), fixing the forward ref cache conflicts, etc.

@ambv OK, I am fine with this as well.

It will take a time for making a review of such large change. But one comment I can say now.

The unparser adds parenthesis for grouping subexpression. They are added even if not strictly needed, e.g. ina + (b * (c ** d)). The problem is not that redundant parenthesis makes an expression less readable. The problem is that they increase the stack consumption when parse the expression again. It is possible that the original expression can be parsed, but parsing the unparsed expression will fail or even crash.

I already encountered with similar problem when worked on the parser of plural form expressions ingettext.py. A C-like syntax is parsed and converted to Python syntax, and the result is evaluated. I minimized the use of parenthesis. If the subexpression operator has higher priority than the operator of the outer expression, parenthesis are not added.

This is not a blocker, and we can solve this problem later, but you can think about this while I'm making a review.

Yes, in case ofgettext the one of purposes was to guard against a malicious input. In the case of annotations it is less likely. The other purpose -- speeding up the compilation.

This problem can be solved by assigning the numerical priority level to expressions and omitting parenthesis only if the current priority level is higher then the level of a super-expression (the sub-expression rather of a super-expression is responsible for adding parenthesis).

I have yet two questions.

1. Is the performance important here (I afraid yes)? The Python implementation would be simpler and more reliable. But much slower.

2. Do we need to support the full Python expression syntax? In particular arithmetic operations. Or it would be enough to support only the small subset used in type annotations? Names, attributes, indexing, tuples, what more?

As I said in my comment on Nov 23rd, to the best of my knowledge, the current state of the diff already omits all cases of spurious parens that occur in valid type annotations.

Don't spend your time on fighting with the extra parens if this distracts you from more prioritized tasks. If you don't solve this problem in your PR I'm going to do this after its merging. I suppose this will not add too much complexity. Your code already avoid producing the extra parens in many cases. This is enough for the initial implementation.

Yet one consideration. Could it help if introduce macros like theVISIT macro insymtable.c andcompile.c? They should call specified function with explicit and implicit arguments, check a result and return a failure if it is failed. Most functions could be just a short sequence of invocations of these macros. This technique is used in many places in CPython sources.

Could it help if introduce macros like the VISIT macro in symtable.c and compile.c?

I was thinking about this when I was originally writing this but wasn't sure if macros aren't reserved just for special usage so I avoided them. If you'd like, I can refactor the file to use a macro instead, you're right, that should shorten it quite a bit.

Alright, this is fully rebased without conflicts and all comments from previous code review are addressed. Things left to do:

• remove special handling of strings
• add support for f-strings
• (maybe?) refactor using a VISIT-style macro

Special handling of strings removed. I plan to add the missing f-string support in the first week of January so that the implementation is hopefully mergeable in 3.7.0a4.

Alright,@serhiy-storchaka, this is complete now, including f-string support! I realize it's pretty last minute, sorry for that.

A piece of useless statistics: this pull request was implementedin full during intercontinental flights. There's something very tranquil about sitting in one place for 10+ hours with no distractions.