Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.4k
gh-132661: DisallowTemplate/str concatenation after PEP 750 spec update#135996
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
sobolevn left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thank you!
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
lysnikolaou commentedJun 30, 2025
Happy to help out / review this PR if you wanna take a stab at it. Feel free to reach out if you have any specific questions. |
Today was, alas, not that day.
davepeck commentedJul 8, 2025
@lysnikolaou Think this is ready. The only question for me is whether we want to go further with |
sobolevn left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thank you! Several small nitpicks :)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Misc/NEWS.d/next/Core_and_Builtins/2025-07-08-23-22-08.gh-issue-132661.34ftJl.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
lysnikolaou left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks@davepeck for working on this! Looks great in general. Left a few unimportant comments and one more significant one about how to implement this in the parser. Also, we'll need to changeast_unparse.c aroundhttps://github.com/python/cpython/blob/main/Python/ast_unparse.c#L715.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
efimov-mikhail commentedJul 16, 2025
LGTM |
lysnikolaou commentedJul 17, 2025
Looks great and aaalmost there. I have one last thing I missed during my previous reviews. If I remember correctly,https://github.com/python/cpython/blob/main/Python/codegen.c#L4084-L4093 can be removed as well, because it's only there to handle After that, it's gonna be good to go, promise! |
davepeck commentedJul 17, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@lysnikolaou Ah, yes -- thank you. Removed now; tests continue to pass. (If we prefer, I can replace |
lysnikolaou left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks good to me! Thanks@davepeck! Great work.
c5e77af intopython:mainUh oh!
There was an error while loading.Please reload this page.
Thanks@davepeck for the PR, and@lysnikolaou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Sorry,@davepeck and@lysnikolaou, I could not cleanly backport this to |
lysnikolaou commentedJul 21, 2025
I can handle tha backport. |
…0 spec update (python#135996)Co-authored-by: sobolevn <mail@sobolevn.me>Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>(cherry picked from commitc5e77af)
…19485)As of [this cpython PR](python/cpython#135996),it is not allowed to concatenate t-strings with non-t-strings,implicitly or explicitly. Expressions such as `"foo" t"{bar}"` are nowsyntax errors.This PR updates some AST nodes and parsing to reflect this change.The structural change is that `TStringPart` is no longer needed, since,as in the case of `BytesStringLiteral`, the only possibilities are thatwe have a single `TString` or a vector of such (representing an implicitconcatenation of t-strings). This removes a level of nesting from manyAST expressions (which is what all the snapshot changes reflect), andsimplifies some logic in the implementation of visitors, for example.The other change of note is in the parser. When we meet an implicitconcatenation of string-like literals, we now count the number oft-string literals. If these do not exhaust the total number ofimplicitly concatenated pieces, then we emit a syntax error. To recoverfrom this syntax error, we encode any t-string pieces as _invalid_string literals (which means we flag them as invalid, record theirrange, and record the value as `""`). Note that if at least one of thepieces is an f-string we prefer to parse the entire string as anf-string; otherwise we parse it as a string.This logic is exactly the same as how we currently treat`BytesStringLiteral` parsing and error recovery - and carries with itthe same pros and cons.Finally, note that I have not implemented any changes in theimplementation of the formatter. As far as I can tell, none are needed.I did change a few of the fixtures so that we are always concatenatingt-strings with t-strings.
…0 spec update (python#135996)Co-authored-by: sobolevn <mail@sobolevn.me>Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
serhiy-storchaka commentedAug 14, 2025
Reminder about backporting.@davepeck@lysnikolaou |
encukou commentedAug 15, 2025
GH-136901 is a backport of this pull request to the 3.14 branch. |
…0 spec update (python#135996)Co-authored-by: sobolevn <mail@sobolevn.me>Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
… PEP 750 spec update (python#135996) (python#136901)Co-authored-by: Dave Peck <davepeck@gmail.com>Co-authored-by: sobolevn <mail@sobolevn.me>
Uh oh!
There was an error while loading.Please reload this page.
Following thesteering council decision and correspondingupdate to PEP750, we are removing support for both implicit and explicit
Template/strconcatenation.