Issue 30923: Add -Wimplicit-fallthrough=0 to Makefile ? (original) (raw)

Created on 2017-07-13 16:36 by matrixise, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
output.txt matrixise,2017-07-13 16:35
Pull Requests
URL Status Linked Edit
PR 2698 merged skrah,2017-07-13 18:42
PR 3132 merged skrah,2017-08-18 11:49
PR 3142 merged skrah,2017-08-18 19:24
PR 3157 merged skrah,2017-08-19 15:39
PR 3205 merged skrah,2017-08-25 11:50
PR 3518 merged vstinner,2017-09-12 17:39
PR 4620 merged vstinner,2017-11-28 23:03
Messages (27)
msg298299 - (view) Author: Stéphane Wirtel (matrixise) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2017-08-21 11:24
Cherry picking has too many conflicts, I'm not backporting this myself.
msg300833 - (view) Author: Stefan Krah (skrah) * (Python committer) 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) * (Python committer) Date: 2017-08-25 12:23
All warnings except for #31275 are dealt with.
msg302003 - (view) Author: STINNER Victor (vstinner) * (Python committer) 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) * (Python committer) 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
History
Date User Action Args
2022-04-11 14:58:49 admin set github: 75106
2017-11-29 23:00:37 vstinner set messages: +
2017-11-28 23:03:01 vstinner set pull_requests: + <pull%5Frequest4534>
2017-09-12 23:09:46 vstinner set messages: +
2017-09-12 17:39:15 vstinner set pull_requests: + <pull%5Frequest3513>
2017-08-25 12:23:13 skrah set status: open -> closedtype: compile errormessages: + resolution: fixedstage: resolved
2017-08-25 12:07:52 skrah set messages: +
2017-08-25 11:50:26 skrah set pull_requests: + <pull%5Frequest3242>
2017-08-21 11:24:20 skrah set messages: +
2017-08-21 11:10:02 skrah set messages: +
2017-08-19 15:50:27 skrah set messages: +
2017-08-19 15:39:06 skrah set pull_requests: + <pull%5Frequest3194>
2017-08-18 19:39:35 skrah set messages: +
2017-08-18 19:24:52 skrah set pull_requests: + <pull%5Frequest3177>
2017-08-18 18:44:44 skrah set messages: +
2017-08-18 11:49:41 skrah set pull_requests: + <pull%5Frequest3166>
2017-08-18 11:24:13 skrah set messages: +
2017-08-18 10:35:14 vstinner set messages: +
2017-07-25 14:54:36 skrah set messages: +
2017-07-25 14:51:12 skrah set messages: +
2017-07-25 14:32:18 cstratak set messages: +
2017-07-25 14:22:49 matrixise set messages: +
2017-07-25 14:17:24 vstinner set messages: +
2017-07-25 14:04:29 matrixise set messages: +
2017-07-25 13:50:12 skrah set messages: +
2017-07-25 12:58:04 matrixise set messages: +
2017-07-25 12:51:20 vstinner set nosy: + vstinnermessages: +
2017-07-25 12:27:20 matrixise set messages: +
2017-07-24 17:15:13 cstratak set nosy: + cstratak
2017-07-24 17:10:14 zach.ware link issue31017 superseder
2017-07-13 20:16:06 skrah set messages: +
2017-07-13 18:58:49 skrah set messages: +
2017-07-13 18:54:22 skrah set messages: +
2017-07-13 18:42:43 skrah set pull_requests: + <pull%5Frequest2764>
2017-07-13 17:58:54 zach.ware set nosy: + zach.ware
2017-07-13 17:52:15 skrah set messages: +
2017-07-13 17:15:51 serhiy.storchaka set nosy: + skrah, serhiy.storchakamessages: +
2017-07-13 16:36:00 matrixise create