-
-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
bpo-37050: Remove expr_text from FormattedValue ast node, use Constant node instead #13597
bpo-37050: Remove expr_text from FormattedValue ast node, use Constant node instead #13597
Conversation
@@ -290,6 +284,15 @@ def test_annotations(self): | |||
eq("(x:=10)") | |||
eq("f'{(x:=10):=10}'") | |||
|
|||
# f-strings with '=' don't round trip very well, so set the expected |
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 think it is worth to move this into separate method. It can even be decorated as cpython_only
. I think that if an alternate implementation is smart enough to roundtrip these case, it is still a valid Python implementation.
|
||
*expression is set to the expression. For an '=' "debug" expression, | ||
*expr_text is set to the debug text (the original text of the expression, | ||
*including the '=' and any whitespace around it, as a string object). If |
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.
*including the '=' and any whitespace around it, as a string object). If | |
including the '=' and any whitespace around it, as a string object). If |
*expression is set to the expression. For an '=' "debug" expression, | ||
*expr_text is set to the debug text (the original text of the expression, | ||
*including the '=' and any whitespace around it, as a string object). If | ||
*not a debug expression, *expr_text set to NULL. */ |
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.
*not a debug expression, *expr_text set to NULL. */ | |
not a debug expression, *expr_text set to NULL. */ |
expr_text_end = *str; | ||
|
||
/* Set *expr_text to the text of the expression. */ | ||
*expr_text = PyUnicode_FromStringAndSize(expr_start, *str-expr_start); |
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.
Note that *expr_text
can be leaked at the error
label.
/* We have a literal, concatenate it. */ | ||
assert(PyUnicode_GET_LENGTH(literal[i]) != 0); | ||
if (FstringParser_ConcatAndDel(state, literal[i]) < 0) | ||
return -1; |
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.
literal[1]
can be leaked here.
Thanks, Serhiy. I was in a rush to get this change in for B1. I'll address these in a subsequent commit. |
…t node instead (pythonGH-13597) When using the "=" debug functionality of f-strings, use another Constant node (or a merged constant node) instead of adding expr_text to the FormattedValue node.
https://bugs.python.org/issue37050