On Mon, 16 Oct 2023, 18:04 Peter Maydell, <peter.mayd...@linaro.org> wrote:
> On Mon, 16 Oct 2023 at 15:58, Manos Pitsidianakis > <manos.pitsidiana...@linaro.org> wrote: > > > > Hello Peter, > > > > On Mon, 16 Oct 2023, 17:13 Peter Maydell, <peter.mayd...@linaro.org> > wrote: > >> > >> On Fri, 13 Oct 2023 at 13:42, Markus Armbruster <arm...@redhat.com> > wrote: > >> > > >> > Emmanouil Pitsidianakis <manos.pitsidiana...@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... > As I state in the cover letter, I personally find that using one macro instead of a comment regex feels more consistent. But your view is valid as well! Let's consider the RFC retracted then. -- Manos >
