Issue 32711: Fix warnings for Python/ast_unparse.c

Python/ast_unparse.c: At top level:
Python/ast_unparse.c:1122:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
 maybe_init_static_strings()
 ^~~~~~~~~~~~~~~~~~~~~~~~~
Python/ast_unparse.c: In function ‘append_ast_binop’:
Python/ast_unparse.c:105:15: warning: ‘op’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (-1 == append_charp(writer, op)) {
               ^~~~~~~~~~~~~~~~~~~~~~~~
Python/ast_unparse.c: In function ‘append_ast_unaryop’:
Python/ast_unparse.c:132:15: warning: ‘op’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (-1 == append_charp(writer, op)) {
               ^~~~~~~~~~~~~~~~~~~~~~~~

I'm getting slightly different warnings with GCC 7.2.1:

Python/ast_unparse.c: In function ‘append_formattedvalue’:
Python/ast_unparse.c:859:41: warning: ordered comparison of pointer with integer zero [-Wextra]
     if (e->v.FormattedValue.format_spec > 0) {
                                         ^
Python/ast_unparse.c: At top level:
Python/ast_unparse.c:1122:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
 maybe_init_static_strings()
 ^~~~~~~~~~~~~~~~~~~~~~~~~
Python/ast_unparse.c: In function ‘append_ast_expr’:
Python/ast_unparse.c:23:16: warning: ‘op’ may be used uninitialized in this function [-Wmaybe-uninitialized]
         return _PyUnicodeWriter_WriteASCIIString(writer, charp, -1);
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Python/ast_unparse.c:119:17: note: ‘op’ was declared here
     const char *op;
                 ^~
Python/ast_unparse.c:23:16: warning: ‘op’ may be used uninitialized in this function [-Wmaybe-uninitialized]
         return _PyUnicodeWriter_WriteASCIIString(writer, charp, -1);
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Python/ast_unparse.c:79:17: note: ‘op’ was declared here
     const char *op;

Hi Christian, I proposed a patch ofr the maybe_init_static_strings and for the const char *op, but I don't know how to fix for the e->v.FormattedValue.format_spec > 0, I don't know how to interpret it :/

But for _PyUnicodeWriter_WriteASCIIString, I don't get that warning.

Hi @Chris, with your help I just fixed the last warning, but I don't have the warning with _PyUnicodeWriter_WriteASCIIString :/

the switch blocks in both functions need to handle invalid input:

default: PyExc_SetString...; return -1;

Hi Christian,

I add Lukasz in the loop because Victor asked to him a small review because he is not sure about the right way, PyErr_XXX or Py_UNREACHABLE.

Thank you for your review.

I think it is better to not add the default case in switch statements. If once we will add a new binary or unary operator, but forgot to update ast_unparse.c the compiler will immediately warn about this if there are no default cases. But if there is a default case the compiler will be silent.

Hi Serhiy,

but currently, there is some warnings because op has no value and can not be assigned to NULL because _PyUnicodeWriter_WriteASCIIString can't accept a NULL value.

What do you propose? Warnings at the compilation step and an eventual crash if we don't handle this case because there is no default?

In my experience, Serhiy's example won't work. C doesn't guarantee that the functions will not be called with an unsupported op. Either my proposal or Victor's proposal are the correct way to solve the warning. Victor's proposal is even better because it makes the interpreter fail when an invalid operator is used.

then I will use the approach of Victor with the Py_UNREACHABLE macro and will push asap.

Christian, I just pushed my code, wait for the feedback from Travis.

Thanks

New changeset 83ab995871ffd504ac229bdbf5b9e9ffc1032815 by Christian Heimes (Stéphane Wirtel) in branch 'master':
bpo-32711: Fix warnings for Python/ast_unparse.c (#5426)
https://github.com/python/cpython/commit/83ab995871ffd504ac229bdbf5b9e9ffc1032815

New changeset 78758f29b13aba6136f4c0a15d4457fbf92c5eef by Christian Heimes (Miss Islington (bot)) in branch '3.7':
[3.7] bpo-32711: Fix warnings for Python/ast_unparse.c (GH-5426) (#5475)
https://github.com/python/cpython/commit/78758f29b13aba6136f4c0a15d4457fbf92c5eef

Thanks!

resolution: fixed
stage: patch review -> resolved