Wow, this is an excellent insight and looks fantastic! I can try to review it later tomorrow but I have an initial question:

Since we are storing the offset in fstring_col_offset, what happens with nested f-strings? Every parser instance has its own 'fstring_col_offset' and therefore they will not collide or there is something to consider on the propagation of these values?

Since we are storing the offset in fstring_col_offset, what happens with nested f-strings? Every parser instance has its own 'fstring_col_offset' and therefore they will not collide or there is something to consider on the propagation of these values?

Nested f-strings should be handled correctly already. I checked test_fstring.py for tests the test the node locations of nested f-strings before pushing and there is one, so I guess it's ok. Hand-testing with some more examples verified this.

fstring_col_offset actually only gets used for the caret in the case of an error. Because only the expression part of the f-string is shown on an error, we need to shift the caret accordingly.

Nested f-strings are no problem because t->col_offset points to the correct column offset of the nested f-string (this is due to the pre-feeding of the parser with a beginning offset and due to correctly computing of the offset in _PyPegen_fill_token when it first gets fetched from the tokenizer). So, if the column offset of the nested fstring is correct, it only gets down to fstring_find_expr_location computing the correct amount of characters between the start of the nested f-string and the start of its expression. This function has been battle-tested by previous Python versions and in the old version of the new parser (the one before this PR), so I guess we're okay.

Does this make sense?

Although is not caused by this PR, I noticed that we have some inconsistency with the old parser regarding f-strings. For instance:

❯ ./python -X oldparser
Python 3.9.0b3+ (heads/deprecation_warnings-dirty:21b84dd204, Jun 21 2020, 19:23:13)
[GCC 10.1.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> x =23; f'\n{1+2} - {lel $ 2}'
  File "<fstring>", line 1
    (lel $ 2)
         ^
SyntaxError: invalid syntax
>>>

❯ ./python
>>> x =23; f'\n{1+2} - {lel $ 2}'
  File "<stdin>", line 1
    (lel $ 2)
             ^
SyntaxError: invalid syntax
>>>

Oh, this is a bug that needs to be fixed. I think I know what's causing it.

I realised that there was a bug in my implementation, in case the fstring wasn't starting at offset 0 (when we compute the offset of the caret in the error text, we also need to account for all the characters before the f-string, in case it's not at offset 0). The fix also fixes the bug @pablogsal mentioned above, so that's a win-win.

Closing and reopening to trigger the CI again

Thanks @lysnikolaou for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

Sorry, @lysnikolaou and @pablogsal, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 1f0f4abb110b9fbade6175842b6a26ab0b8df6dd 3.9

GH-21190 is a backport of this pull request to the 3.9 branch.

🎉 🎉

I feel like this is one of the most major improvements we've had lately. Thanks for the review @pablogsal!

I feel like this is one of the most major improvements we've had lately. Thanks for the review @pablogsal!

Yeah, me too! This was a very good insight and a great job. Well done!