bpo-32711: Fix warnings for Python/ast_unparse.c #5426
Conversation
matrixise
force-pushed
the
bpo-32711_fix_warnings_in_python_astunparse
branch
from
351f1a3 to
4c41d41
Compare
|
with the help of @tiran I have modified my commit and fix all the warnings in |
tiran
left a comment
tiran
left a comment
There was a problem hiding this comment.
Rather then setting op to NULL, please modify the switch block. You either have to default: op = ""; break; or raise an exception and return -1.
Python/ast_unparse.c
Outdated
There was a problem hiding this comment.
You cannot pass NULL to append_charp because _PyUnicodeWriter_WriteASCIIString doesn't handle NULL as second argument.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
matrixise
force-pushed
the
bpo-32711_fix_warnings_in_python_astunparse
branch
from
4c41d41 to
e97538a
Compare
Fix the warnings in the new Python/ast_unparse.c file
matrixise
force-pushed
the
bpo-32711_fix_warnings_in_python_astunparse
branch
from
e97538a to
dff6d17
Compare
|
I would like to be sure, for a fix as this one, do we need to add a News file ? maybe we could add a |
Python/ast_unparse.c
Outdated
| case FloorDiv: op = " // "; break; | ||
| case Pow: op = " ** "; break; | ||
| default: | ||
| PyErr_SetString(PyExc_SystemError, |
There was a problem hiding this comment.
If this case cannot happen, maybe Py_UNREACHABLE(); is enough here?
Python/ast_unparse.c
Outdated
| default: | ||
| PyErr_SetString(PyExc_SystemError, | ||
| "unknown operator inside f-string"); | ||
| return -1; |
There was a problem hiding this comment.
in the default ? ok, Py_UNREACHABLE() is a macro for abort()
There was a problem hiding this comment.
yes. instead of PyErr_SetString()+return -1. But it's an open question, I don't know if if's safe. That's why I asked @ambv to take a look.
There was a problem hiding this comment.
yep, I do understand ;-)
|
@ambv: would you mind to have a look? |
|
@ambv just a small ping/pong for the status of this PR. |
|
cool and now, who wants to have the responsibility of the merge for this PR ? ;-) |
|
Thanks @matrixise for the PR, and @tiran for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7. |
|
@tiran: Please replace |
|
GH-5475 is a backport of this pull request to the 3.7 branch. |
* bpo-32711: Fix warnings for Python/ast_unparse.c (cherry picked from commit 83ab995) Co-authored-by: Stéphane Wirtel <[email protected]>
|
Sorry, was slammed this week. Thanks for fixing so quickly! |
7 participants
Fix the warnings in the new Python/ast_unparse.c file
https://bugs.python.org/issue32711