gh-129093: Fix f-string debug text sometimes getting cut off when expression contains !

[tomasr8]

Here's what I think is happening (though I may be wrong, the lexer code and especially the f-string code is really complex):

For an f-string replacement field such as {1!=2=} we end up in this branch of the parser:

cpython/Grammar/python.gram

Lines 916 to 918 in 29caec6

	fstring_replacement_field[expr_ty]:
	| '{' a=annotated_rhs debug_expr='='? conversion=[fstring_conversion] format=[fstring_full_format_spec] rbrace='}' {
	_PyPegen_formatted_value(p, a, debug_expr, conversion, format, rbrace, EXTRA) }

The original source (i.e. debug_expr) is stored on the rbrace's token->metadata:

cpython/Parser/lexer/lexer.c

Line 171 in 29caec6

token->metadata = res;

which is how the _PyPegen_formatted_value function injects it into the AST (as debug_text):

cpython/Parser/action_helpers.c

Lines 1492 to 1493 in 29caec6

	expr_ty debug_text = _PyAST_Constant(debug_metadata, NULL, lineno, col_offset + 1, debug_end_line,
	debug_end_offset - 1, p->arena);

The token->metadata itself gets the debug text from tok_mode->last_expr_buffer and tok_mode->last_expr_end.

cpython/Parser/lexer/lexer.c

Lines 159 to 163 in 29caec6

	res = PyUnicode_DecodeUTF8(
	tok_mode->last_expr_buffer,
	tok_mode->last_expr_size - tok_mode->last_expr_end,
	NULL
	);

If we go back to the example {1!=2=}, to parse the fstring_replacement_field rule, the parser first tries to parse the annotated_rhs which consumes 1 at first since it's a valid expression. It then skips debug_expr='='? and starts parsing the conversion specifier which matches the following !. Now that it hit !, it sets tok_mode->last_expr_end in _PyLexer_update_fstring_expr:

cpython/Parser/lexer/lexer.c

Lines 212 to 218 in 29caec6

	case '}':
	case '!':
	case ':':
	if (tok_mode->last_expr_end == -1) {
	tok_mode->last_expr_end = strlen(tok->start);
	}
	break;

The problem is that now the parser needs to backtrack. The second time it parses the full 1!=2 as annotated_rhs but because tok_mode->last_expr_end is guarded with if(tok_mode->last_expr_end == -1){} it never gets updated which is what is causing this issue.

Simply removing that if check seems to work, but I'm not 100% sure this is the best way to fix it. It'd be great if a parser expert could check this :)

• Issue: f-string evaluation of conditional expressions with != operator seems to fail #129093

[pablogsal]

This looks like a correct analysis.

Thanks a lot @tomasr8 for dedicating the time to chase the original cause

[pablogsal]

I have made some spelunking and I have chased this code to the original implementation in commit 1ef61cf. The code was protecting against a condition that is not true anymore and unfortunately never got removed

[miss-islington-app]

Thanks @tomasr8 for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

[miss-islington-app]

Thanks @tomasr8 for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

[miss-islington-app]

Sorry, @tomasr8 and @pablogsal, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 767cf708449fbf13826d379ecef64af97d779510 3.12

[bedevere-app]

GH-129163 is a backport of this pull request to the 3.13 branch.

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot iOS ARM64 Simulator 3.13 has failed when building commit a379749.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1386/builds/950) and take a look at the build logs.
4. Check if the failure is related to this commit (a379749) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1386/builds/950

Summary of the results of the build (if available):

Click to see traceback logs

remote: Enumerating objects: 13, done.        
remote: Counting objects:  50% (1/2)        
remote: Counting objects: 100% (2/2)        
remote: Counting objects: 100% (2/2), done.        
remote: Compressing objects:  50% (1/2)        
remote: Compressing objects: 100% (2/2)        
remote: Compressing objects: 100% (2/2), done.        
remote: Total 13 (delta 0), reused 0 (delta 0), pack-reused 11 (from 2)        
From https://github.com/python/cpython
 * branch                    3.13       -> FETCH_HEAD
Note: switching to 'a3797492179c249417a06d2499a7d535d453ac2c'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at a3797492179 [3.13] gh-129093: Fix f-string debug text sometimes getting cut off when expression contains `!` (GH-129159) (#129163)
Switched to and reset branch '3.13'

configure: WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)
configure: WARNING: pkg-config is missing. Some dependencies may not be detected correctly.

configure: WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)
configure: WARNING: pkg-config is missing. Some dependencies may not be detected correctly.

Found more than one new device: {'5CAA0336-9CE1-4222-BFE3-ADA405F766DE', 'DD108383-685A-4400-BF30-013AA82C4A61'}
make: *** [testios] Error 1

[mhsmith]

@freakboy3742: This iOS failure has happened twice in the last 24 hours on each of the main and 3.13 branches, but with successful builds in between.

[pablogsal]

Yeah is certainly unrelated to this PR

[freakboy3742]

Yeah - this specific failure is a weird race condition that I thought would be fairly rare, but I've now seen it a couple of times. Logged as #129200.