[llvm-dev] RFC Enable -Wimplicit-fallthrough for clang as well as GCC
Alexander Kornienko via llvm-dev
llvm-dev at lists.llvm.org
Wed Oct 31 17:11:40 PDT 2018
+1 obviously ;) Thanks for picking this up!
On Wed, Oct 31, 2018 at 10:24 PM Reid Kleckner <rnk at google.com> 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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181101/9fecd0a2/attachment.html>
More information about the llvm-dev
mailing list