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

[Aaron Ballman via llvm-dev]

On Wed, Oct 31, 2018 at 5:24 PM Reid Kleckner via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> 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.

+1, I think this is worth doing.

~Aaron

> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev