[3.13] gh-154711: Strip comments after escaped quotes in f-string debug text - #154914
ArneshBanerjee wants to merge 1 commit into
Conversation
|
All commit authors signed the Contributor License Agreement. |
| @@ -0,0 +1,3 @@ | |||
| Fix a bug where a comment placed after a string that ends with an escaped | |||
There was a problem hiding this comment.
This goes back to 3.12 (gh-112243) and the same code is still in 3.13 and 3.14, so we want the backport labels here, no?
There was a problem hiding this comment.
Confirmed, the same escape-skip bug is present in 3.12 (tokenizer.c) and 3.13 (lexer.c) as well. Happy to have backport labels added.
| while (i < tok_mode->last_expr_size - tok_mode->last_expr_end) { | ||
| char ch = tok_mode->last_expr_buffer[i]; | ||
|
|
||
| // Copy escaped characters as-is. This keeps an escaped quote from |
There was a problem hiding this comment.
Nit, feel free to ignore: we now have the same escape/quote scanning written twice, and the two copies drifting apart is what caused this bug in the first place. Maybe we can pull it into a small helper in a follow-up so they cannot diverge again.
There was a problem hiding this comment.
Good point, agreed a shared helper would prevent this drift. Opened gh-155516 to track it as a follow-up.
|
@pablogsal gentle ping on this one. The two review comments are addressed: I confirmed the same escape-skip bug exists in 3.12 and 3.13, and the shared-helper cleanup is tracked separately in gh-155516. Could you add the backport labels when you get a chance? I can't set them myself. |
|
@pablogsal heads up: main no longer has this bug after the tokenizer rework in gh-153569. Comments are now tracked as token spans, so an escaped quote can't confuse the scan anymore. I built current main and the comment is stripped the same way with and without an escaped quote. The old scanning loop is still in |
|
Thanks for checking! Let's do what you propose |
…ng debug text When a replacement field ended with a string that had an escaped quote and was followed by a comment, the comment leaked into the debug f-string output. The comment detection loop in set_fstring_expr skips escaped characters, but the comment stripping loop did not. So an escaped quote left the scanner thinking it was still inside a string, and the comment was never removed. This makes the stripping loop skip escaped characters the same way the detection loop does. 3.14 and later already have this fix from pythongh-154719.
3de01fa to
2877218
Compare
ArneshBanerjee
changed the title
|
@pablogsal small correction to my last comment. I checked the branches again, and 3.14 and 3.15 already have this fix. It came in with the gh-154719 backports on Sep 13, so I was wrong about those two. Only 3.13 still has the bug. So I retargeted this PR to 3.13, with the f-string test only since 3.13 has no t-strings. The tests for main are in #158220. Those could be backported to 3.15 and 3.14, which have the fix but no test for this case. |
Please don't do this: the branches diverged years ago, and this PR now has many conflicts and has pinged dozens of CODEOWNERS. Please open a fresh PR against |
|
Sorry about that. Opened #158310 against 3.13 from a fresh checkout. |
3 participants
When a replacement field ended with a string that had an escaped quote and was followed by a comment, the comment leaked into the debug f-string output.
The comment detection loop in
set_fstring_exprskips escaped characters, but the comment stripping loop did not. So an escaped quote left the scanner thinking it was still inside a string, and the comment was never removed.This makes the stripping loop skip escaped characters the same way the detection loop does. 3.14 and 3.15 already have the same fix from the gh-154719 backports, and main is not affected after gh-153569, so this is 3.13 only.