[PATCH] D53950: Enable -Wimplicit-fallthrough for clang as well as GCC

[Reid Kleckner via Phabricator via llvm-commits]

rnk created this revision.
rnk added reviewers: alexfh, rsmith, lattner, rtrieu, EricWF.
Herald added subscribers: jsji, arphaman, atanasyan, christof, haicheng, javed.absar, kbarton, aheejin, hiraditya, kristof.beyls, eraman, jgravelle-google, sbc100, mgorny, nhaehnle, jvesely, nemanjai, sdardis, dschuff, arsenm, MatzeB.
Herald added a reviewer: bollu.

Alex Kornienko proposed enabling this warning back in 2012 here:
http://lists.llvm.org/pipermail/llvm-dev/2012-July/051386.html

At the time, Chris Lattner said he didn't feel it was worth annotating
all of LLVM and Clang with a new macro to enable this warning.

However, GCC 7 added this warning as part of -Wextra, and we've slowly
annotated most of LLVM and Clang with LLVM_FALLTHROUGH.

At this point, I think we should re-evaluate this decision and enable
this warning for both clang and GCC.

Since our codebase is already annotated, it will help developers (like
me), who use Clang locally, to find unintended fallthrough bugs in their
code. For example, I committed r345676, which had to be reverted because
of an unintended fallthrough. This warning would've helped.

There is also marginal benefit to aligning warnings between GCC and
Clang. While there will always be divergence in warnings between GCC and
Clang, when possible, it saves time when clang can diagnose things that
would later become a -Werror warning on some GCC 7 buildbot.

This is a summary of differences in the behavior of
-Wimplicit-fallthrough between clang and GCC:

1. GCC recognizes comments that say "fall through" as annotations, clang doesn't
2. GCC doesn't warn on "case N: foo(); default: break;", clang does
3. GCC doesn't warn when the case contains a switch, but falls through the outer case. See the AArch64ISelLowering.cpp change for an instance where this almost caused a bug, but a redundant check saved us. I've removed the redundant check.
4. Clang warns on LLVM_FALLTHROUGH after llvm_unreachable. GCC doesn't, so I removed the one instance of this that I found.

Changing Clang's behavior in light of these differences is out of scope
for me. I want developers who compile with any of the last 4 years of
clang releases to be able to use this warning, and those releases have
the behavior described above. If you want to discuss changing Clang to
be more like GCC here, please file a bug or start a thread on cfe-dev.

I intend to split out the libc++ parts of this patch, which are just
included for reference. The rest I plan to commit all at once, since
clang, lld, llvm, and lldb all use the same warning settings and
LLVM_FALLTHROUGH annotation macro.

To conclude and restate, this warning is already enabled for GCC and
we've already annotated most of our codebase for it, so let's enable the
warning for clang.


https://reviews.llvm.org/D53950

Files:
  clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
  clang/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/Analysis/FormatString.cpp
  clang/lib/Analysis/ReachableCode.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGObjCRuntime.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/VarBypassDetector.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/lib/Index/CommentToXML.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Rewrite/HTMLRewrite.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/tools/clang-func-mapping/ClangFnMapGen.cpp
  libcxx/include/__config
  libcxxabi/src/demangle/ItaniumDemangle.h
  lld/lib/ReaderWriter/MachO/ArchHandler_x86_64.cpp
  lld/lib/ReaderWriter/MachO/MachOLinkingContext.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/include/llvm/Demangle/ItaniumDemangle.h
  llvm/lib/Analysis/InlineCost.cpp
  llvm/lib/DebugInfo/CodeView/SymbolRecordMapping.cpp
  llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
  llvm/lib/Target/AMDGPU/R600MachineScheduler.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/BPF/AsmParser/BPFAsmParser.cpp
  llvm/lib/Target/Hexagon/HexagonConstExtenders.cpp
  llvm/lib/Target/Hexagon/HexagonConstPropagation.cpp
  llvm/lib/Target/Hexagon/HexagonISelDAGToDAG.cpp
  llvm/lib/Target/Hexagon/HexagonMachineScheduler.cpp
  llvm/lib/Target/Hexagon/HexagonVLIWPacketizer.cpp
  llvm/lib/Target/Hexagon/MCTargetDesc/HexagonMCDuplexInfo.cpp
  llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
  llvm/lib/Target/Mips/MipsAsmPrinter.cpp
  llvm/lib/Target/Mips/MipsSEISelDAGToDAG.cpp
  llvm/lib/Target/PowerPC/PPCFastISel.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/SystemZ/SystemZISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
  llvm/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  polly/lib/Analysis/ScopBuilder.cpp
  polly/lib/Analysis/ScopDetection.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53950.172020.patch
Type: text/x-patch
Size: 41574 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181031/62b05ba2/attachment.bin>