Repository navigation
Add -Wimplicit-fallthrough=0 to Makefile ? #75106
Description
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2017-08-25.12:23:13.249>
created_at = <Date 2017-07-13.16:36:00.767>
labels = ['build', '3.7']
title = 'Add -Wimplicit-fallthrough=0 to Makefile ?'
updated_at = <Date 2017-11-29.23:00:37.850>
user = 'https://github.com/matrixise'bugs.python.org fields:
activity = <Date 2017-11-29.23:00:37.850>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2017-08-25.12:23:13.249>
closer = 'skrah'
components = []
creation = <Date 2017-07-13.16:36:00.767>
creator = 'matrixise'
dependencies = []
files = ['47012']
hgrepos = []
issue_num = 30923
keywords = []
message_count = 27.0
messages = ['298299', '298300', '298302', '298306', '298307', '298312', '299063', '299070', '299072', '299085', '299086', '299089', '299090', '299091', '299092', '299093', '300483', '300490', '300526', '300527', '300585', '300620', '300622', '300833', '300835', '302003', '307269']
nosy_count = 6.0
nosy_names = ['vstinner', 'skrah', 'zach.ware', 'serhiy.storchaka', 'matrixise', 'cstratak']
pr_nums = ['2698', '3132', '3142', '3157', '3205', '3518', '4620']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue30923'
versions = ['Python 3.7']
Activity
matrixise commentedon Jul 13, 2017
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
serhiy-storchaka commentedon Jul 13, 2017
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.
skrah commentedon Jul 13, 2017
It's a useful warning, but I find it annoying to add 20 "fall through" comments. I may add a pragma at some point.
skrah commentedon Jul 13, 2017
New changeset 72b5433 by Stefan Krah in branch 'master':
bpo-30923: Suppress fall-through warnings in libmpdec. (bpo-2698)
72b5433
skrah commentedon Jul 13, 2017
Hmm, that took about 20 min to commit a 3 line diff. Now I'm watching the buildbots...
skrah commentedon Jul 13, 2017
Stéphane, if you want the libmpdec change cherry picked and are willing to do the (significant) work of backporting, I'll merge it.
matrixise commentedon Jul 25, 2017
@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 ?
vstinner commentedon Jul 25, 2017
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.
matrixise commentedon Jul 25, 2017
yep, currently, 3.6 and 3.5 are in 'bug fix' mode, and in this case, it's not a bug.
skrah commentedon Jul 25, 2017
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.
matrixise commentedon Jul 25, 2017
Stefan, ask on python-dev ml, or we have to ask to Ned Deily for this version ?
vstinner commentedon Jul 25, 2017
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.
matrixise commentedon Jul 25, 2017
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/
stratakis commentedon Jul 25, 2017
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.
1 remaining item
skrah commentedon Jul 25, 2017
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.
vstinner commentedon Aug 18, 2017
The following commit broke _decimal compilation on the "x86 Tiger 3.x" buildbot:
commit 72b5433
Author: Stefan Krah <skrah@bytereef.org>
Date: Thu Jul 13 20:54:20 2017 +0200
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"
skrah commentedon Aug 18, 2017
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.
skrah commentedon Aug 18, 2017
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.
skrah commentedon Aug 18, 2017
New changeset d73a960 by Stefan Krah in branch 'master':
bpo-30923: Disable warning that has been part of -Wextra since gcc-7.0. (bpo-3142)
d73a960
skrah commentedon Aug 19, 2017
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.
skrah commentedon Aug 21, 2017
New changeset f432a32 by Stefan Krah in branch 'master':
bpo-30923: Silence fall-through warnings included in -Wextra since gcc-7.0. (bpo-3157)
f432a32
skrah commentedon Aug 21, 2017
Cherry picking has too many conflicts, I'm not backporting this myself.
skrah commentedon Aug 25, 2017
New changeset 9e1e6f5 by Stefan Krah in branch 'master':
bpo-30923: Silence fall-through warnings in libexpat build. (bpo-3205)
9e1e6f5
skrah commentedon Aug 25, 2017
All warnings except for bpo-31275 are dealt with.
vstinner commentedon Sep 12, 2017
New changeset c0e7736 by Victor Stinner in branch '3.6':
[3.6] bpo-30923: Silence fall-through warnings included in -Wextra since gcc-7.0 (bpo-3518)
c0e7736
vstinner commentedon Nov 29, 2017
New changeset dedcbee by Victor Stinner in branch '3.6':
[3.6] bpo-30923, bpo-31279: Fix GCC warnings (bpo-4620)
dedcbee