Re: [PATCH v4 08/11] xen/compiler: import 'fallthrough' keyword from linux

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 12 Jan 2021, Stefano Stabellini wrote:
> On Tue, 12 Jan 2021, Jan Beulich wrote:
> > On 08.01.2021 15:46, Rahul Singh wrote:
> > > -Wimplicit-fallthrough warns when a switch case falls through. Warning
> > > can be suppress by either adding a /* fallthrough */ comment, or by
> > > using a null statement: __attribute__ ((fallthrough))
> > 
> > Why is the comment variant (which we use in many places already,
> > albeit with varying wording) not the route of choice?
> 
> See previous discussion:
> 
> https://marc.info/?l=xen-devel&m=160707274517270
> https://marc.info/?l=xen-devel&m=160733742810605
> https://marc.info/?l=xen-devel&m=160733852011023
> 
> We thought it would be best to introduce "fallthrough" and only resort
> to comments as a plan B. The usage of the keyword should allow GCC to do
> better checks.
> 
> 
> > > Define the pseudo keyword 'fallthrough' for the ability to convert the
> > > various case block /* fallthrough */ style comments to null statement
> > > "__attribute__((__fallthrough__))"
> > > 
> > > In C mode, GCC supports the __fallthrough__ attribute since 7.1,
> > > the same time the warning and the comment parsing were introduced.
> > > 
> > > fallthrough devolves to an empty "do {} while (0)" if the compiler
> > > version (any version less than gcc 7) does not support the attribute.
> > 
> > What about Coverity? It would be nice if we wouldn't need to add
> > two separate constructs everywhere to make both compiler and static
> > code checker happy.
> 
> I don't think I fully understand your reply here: Coverity doesn't come
> into the picture. Given that GCC provides a special keyword to implement
> fallthrough, it makes sense to use it when available. When it is not
> available (e.g. clang or older GCC) we need to have an alternative to
> suppress the compiler warnings. Hence the need for this check:
> 
>   #if (!defined(__clang__) && (__GNUC__ >= 7))

I forgot to mention that it matches the implementation in Linux, see
include/linux/compiler_attributes.h and 294f69e662d15.