msg298299 - (view) |
Author: Stéphane Wirtel (matrixise) *  |
Date: 2017-07-13 16:35 |
Hi all, Since I use the last version of Fedora 26 with gcc-7.1.1, I have these warnings (see output.txt file) We could add -Wimplicit-fallthrough=0 to Makefile ? it will disable the fallthrough of the coed. What do you think about that ? What's your feedback on this option and can we use it in the case of CPython ? Thank you |
|
|
msg298300 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-07-13 17:15 |
It seems to me that by default the compiler recognizes a wide variety of "falls through" comments. Thus we need just add missed comments. There are warnings emitted when compile imported third-party code. We should ask Stefan for adding "falls through" comments in his libmpdec. And either compile expat with -Wimplicit-fallthrough=0, or wait until warnings will be fixed in upstream, or remove the bundled copy of expat. |
|
|
msg298302 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-07-13 17:52 |
It's a useful warning, but I find it annoying to add 20 "fall through" comments. I may add a pragma at some point. |
|
|
msg298306 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-07-13 18:54 |
New changeset 72b543308ee3087e3fa247981f5cb4be1138c515 by Stefan Krah in branch 'master': bpo-30923: Suppress fall-through warnings in libmpdec. (#2698) https://github.com/python/cpython/commit/72b543308ee3087e3fa247981f5cb4be1138c515 |
|
|
msg298307 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-07-13 18:58 |
Hmm, that took about 20 min to commit a 3 line diff. Now I'm watching the buildbots... |
|
|
msg298312 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-07-13 20:16 |
Stéphane, if you want the libmpdec change cherry picked and are willing to do the (significant) work of backporting, I'll merge it. |
|
|
msg299063 - (view) |
Author: Stéphane Wirtel (matrixise) *  |
Date: 2017-07-25 12:27 |
@stefan In fact I could create the PR for the backports, but I have again the same issues for the rest of CPython. I have checked your code, and in my case 'gcc' is in the cc string, 'gcc -pthread' and you have fixed for libmpdec. But for the other modules ? |
|
|
msg299070 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-07-25 12:51 |
Hum, I don't think that it's worth it to backport changes which only fix warnings. Usually, we first focus on fixing all warnings on master. |
|
|
msg299072 - (view) |
Author: Stéphane Wirtel (matrixise) *  |
Date: 2017-07-25 12:58 |
yep, currently, 3.6 and 3.5 are in 'bug fix' mode, and in this case, it's not a bug. |
|
|
msg299085 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-07-25 13:50 |
Well, it's not a bug, but perhaps it is annoying for users of this gcc version if they compile 3.6 often. Actually, I think gcc should not include this warning in -Wextra. It's something that could be run manually before a release. I'd vote for making -Wno-implicit-fallthrough global. |
|
|
msg299086 - (view) |
Author: Stéphane Wirtel (matrixise) *  |
Date: 2017-07-25 14:04 |
Stefan, ask on python-dev ml, or we have to ask to Ned Deily for this version ? |
|
|
msg299089 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-07-25 14:17 |
I'm ok to kill warnings, but I would suggest to first fix most GCC7 warnings because starting to discuss backports. I expect that it will require multiple changes. |
|
|
msg299090 - (view) |
Author: Stéphane Wirtel (matrixise) *  |
Date: 2017-07-25 14:22 |
I am not for a backport, it's not a security fix or a bug fix. for my case, I just want to "kill" the warnings, maybe we could check the version of gcc and add "-Wimplicit-fallthrough=0". Here is a good explanation of the 'fallthrough' in gcc7 https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/ |
|
|
msg299091 - (view) |
Author: Charalampos Stratakis (cstratak) * |
Date: 2017-07-25 14:32 |
Yeah the warnings are quit annoying when compiling the master and 3.6 branch. Fedora 26 currently includes gcc 7 which emits those warnings, other distros will follow up when they update gcc, so I'd think it would be better to fix it. |
|
|
msg299092 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-07-25 14:51 |
We can check for the version, but all versions of gcc that I tested accept and ignore -Wno-implicit-fallthrough, even though they don't actually have -Wimplicit-fallthrough. Of course they choke on -Wimplicit-fallthrough=0. |
|
|
msg299093 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-07-25 14:54 |
I think the fall-through blog notes are slightly overstated. :-) "The switch fallthrough has been widely considered a design defect in C." It's an important feature. |
|
|
msg300483 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-08-18 10:35 |
The following commit broke _decimal compilation on the "x86 Tiger 3.x" buildbot: commit 72b543308ee3087e3fa247981f5cb4be1138c515 Author: Stefan Krah <skrah@bytereef.org> Date: Thu Jul 13 20:54:20 2017 +0200 bpo-30923: Suppress fall-through warnings in libmpdec. (#2698) http://buildbot.python.org/all/builders/x86%20Tiger%203.x/builds/1068/steps/compile/logs/stdio building '_decimal' extension gcc -fno-strict-aliasing -Wsign-compare -g -O0 -Wall -Wstrict-prototypes -std=c99 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -DUNIVERSAL=1 -I/Users/db3l/buildarea/3.x.bolen-tiger/build/Modules/_decimal/libmpdec -I./Include -I. -I/usr/local/include -I/Users/db3l/buildarea/3.x.bolen-tiger/build/Include -I/Users/db3l/buildarea/3.x.bolen-tiger/build -c /Users/db3l/buildarea/3.x.bolen-tiger/build/Modules/_decimal/_decimal.c -o build/temp.macosx-10.4-i386-3.7-pydebug/Users/db3l/buildarea/3.x.bolen-tiger/build/Modules/_decimal/_decimal.o -Wno-implicit-fallthrough cc1: error: unrecognized command line option "-Wno-implicit-fallthrough" |
|
|
msg300490 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-08-18 11:24 |
Thanks. I tried to revert it, but got: remote: Resolving deltas: 100% (2/2), completed with 2 local objects. remote: error: GH006: Protected branch update failed for refs/heads/master. remote: error: 2 of 2 required status checks are expected. To https://github.com/python/cpython.git ! [remote rejected] master -> master (protected branch hook declined) error: failed to push some refs to 'https://github.com/python/cpython.git' So I'm supposed to spend 20 min creating an irrelevant PR to revert a 3 line diff?! I would never have finished _decimal under these conditions. |
|
|
msg300526 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-08-18 18:44 |
So I installed gcc-7.2.0 from source. Hilariously compiling gcc *itself* emits fallthrough warnings! Then I tried to be a good open source drone and add 20 /* fall through */ comments to libmpdec. gcc is too stupid to recognize the /* fall through */ at the #endif position. Perhaps the author of this gcc warning wants to submit a patch. |
|
|
msg300527 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-08-18 19:39 |
New changeset d73a960c575207539c3f9765cff26d4fff400b45 by Stefan Krah in branch 'master': bpo-30923: Disable warning that has been part of -Wextra since gcc-7.0. (#3142) https://github.com/python/cpython/commit/d73a960c575207539c3f9765cff26d4fff400b45 |
|
|
msg300585 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-08-19 15:50 |
PR 3157 addresses everything apart from expat and https://github.com/python/cpython/blob/master/Modules/cjkcodecs/_codecs_iso2022.c#L816 I'm not sure about that one. It looks harmless but a bit odd. |
|
|
msg300620 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-08-21 11:10 |
New changeset f432a3234f9f2ee09bd40be03e06bf72865ee375 by Stefan Krah in branch 'master': bpo-30923: Silence fall-through warnings included in -Wextra since gcc-7.0. (#3157) https://github.com/python/cpython/commit/f432a3234f9f2ee09bd40be03e06bf72865ee375 |
|
|
msg300622 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-08-21 11:24 |
Cherry picking has too many conflicts, I'm not backporting this myself. |
|
|
msg300833 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-08-25 12:07 |
New changeset 9e1e6f528f3fec16b9bd99f5ee38048ffec04a81 by Stefan Krah in branch 'master': bpo-30923: Silence fall-through warnings in libexpat build. (#3205) https://github.com/python/cpython/commit/9e1e6f528f3fec16b9bd99f5ee38048ffec04a81 |
|
|
msg300835 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2017-08-25 12:23 |
All warnings except for #31275 are dealt with. |
|
|
msg302003 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-09-12 23:09 |
New changeset c0e77364ca29df6cfb311e79892955c92bd8e595 by Victor Stinner in branch '3.6': [3.6] bpo-30923: Silence fall-through warnings included in -Wextra since gcc-7.0 (#3518) https://github.com/python/cpython/commit/c0e77364ca29df6cfb311e79892955c92bd8e595 |
|
|
msg307269 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-11-29 23:00 |
New changeset dedcbee04cd52790027ecfb46cb3aa33efebdc84 by Victor Stinner in branch '3.6': [3.6] bpo-30923, bpo-31279: Fix GCC warnings (#4620) https://github.com/python/cpython/commit/dedcbee04cd52790027ecfb46cb3aa33efebdc84 |
|
|