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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 8 Jan 2021, 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))
> 
> 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.
> 
> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
> ---
> Changes in V4:
>  - This patch is introduce in this verison.
> ---
>  xen/include/xen/compiler.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
> index e643e69128..0ec0b4698e 100644
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -33,6 +33,22 @@
>  #define unreachable() __builtin_unreachable()
>  #endif
>  
> +/*
> + * Add the pseudo keyword 'fallthrough' so case statement blocks
> + * must end with any of these keywords:
> + *   break;
> + *   fallthrough;
> + *   goto <label>;
> + *   return [expression];
> + *
> + *  gcc: 
> https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
> + */
> +#if (!defined(__clang__) && (__GNUC__ >= 7))
> +# define fallthrough        __attribute__((__fallthrough__))
> +#else
> +# define fallthrough        do {} while (0)  /* fallthrough */
> +#endif
> +
>  #ifdef __clang__
>  /* Clang can replace some vars with new automatic ones that go in .data;
>   * mark all explicit-segment vars 'used' to prevent that. */

It would be nicer to use __has_attribute to check if fallthrough is
supported by the compiler, but I wouldn't want to have to implement
__has_attribute by hand for the old compilers that don't have it. If we
are in doubt whether the compiler has has_attribute or not, then I think
it is better to do what you did here and avoid the problem altogether.


Linux states:

 * __has_attribute is supported on gcc >= 5, clang >= 2.9 and icc >= 17.


Unfortunately gcc 4.9 is old but still around. I don't think we made any
statements in Xen about gcc support >= 5. Hence, I think your patch is
good as it is.


Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>