Disallow implicit concatenation of t-strings and other string types#19485
Merged
dylwil3 merged 14 commits intoastral-sh:mainfrom Jul 27, 2025
Merged
Disallow implicit concatenation of t-strings and other string types#19485dylwil3 merged 14 commits intoastral-sh:mainfrom
dylwil3 merged 14 commits intoastral-sh:mainfrom
Conversation
Contributor
|
Contributor
|
dylwil3
commented
Jul 25, 2025
| InterpolatedStringLiteralElement { | ||
| range: 10..17, | ||
| node_index: AtomicNodeIndex(..), | ||
| value: "{foo}", |
Collaborator
Author
There was a problem hiding this comment.
This is correct - the actual test has
let source = r#"t"{a}{ b }{{foo}}""#;so we are correctly reading the escaped braces.
Comment on lines
42
to
45
| ExprNumberLiteral { | ||
| node_index: AtomicNodeIndex(..), | ||
| range: 3..5, | ||
| range: 9..11, | ||
| value: Int( |
Collaborator
Author
There was a problem hiding this comment.
This is also correct - it's just one of those unfortunate diffs that makes it look like something changed when it was just moved.
19a8667 to
1762483
Compare
MichaReiser
approved these changes
Jul 26, 2025
1762483 to
34a9313
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As of this cpython PR, it is not allowed to concatenate t-strings with non-t-strings, implicitly or explicitly. Expressions such as
"foo" t"{bar}"are now syntax errors.This PR updates some AST nodes and parsing to reflect this change.
The structural change is that
TStringPartis no longer needed, since, as in the case ofBytesStringLiteral, the only possibilities are that we have a singleTStringor a vector of such (representing an implicit concatenation of t-strings). This removes a level of nesting from many AST expressions (which is what all the snapshot changes reflect), and simplifies some logic in the implementation of visitors, for example.The other change of note is in the parser. When we meet an implicit concatenation of string-like literals, we now count the number of t-string literals. If these do not exhaust the total number of implicitly concatenated pieces, then we emit a syntax error. To recover from this syntax error, we encode any t-string pieces as invalid string literals (which means we flag them as invalid, record their range, and record the value as
""). Note that if at least one of the pieces is an f-string we prefer to parse the entire string as an f-string; otherwise we parse it as a string.This logic is exactly the same as how we currently treat
BytesStringLiteralparsing and error recovery - and carries with it the same pros and cons.Finally, note that I have not implemented any changes in the implementation 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 concatenating t-strings with t-strings.