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

Define _PyUnicode_DecodeUnicodeEscape even on Python 3.6+ #171

Merged
merged 1 commit into from Nov 10, 2021

Conversation

@hroncok
Copy link
Contributor

@hroncok hroncok commented Nov 10, 2021

Fixes #169

I have tested this on 3.9.8, 3.9.7 and 3.10.0 only. The CI tested this more.

@hroncok
Copy link
Contributor Author

@hroncok hroncok commented Nov 10, 2021

Some OSes on the CI install 3.9.7 and some install 3.9.8, which is convenient. It passes on both.

@hauntsaninja
Copy link
Collaborator

@hauntsaninja hauntsaninja commented Nov 10, 2021

Awesome, thank you!

@srittau
Copy link
Collaborator

@srittau srittau commented Nov 10, 2021

My knowledge of Python internals and typed_ast is very limited, but wouldn't it be safer to get rid of _PyUnicode_DecodeUnicodeEscape and call PyUnicode_DecodeUnicodeEscape directly? All the underlined function does is set first_invalid_escape of the only caller to NULL.

Edit: And that would mean the following block never gets called.

@hroncok
Copy link
Contributor Author

@hroncok hroncok commented Nov 10, 2021

My knowledge of typed_ast is nonexistent. I have no idea. I think this is a shim needed to support pre-3.6 Python as well.

@srittau
Copy link
Collaborator

@srittau srittau commented Nov 10, 2021

Well, let's merge this. I will send a separate PR trying my idea.

@srittau srittau merged commit 1232867 into python:master Nov 10, 2021
16 checks passed
@hroncok hroncok deleted the no-_PyUnicode_DecodeUnicodeEscape branch Nov 10, 2021
@hauntsaninja
Copy link
Collaborator

@hauntsaninja hauntsaninja commented Nov 10, 2021

So the case where we hit that block previously is, for example:

from typed_ast import ast3
ast3.parse(r'''
print('\Q')
''')

The only user visible difference I've been able to see is that if running with e.g. python -Werror, a SyntaxError is raised instead of a DeprecationWarning. The reason the code does this translation is because SyntaxErrors provide location information. See warn_invalid_escape_sequence. The resulting slight difference in behaviour between Python versions seems fine to me / seemed fine to whoever implemented this for Python 3.5 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants