| msg76123 - (view) |
Author: Akira Kitada (akitada) * |
Date: 2008-11-20 18:20 |
| Some compilers don't understand "%zd" format specifer and gcc 2.95.4 is one of them. (You can find more on http://www.and.org/vstr/printf_comparison) When building Python with such compilers, you will see a lot of "warning: unknown conversion type character `z' in format" messages. This is not a big issue but I think this can be fixed by using autoconf right way because configure script seems checking availability of "zd". "checking for %zd printf() format support" So I suppose there is a way to eliminate this warning completely. |
|
|
| msg76126 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2008-11-20 18:43 |
| If you start at line 3652 in configure.in you fill find the check for the %zd format specifier. Any patch to make it more robust would be appreciated. |
|
|
| msg76273 - (view) |
Author: Akira Kitada (akitada) * |
Date: 2008-11-23 19:26 |
| I looked into the code and found these warnings about 'z' were not from printf family but PyString_FromFormat. Apparently PyString_FromFormat handles the 'z' itself, without delegating that to printf family. Then why am I getting these warnings? I could remove these by using PY_FORMAT_SIZE_T, but it's not for them, according to http://bugs.python.org/issue3743 and the source code. |
|
|
| msg76279 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2008-11-23 22:52 |
| You receive these messages because PyString_FromFormat is declared with a special directive __attribute__(format(printf, 1, 2)). Gcc then takes the format string as a regular printf format and validates it against the passed arguments. You can safely ignore these warnings. Maybe python should disable this __attribute__ for old compilers. |
|
|
| msg76360 - (view) |
Author: Akira Kitada (akitada) * |
Date: 2008-11-24 21:00 |
| Amaury, thank you for pointing that out. Attached patch disables this "__attribute__" when PY_FORMAT_SIZE_T is undefined after configure script. I confirmed this eliminate warnings in FreeBSD 4.11 |
|
|
| msg76370 - (view) |
Author: Roumen Petrov (rpetrov) * |
Date: 2008-11-24 23:47 |
| Technically patch is not correct : it disable __attribute__ always . The name of the macro is Py_GCC_ATTRIBUTE. It is a generic name and is possible(in future) macro to be used set other attributes. It is just a warning and is not related to "compile error" (issue type). I will agree with patch that change name of attribute(as example Py_GCC_FORMAT_ATTRIBUTE) that show its specific usage. |
|
|
| msg76416 - (view) |
Author: Akira Kitada (akitada) * |
Date: 2008-11-25 16:09 |
| Roumen, Thanks for the feedback! This patch disables __attribute__ when PY_FORMAT_SIZE_T isn't defined, not always. About the name of the macro, I think you are right. Py_GCC_FORMAT_ATTRIBUTE would be much better name if it is not used for other uses. (I'm not sure) I'll look into the code more and if it seems being safe to rename it, I'll update the patch and resubmit it. |
|
|
| msg76439 - (view) |
Author: Roumen Petrov (rpetrov) * |
Date: 2008-11-25 23:11 |
| Akira Kitada wrote: > Akira Kitada <akitada@gmail.com> added the comment: > > Roumen, > Thanks for the feedback! > > This patch disables __attribute__ when PY_FORMAT_SIZE_T isn't defined, > not always. I would like to clarify. After patch if PY_FORMAT_SIZE_T isn't defined then __attribute__(XXX) will be ignored always irrespective of content, i.e. XXX. > About the name of the macro, I think you are right. > Py_GCC_FORMAT_ATTRIBUTE would be much better name if it is not used for > other uses. (I'm not sure) > I'll look into the code more and if it seems being safe to rename it, > I'll update the patch and resubmit it. The current python code use Py_GCC_ATTRIBUTE only for 'format(printf(...)) ' attribute. The patch will prevent python to use this macro in future for other attributer. Also I'm not sure that I would like warning to be hidden. As example I know that GCC(mingw) 3.5.x don't support %z and I prefer to see those warnings instead to hide them. Roumen |
|
|
| msg76442 - (view) |
Author: Akira Kitada (akitada) * |
Date: 2008-11-25 23:39 |
| Thank you again for the feedback. I think those warnings are not so useful, or even misleading, when we know they are handled appropriately. with these warnings hidden, it would get much more easy to read build log and find real warnings there. As for renaming Py_GCC_ATTRIBUTE, yes this would prevent python to use this macro in future for other attributer, but after all python does not need it for now. So I think that's not a problem. Someone will write it when he/she need it. |
|
|
| msg76585 - (view) |
Author: Akira Kitada (akitada) * |
Date: 2008-11-29 10:48 |
| The new patch attached renamed Py_GCC_ATTRIBUTE to Py_GCC_FORMAT_ATTRIBUTE. As Roumen pointed out, "the current python code use Py_GCC_ATTRIBUTE only for 'format(printf(...)) ' attribute.", so this change should not be no harm itself. Because a GCC attribute was/will be introduced in different version of GCC, we cannot have generic Py_GCC_ATTRIBUTE easily. You can find availability of a attribute at http://www.ohse.de/uwe/articles/gcc-attributes.html http://www.ohse.de/uwe/articles/gcc-attributes.html |
|
|
| msg76587 - (view) |
Author: Akira Kitada (akitada) * |
Date: 2008-11-29 12:15 |
| Tested my patch on * FreeBSD 4.11 (gcc version 2.95.4, does not support "z") * FreeBSD 6.3 (gcc version 3.4.6, supports "z") and it worked fine on both. (using trunk) |
|
|
| msg76588 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2008-11-29 12:44 |
| I'm opposed to applying this patch to a maintenance branch. It removes the Py_GCC_ATTRIBUTE macro, which some extension modules might rely on. I also wonder whether it is really necessary to complicate the code just so that gcc 2.95 stops producing some warnings. So I propose to close this as "won't fix". |
|
|
| msg76593 - (view) |
Author: Akira Kitada (akitada) * |
Date: 2008-11-29 13:28 |
| Martin, Thank you for the feedback. I think we can avoid the problem that would be introduced by the removal of Py_GCC_ATTRIBUTE by leaving it as is (probably adding "#warning this macro is deprecated. Please use Py_GCC_FORMAT_ATTRIBUTE instead"). The patch looks not so complicated to me. It's just a renaming of a macro. The new name is more explicit. IMHO, source code should be kept warning-free, as long as the fix is simple and it does not cost much. So I think now the question would be whether having explicitly named macro and warning-free code for old compilers worth the effort of renaming all Py_GCC_ATTRIBUTE to Py_GCC_FORMAT_ATTRIBUTE. I would like to leave the decision to the core developers. |
|
|
| msg76594 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2008-11-29 13:36 |
| Ok. Removing 2.5.3 from the target releases until a decision is made (and potentially a new patch is available). |
|
|
| msg76601 - (view) |
Author: Roumen Petrov (rpetrov) * |
Date: 2008-11-29 16:54 |
| My idea was not to remove Py_GCC_ATTRIBUTE but the new macro with specific name that use Py_GCC_ATTRIBUTE. If macro name is Py_GCC_FORMAT_ATTRIBUTE then something similar to : #if !defined(PY_FORMAT_SIZE_T) #define Py_GCC_FORMAT_ATTRIBUTE(type, str_idx, arg_idx) #else #define Py_GCC_FORMAT_ATTRIBUTE(type, str_idx, arg_idx) Py_GCC_ATTRIBUTE((format(type, str_idx, arg_idx)) #endif and next in the code s/Py_GCC_ATTRIBUTE(...)/Py_GCC_FORMAT_ATTRIBUTE(...)/g as rest of the proposed patch. |
|
|
| msg76647 - (view) |
Author: Akira Kitada (akitada) * |
Date: 2008-11-30 17:56 |
| Attached patch just leaves Py_GCC_ATTRIBUTE as it is now. Here is the highlight. /* * Hide GCC's format attribute from compilers if one of the following is true: * a) the compiler does not support it and not on RISC OS * b) the compiler does not support "z" printf format specifier */ #if ((!defined(__GNUC__) | |
__GNUC__ < 2 |
|
| msg107985 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2010-06-17 02:05 |
| Martin said: I also wonder whether it is really necessary to complicate the code just so that gcc 2.95 stops producing some warnings. So I propose to close this as "won't fix". Is 2.95 still used much? If not, the above sounds good. |
|
|
| msg108043 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2010-06-17 18:37 |
| According to http://gcc.gnu.org/releases.html, gcc 2.95.3 is about 9 years old, so we don't need to care about warnings. |
|
|
| msg108046 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2010-06-17 18:49 |
| I agree. |
|
|