bpo-30529: Fix errors for invalid whitespaces in f-string subexpressions. #1888
Conversation
…ons. 'invalid character in identifier' now is raised instead of 'f-string: empty expression not allowed' if a subexpression contains only whitespaces and they are not accepted by Python parser.
serhiy-storchaka
added
needs backport to 3.6
type-bug
ericvsmith
left a comment
ericvsmith
left a comment
There was a problem hiding this comment.
In general, it looks good, although I think it could be improved with the few tweaks I've mentioned. Not needing to convert from UTF-8 is definitely an improvement. Thanks!
Python/ast.c
Outdated
| all_whitespace = 0; | ||
| for (s = expr_start; s != expr_end; s++) { | ||
| char c = *s; | ||
| if (!(c == ' ' || c == '\t' || c == '\n' || c == '\014')) { |
There was a problem hiding this comment.
Wouldn't '\014' (formfeed) be better written as '\f'?
There was a problem hiding this comment.
The Python parser uses '\014'. I prefer writing it as '\f' too.
Lib/test/test_fstring.py
Outdated
| "f'{ !r}'", | ||
| "f'{10:{ }}'", | ||
| "f' { } '", | ||
| "f'''{\t\x0c\r\n}'''", |
There was a problem hiding this comment.
\x0c is \f, isn't it? I'd use that, instead.
There was a problem hiding this comment.
Yes. For unknown reasons the repr of '\f' is "'\x0c'".
| "f'{:x'", | ||
| ]) | ||
|
|
||
| self.assertAllRaise(SyntaxError, 'invalid character in identifier', |
There was a problem hiding this comment.
Can you add a comment here that says that eval("\xa0") raises SyntaxError with the "invalid character in identifier" text? I think that it's important to know that what you're doing here is trying to match the Python parser. (I realize that, with your change, you're "matching" by actually using the Python parser.)
Or, maybe also add 'eval("\xa0")' to the array to make it obvious that we want the same error raised in both cases?
| for (i = 0; i < len; i++) { | ||
| if (!Py_UNICODE_ISSPACE(PyUnicode_READ(kind, data, i))) { | ||
| all_whitespace = 0; | ||
| for (s = expr_start; s != expr_end; s++) { |
There was a problem hiding this comment.
Can you add a comment here that you're skipping the same whitespace that the Python parser recognizes, and that's why we're specifically not calling Py_UNICODE_ISSPACE? I can see someone (might be me!) and not realizing that Python itself doesn't use Py_UNICODE_ISSPACE and trying to add it back.
…pressions. (pythonGH-1888) 'invalid character in identifier' now is raised instead of 'f-string: empty expression not allowed' if a subexpression contains only whitespaces and they are not accepted by Python parser. (cherry picked from commit 2e9cd58)
4 participants
'invalid character in identifier' now is raised instead of
'f-string: empty expression not allowed' if a subexpression contains
only whitespaces and they are not accepted by Python parser.