[llvm-dev] RFC Enable -Wimplicit-fallthrough for clang as well as GCC

[Matthias Braun via llvm-dev]

+1! Accidental fallthrough is a common bug class in my experience and this a great tool to avoid them.

In reality you already get mail from some buildbots anyway when you forget LLVM_FALLTHROUGH AFAIK so this just codifies existing practice.

> On Oct 31, 2018, at 3:25 PM, Justin Bogner via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> +1. I tried to do this a couple of years ago (not knowing about the
> proposal in 2012) but there was too much to annotate at the time. It
> seems like this is easy to do now.
> 
> Reid Kleckner via llvm-dev <llvm-dev at lists.llvm.org> writes:
>> 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 posted a patch with the this RFC as the commit message here so you can
>> see what this looks like now:
>> https://reviews.llvm.org/D53950
>> 
>> To summarize, 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.
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev