-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
gh-135148: Correctly handle f/t strings with comments and debug expressions #135198
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
bd49471
to
5f8456e
Compare
Parser/lexer/lexer.c
Outdated
|
||
// Process each character of input |
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.
@lysnikolaou please read this carefully. I am mostly sure its correct but it really needs some extra eyes
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.
Yeah, I was thinking something very similar as a solution, too! I found one problem, but it looks good otherwise.
Parser/lexer/lexer.c
Outdated
if (i > 0 && tok_mode->last_expr_buffer[i-1] == ch && | ||
i > 1 && tok_mode->last_expr_buffer[i-2] == ch) { | ||
// Found triple quote - copy all three quotes | ||
result[j++] = ch; | ||
result[j++] = ch; | ||
result[j++] = ch; | ||
i += 2; // Skip the other two quotes | ||
continue; | ||
} |
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.
I don't think this is gonna work as intended. Let's say we have a triple-quoted string. Then, I think the following sequence is going to happen:
ch == '"'
,!in_string
, so we're doingin_string = 1; string_quote = ch;
ch == '"'
,in_string
,ch == string_quote
, but the insideif
fails because we're only on the second quote, so we're settingin_string = 0
backch == '"'
,!in_string
and we're settingin_string = 1
again.
For example the following does not work correctly:
>>> f'{ # some comment goes here
... """hello"""=}'
' \n"""hello"""""\'hello\''
Same goes for the construct above where we're doing the hash detection. Example that fails there:
>>> f'{""" # no c
... """ # c
... =}'
'""" # no c\n""" # c\n=\' # no c\\n\''
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.
Good catch sir!
I have pushed a version that addresses this and simplifies stuff a bit
When you're done making the requested changes, leave the comment: |
Uh oh!
There was an error while loading. Please reload this page.