Repository navigation
Disable -Wimplicit-fallthrough warning for Modules/expat #125506
Description
Bug report
Right now compilers produce a lot of warnings for vendored modules:
But, there's not much we can do about them, since they are vendored. For example, siphash has explicit fall-through comments:
cpython/Modules/expat/siphash.h
Lines 235 to 243 in 92af191
But, it is still reported:
In file included from ./Modules/expat/xmlparse.c:121:
./Modules/expat/siphash.h:238:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
case 6:
^
./Modules/expat/siphash.h:238:3: note: insert '__attribute__((fallthrough));' to silence this warning
case 6:
^
__attribute__((fallthrough));
./Modules/expat/siphash.h:238:3: note: insert 'break;' to avoid fall-through
case 6:
^
break;
./Modules/expat/siphash.h:241:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
case 5:
^
./Modules/expat/siphash.h:241:3: note: insert '__attribute__((fallthrough));' to silence this warning
case 5:
^
__attribute__((fallthrough));
./Modules/expat/siphash.h:241:3: note: insert 'break;' to avoid fall-through
case 5:
^
break;
./Modules/expat/siphash.h:244:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
case 4:
^
./Modules/expat/siphash.h:244:3: note: insert '__attribute__((fallthrough));' to silence this warning
case 4:
^
__attribute__((fallthrough));
./Modules/expat/siphash.h:244:3: note: insert 'break;' to avoid fall-through
case 4:
^
break;
./Modules/expat/siphash.h:247:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
case 3:
^
./Modules/expat/siphash.h:247:3: note: insert '__attribute__((fallthrough));' to silence this warning
case 3:
^
__attribute__((fallthrough));
./Modules/expat/siphash.h:247:3: note: insert 'break;' to avoid fall-through
case 3:
^
break;
./Modules/expat/siphash.h:250:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
case 2:
^
./Modules/expat/siphash.h:250:3: note: insert '__attribute__((fallthrough));' to silence this warning
case 2:
^
__attribute__((fallthrough));
./Modules/expat/siphash.h:250:3: note: insert 'break;' to avoid fall-through
case 2:
^
break;
./Modules/expat/siphash.h:253:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
case 1:
^
./Modules/expat/siphash.h:253:3: note: insert '__attribute__((fallthrough));' to silence this warning
case 1:
^
__attribute__((fallthrough));
./Modules/expat/siphash.h:253:3: note: insert 'break;' to avoid fall-through
case 1:
^
break;
./Modules/expat/siphash.h:256:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
case 0:
^
./Modules/expat/siphash.h:256:3: note: insert 'break;' to avoid fall-through
case 0:
^
break;
./Modules/expat/xmlparse.c:1944:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
default:
^
./Modules/expat/xmlparse.c:1944:3: note: insert '__attribute__((fallthrough));' to silence this warning
default:
^
__attribute__((fallthrough));
./Modules/expat/xmlparse.c:1944:3: note: insert 'break;' to avoid fall-through
default:
^
break;
./Modules/expat/xmlparse.c:2067:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
default:
^
./Modules/expat/xmlparse.c:2067:3: note: insert '__attribute__((fallthrough));' to silence this warning
default:
^
__attribute__((fallthrough));
./Modules/expat/xmlparse.c:2067:3: note: insert 'break;' to avoid fall-through
default:
^
break;
./Modules/expat/xmlparse.c:2096:5: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
default:; /* should not happen */
^
./Modules/expat/xmlparse.c:2096:5: note: insert '__attribute__((fallthrough));' to silence this warning
default:; /* should not happen */
^
__attribute__((fallthrough));
./Modules/expat/xmlparse.c:2096:5: note: insert 'break;' to avoid fall-through
default:; /* should not happen */
^
break;
./Modules/expat/xmlparse.c:2292:5: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
default:;
^
./Modules/expat/xmlparse.c:2292:5: note: insert '__attribute__((fallthrough));' to silence this warning
default:;
^
__attribute__((fallthrough));
./Modules/expat/xmlparse.c:2292:5: note: insert 'break;' to avoid fall-through
default:;
^
break;
./Modules/expat/xmlparse.c:4864:5: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
case XML_ROLE_ENTITY_PUBLIC_ID:
^
./Modules/expat/xmlparse.c:4864:5: note: insert '__attribute__((fallthrough));' to silence this warning
case XML_ROLE_ENTITY_PUBLIC_ID:
^
__attribute__((fallthrough));
./Modules/expat/xmlparse.c:4864:5: note: insert 'break;' to avoid fall-through
case XML_ROLE_ENTITY_PUBLIC_ID:
^
break;
./Modules/expat/xmlparse.c:5192:5: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
case XML_ROLE_ENTITY_SYSTEM_ID:
^
./Modules/expat/xmlparse.c:5192:5: note: insert '__attribute__((fallthrough));' to silence this warning
case XML_ROLE_ENTITY_SYSTEM_ID:
^
__attribute__((fallthrough));
./Modules/expat/xmlparse.c:5192:5: note: insert 'break;' to avoid fall-through
case XML_ROLE_ENTITY_SYSTEM_ID:
^
break;
./Modules/expat/xmlparse.c:6050:5: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
case XML_TOK_ATTRIBUTE_VALUE_S:
^
./Modules/expat/xmlparse.c:6050:5: note: insert '__attribute__((fallthrough));' to silence this warning
case XML_TOK_ATTRIBUTE_VALUE_S:
^
__attribute__((fallthrough));
./Modules/expat/xmlparse.c:6050:5: note: insert 'break;' to avoid fall-through
case XML_TOK_ATTRIBUTE_VALUE_S:
^
break;
./Modules/expat/xmlparse.c:6303:5: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
case XML_TOK_DATA_NEWLINE:
^
./Modules/expat/xmlparse.c:6303:5: note: insert '__attribute__((fallthrough));' to silence this warning
case XML_TOK_DATA_NEWLINE:
^
__attribute__((fallthrough));
./Modules/expat/xmlparse.c:6303:5: note: insert 'break;' to avoid fall-through
case XML_TOK_DATA_NEWLINE:
^
break;
My opinion is that we only should show actionable warnings, that we can actually fix.
CC @hugovk
Activity
hugovk commentedon Oct 15, 2024
cc @nohlson @sethmlarson
rruuaanng commentedon Oct 15, 2024
In fact, We can add
-Wno-implicit-fallthroughto suppress it, Because this warning only means that thecasestatement has nobreak.(Although this will result in no warning being triggered if the user writes a statement without a break).
sobolevn commentedon Oct 15, 2024
@rruuaanng
I think that we should not add anything here, because we are working with a vendored module that cannot be modified (without a very strong reason to).
Again, developers should not touch vendored libs without a very strong reason. Fixing such a warning is not good enough reason to do so.
sethmlarson commentedon Oct 15, 2024
I'm in agreement that we shouldn't be warning on code we don't control. Our current setup for warning tooling applies the warning to all code covered by
CFLAGS_NODIST, so even if the warnings don't cause CI to fail they still show up in logs. I don't know if there's an easy way to ignore whole directories that we vendor at theconfigurelevel to not apply warnings to those files?erlend-aasland commentedon Oct 15, 2024
We don't have such functionality in
configure.ac, but you should be fine with adding-Wno-implicit-fallthroughafter-Wimplicit-fallthrough. I don't remember from the top of my head ifLIBEXPAT_CFLAGSare added before or afterCFLAGS_NODIST, but ifLIBEXPAT_CFLAGSare added postCFLAGS_NODIST, getting rid of those warnings for libexpat should be pretty trivial1.PoC hack
Footnotes
a trivial, but dirty, hack in
Makefile.pre.in, that is ↩hartwork commentedon Jun 25, 2026
@StanFromIreland I believe that fixing #152216 — updating the bundled Expat to 2.8.2 — fixes this^^ "for free" as a side effect because of the changes that @neek78 contributed in libexpat/libexpat#1224 .
StanFromIreland commentedon Jun 25, 2026
Yes, thanks for the ping! I removed these from the ignore file in the PR, I'm happy to report it's now empty :-)