lkml.org 
[lkml]   [2020]   [Nov]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 000/141] Fix fall-through warnings for Clang
    From
    Date

    Hi,

    On 11/20/20 12:53, Jakub Kicinski wrote:
    > On Fri, 20 Nov 2020 12:21:39 -0600 Gustavo A. R. Silva wrote:
    >> This series aims to fix almost all remaining fall-through warnings in
    >> order to enable -Wimplicit-fallthrough for Clang.
    >>
    >> In preparation to enable -Wimplicit-fallthrough for Clang, explicitly
    >> add multiple break/goto/return/fallthrough statements instead of just
    >> letting the code fall through to the next case.
    >>
    >> Notice that in order to enable -Wimplicit-fallthrough for Clang, this
    >> change[1] is meant to be reverted at some point. So, this patch helps
    >> to move in that direction.
    >>
    >> Something important to mention is that there is currently a discrepancy
    >> between GCC and Clang when dealing with switch fall-through to empty case
    >> statements or to cases that only contain a break/continue/return
    >> statement[2][3][4].
    >
    > Are we sure we want to make this change? Was it discussed before?
    >
    > Are there any bugs Clangs puritanical definition of fallthrough helped
    > find?
    >
    > IMVHO compiler warnings are supposed to warn about issues that could
    > be bugs. Falling through to default: break; can hardly be a bug?!

    The justification for this is explained in this same changelog text:

    Now that the -Wimplicit-fallthrough option has been globally enabled[5],
    any compiler should really warn on missing either a fallthrough annotation
    or any of the other case-terminating statements (break/continue/return/
    goto) when falling through to the next case statement. Making exceptions
    to this introduces variation in case handling which may continue to lead
    to bugs, misunderstandings, and a general lack of robustness. The point
    of enabling options like -Wimplicit-fallthrough is to prevent human error
    and aid developers in spotting bugs before their code is even built/
    submitted/committed, therefore eliminating classes of bugs. So, in order
    to really accomplish this, we should, and can, move in the direction of
    addressing any error-prone scenarios and get rid of the unintentional
    fallthrough bug-class in the kernel, entirely, even if there is some minor
    redundancy. Better to have explicit case-ending statements than continue to
    have exceptions where one must guess as to the right result. The compiler
    will eliminate any actual redundancy.

    Note that there is already a patch in mainline that addresses almost
    40,000 of these issues[6].

    [1] commit e2079e93f562c ("kbuild: Do not enable -Wimplicit-fallthrough for clang for now")
    [2] ClangBuiltLinux#636
    [3] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91432
    [4] https://godbolt.org/z/xgkvIh
    [5] commit a035d552a93b ("Makefile: Globally enable fall-through warning")
    [6] commit 4169e889e588 ("include: jhash/signal: Fix fall-through warnings for Clang")

    Thanks
    --
    Gustavo

    \
     
     \ /
      Last update: 2020-11-20 20:31    [W:4.080 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site