[PATCH] D91895: [Clang] improve -Wimplicit-fallthrough GCC compat

2020-11-25 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In D91895#2417170 , @kees wrote: > I think this should warn too. While this won't turn into a "missing break" > error, there's no way to know (from looking at code) what the _intent_ is > here. Hear, hear. +1 on everything

[PATCH] D91895: [Clang] improve -Wimplicit-fallthrough GCC compat

2020-11-25 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. The kernel's [[ https://www.kernel.org/doc/html/latest/process/deprecated.html#implicit-switch-case-fall-through | stance ]] on switch statements reads: | | All switch/case blocks must end in one of: break; fallthrough; continue; goto ; return

[PATCH] D91895: [Clang] improve -Wimplicit-fallthrough GCC compat

2020-11-23 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. I'd suggest instead of removing these warnings, we split the warning flag in two: 1. A warning flag that warns that there is implicit fallthrough that's not equivalent to `break;` (which sounds like it's what the kernel folks want in the 99% case) -- that is, warn if

[PATCH] D91895: [Clang] improve -Wimplicit-fallthrough GCC compat

2020-11-23 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. I'm happy to modify the patch based on feedback from LKML, though switch (x) { case 0: ++x; default: break; } is the important case that's 99% of the differences between GCC and Clang, which should not get removed from this patch.

[PATCH] D91895: [Clang] improve -Wimplicit-fallthrough GCC compat

2020-11-23 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1102 + return llvm::any_of(B->succs(), + [this](const CFGBlock::AdjacentBlock ) { +if (const CFGBlock *S =

[PATCH] D91895: [Clang] improve -Wimplicit-fallthrough GCC compat

2020-11-23 Thread Nathan Chancellor via Phabricator via cfe-commits
nathanchance added a comment. I think that not warning upon falling through to `goto` or `return` is a mistake. We have a confirmed case of a bug where `clang` would catch this bug whereas `gcc` would not: https://lore.kernel.org/lkml/20201121124019.21116-1-ogab...@kernel.org/. I suspect that