linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Joe Perches <joe@perches.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Kees Cook <keescook@chromium.org>, Borislav Petkov <bp@alien8.de>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>, Pavel Machek <pavel@ucw.cz>,
	"Gustavo A . R . Silva" <gustavo@embeddedor.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Kan Liang <kan.liang@linux.intel.com>,
	Namhyung Kim <namhyung@kernel.org>, Jiri Olsa <jolsa@redhat.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Shawn Landden <shawn@git.icu>,
	"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<x86@kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>,
	Nathan Chancellor <natechancellor@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Miller <davem@davemloft.net>,
	clang-built-linux <clang-built-linux@googlegroups.com>
Subject: Re: [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use
Date: Mon, 7 Oct 2019 11:14:31 -0700	[thread overview]
Message-ID: <CAKwvOdkVZ64sLppKxF1XRgarPmCbhw1WLsSq1VcV1tagPgWtUg@mail.gmail.com> (raw)
In-Reply-To: <CANiq72kU2_s=58HqdN6VMGDAh_+G+dtns9xzoc4huSVwP+ZXUg@mail.gmail.com>

On Sat, Oct 5, 2019 at 10:17 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Hi Joe,
>
> On Sat, Oct 5, 2019 at 6:46 PM Joe Perches <joe@perches.com> wrote:
> >
> > Reserve the pseudo keyword 'fallthrough' for the ability to convert the

Have we precedent already for "pseudo keywords?"  I kind of like the
double underscore prefix we use for attributes (which this is one of),
or at least making macro's ALLCAPS as some sort of convention.
Otherwise, someone might be confused on seeing `fallthrough` sprinkled
throughout the code without knowing how it works. `__fallthough` or
`FALLTHROUGH` are maybe less surprising (and potentially easier to
grep)?  Sorry if this has already been discussed; from Miguel's link
below it looks like there used to be underscore prefixes before?

> > various case block /* fallthrough */ style comments to appear to be an
> > actual reserved word with the same gcc case block missing fallthrough
> > warning capability.
> >
> > All switch/case blocks now must end in one of:
> >
> >         break;
> >         fallthrough;
> >         goto <label>;
> >         return [expression];
> >         continue;
> >
> > fallthough is gcc's __attribute__((__fallthrough__)) which was introduced
> > in gcc version 7..
>
> Nits: double period, missing "r" in fallthough.
>
> > fallthrough devolves to an empty "do {} while (0)" if the compiler
> > version (any version less than gcc 7) does not support the attribute.
>
> Perhaps add a short note why (empty statement warnings maybe? I don't
> remember them but it was months ago so maybe it changed).
>
> > Signed-off-by: Joe Perches <joe@perches.com>
> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> Please add Dan's Suggested-by and copy the things I wrote in the
> commit message when I proposed this:
>
>   https://github.com/ojeda/linux/commit/668f011a2706ea555987e263f609a5deba9c7fc4
>
> > ---
> >  include/linux/compiler_attributes.h | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> > index 6b318efd8a74..cdf016596659 100644
> > --- a/include/linux/compiler_attributes.h
> > +++ b/include/linux/compiler_attributes.h
> > @@ -40,6 +40,7 @@
> >  # define __GCC4_has_attribute___noclone__             1
> >  # define __GCC4_has_attribute___nonstring__           0
> >  # define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
> > +# define __GCC4_has_attribute___fallthrough__         0
>
> This goes after __externally_visible__.
>
> >  #endif
> >
> >  /*
> > @@ -185,6 +186,22 @@
> >  # define __noclone
> >  #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
>
> This also goes after __externally_visible__.
>
> Please add:
>
>   * Optional: only supported since gcc >= 7.1
>   * Optional: only supported since clang >= 10
>   * Optional: not supported by icc
>
> As well as:
>
>   clang: https://clang.llvm.org/docs/AttributeReference.html#fallthrough
>
> See how I did it in the link above:
>
>   https://github.com/ojeda/linux/commit/668f011a2706ea555987e263f609a5deba9c7fc4
>
> > + */
> > +#if __has_attribute(__fallthrough__)
> > +# define fallthrough                    __attribute__((__fallthrough__))
> > +#else
> > +# define fallthrough                    do {} while (0)  /* fallthrough */
> > +#endif
> > +
> >  /*
> >   * Note the missing underscores.
> >   *
> > --
> > 2.15.0
> >
>
> Cheers,
> Miguel



-- 
Thanks,
~Nick Desaulniers

  reply	other threads:[~2019-10-07 18:14 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-05 16:46 [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Joe Perches
2019-10-05 16:46 ` [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled Joe Perches
2019-10-07 18:08   ` Nick Desaulniers
2019-10-10 20:34   ` Kees Cook
2019-10-11 12:20   ` Neil Horman
2019-10-05 16:46 ` [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use Joe Perches
2019-10-05 17:17   ` Miguel Ojeda
2019-10-07 18:14     ` Nick Desaulniers [this message]
2019-10-07 18:28       ` Joe Perches
2019-10-10 20:37     ` Kees Cook
2019-10-11 22:25       ` Miguel Ojeda
2019-10-05 16:46 ` [PATCH 3/4] Documentation/process: Add fallthrough pseudo-keyword Joe Perches
2019-10-05 17:47   ` Miguel Ojeda
2019-10-09 21:51     ` Nick Desaulniers
2019-10-05 16:46 ` [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways Joe Perches
2019-10-05 17:31   ` Miguel Ojeda
2019-10-06  6:35     ` Joe Perches
2019-10-10 20:39       ` Kees Cook
2019-10-10 20:48         ` Joe Perches
2019-10-09 22:37   ` Nick Desaulniers
2019-10-11 16:29 ` [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Linus Torvalds
2019-10-11 17:43   ` Joe Perches
2019-10-11 17:46     ` Linus Torvalds
2019-10-12  2:14       ` Joe Perches
2019-10-11 18:01   ` Miguel Ojeda
2019-10-11 22:07     ` Kees Cook
2019-10-11 22:26       ` Miguel Ojeda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKwvOdkVZ64sLppKxF1XRgarPmCbhw1WLsSq1VcV1tagPgWtUg@mail.gmail.com \
    --to=ndesaulniers@google.com \
    --cc=acme@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=clang-built-linux@googlegroups.com \
    --cc=davem@davemloft.net \
    --cc=gustavo@embeddedor.com \
    --cc=hpa@zytor.com \
    --cc=joe@perches.com \
    --cc=jolsa@redhat.com \
    --cc=kan.liang@linux.intel.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=namhyung@kernel.org \
    --cc=natechancellor@gmail.com \
    --cc=pavel@ucw.cz \
    --cc=shawn@git.icu \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH 2/4] compiler_attributes.h: Add '\''fallthrough'\'' pseudo keyword for switch/case use' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).