Re: [RFC PATCH 00/78] Strict disable implicit fallthrough

[Peter Maydell]

On Mon, 16 Oct 2023 at 15:58, Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> Hello Peter,
>
> On Mon, 16 Oct 2023, 17:13 Peter Maydell, <peter.maydell@linaro.org> wrote:
>>
>> On Fri, 13 Oct 2023 at 13:42, Markus Armbruster <armbru@redhat.com> wrote:
>> >
>> > Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
>> >
>> > > Hello,
>> > >
>> > > This RFC is inspired by the kernel's move to -Wimplicit-fallthrough=3
>> > > back in 2019.[0]
>> > > We take one step (or two) further by increasing it to 5 which rejects
>> > > fall through comments and requires an attribute statement.
>> > >
>> > > [0]:
>> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a035d552a93b
>> > >
>> > > The line differences are not many, but they spread all over different
>> > > subsystems, architectures and devices. An attempt has been made to split
>> > > them in cohesive patches to aid post-RFC review. Part of the RFC is to
>> > > determine whether these patch divisions needs improvement.
>> > >
>> > > Main questions this RFC poses
>> > > =============================
>> > >
>> > > - Is this change desirable and net-positive.
>> >
>> > Unwanted fallthrough is an easy mistake to make, and
>> > -Wimplicit-fallthrough=N helps avoid it.  The question is how far up we
>> > need to push N.  Right now we're at N=2.  Has unwanted fallthrough been
>> > a problem?
>>
>> Mmm, this is my opinion I think. We have a mechanism for
>> catching "forgot the 'break'" already (our =2 setting) and
>> a way to say "intentional" in a fairly natural way (add the
>> comment). Does pushing N up any further gain us anything
>> except a load of churn?
>>
>> Also, the compiler is not the only thing that processes our
>> code: Coverity also looks for "unexpected fallthrough" issues,
>> so if we wanted to switch away from our current practice we
>> should check whether what we're switching to is an idiom
>> that Coverity recognises.
>
>
> It is a code style change as the cover letter mentions, it's not related to 
> the static analysis itself.

Yes, exactly. As a code style change it needs a fairly high level
of justification for the code churn, and the cover letter
doesn't really provide one...

thanks
-- PMM