| Message ID | 20190815225844.145726-1-nhuck@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] kbuild: Require W=1 for -Wimplicit-fallthrough with clang | expand |
On Thu, Aug 15, 2019 at 3:59 PM 'Nathan Huckleberry' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote: > > Clang is updating to support -Wimplicit-fallthrough on C > https://reviews.llvm.org/D64838. Since clang does not > support the comment version of fallthrough annotations > this update causes an additional 50k warnings. Most > of these warnings (>49k) are duplicates from header files. > > This patch is intended to be reverted after the warnings > have been cleaned up. > > Signed-off-by: Nathan Huckleberry <nhuck@google.com> > Suggested-by: Nathan Chancellor <natechancellor@gmail.com> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > --- > Changes v1->v2 > * Move code to preexisting ifdef > scripts/Makefile.extrawarn | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > index a74ce2e3c33e..95973a1ee999 100644 > --- a/scripts/Makefile.extrawarn > +++ b/scripts/Makefile.extrawarn > @@ -70,5 +70,6 @@ KBUILD_CFLAGS += -Wno-initializer-overrides > KBUILD_CFLAGS += -Wno-format > KBUILD_CFLAGS += -Wno-sign-compare > KBUILD_CFLAGS += -Wno-format-zero-length > +KBUILD_CFLAGS += $(call cc-option,-Wno-implicit-fallthrough) Clang seems to support -Wno-implicit-fallthrough since 3.2. You can remove the cc-option, since you'll need a newer clang than that to build the kernel.
Hi. On Fri, Aug 16, 2019 at 7:59 AM Nathan Huckleberry <nhuck@google.com> wrote: > > Clang is updating to support -Wimplicit-fallthrough on C > https://reviews.llvm.org/D64838. Since clang does not > support the comment version of fallthrough annotations > this update causes an additional 50k warnings. Most > of these warnings (>49k) are duplicates from header files. > > This patch is intended to be reverted after the warnings > have been cleaned up. > > Signed-off-by: Nathan Huckleberry <nhuck@google.com> > Suggested-by: Nathan Chancellor <natechancellor@gmail.com> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > --- > Changes v1->v2 > * Move code to preexisting ifdef > scripts/Makefile.extrawarn | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > index a74ce2e3c33e..95973a1ee999 100644 > --- a/scripts/Makefile.extrawarn > +++ b/scripts/Makefile.extrawarn > @@ -70,5 +70,6 @@ KBUILD_CFLAGS += -Wno-initializer-overrides > KBUILD_CFLAGS += -Wno-format > KBUILD_CFLAGS += -Wno-sign-compare > KBUILD_CFLAGS += -Wno-format-zero-length > +KBUILD_CFLAGS += $(call cc-option,-Wno-implicit-fallthrough) > endif > endif > -- > 2.23.0.rc1.153.gdeed80330f-goog > Perhaps, is the following even cleaner? diff --git a/Makefile b/Makefile index 1b23f95db176..cebc6bf5372e 100644 --- a/Makefile +++ b/Makefile @@ -751,6 +751,9 @@ else # These warnings generated too much noise in a regular build. # Use make W=1 to enable them (see scripts/Makefile.extrawarn) KBUILD_CFLAGS += -Wno-unused-but-set-variable + +# Warn about unmarked fall-throughs in switch statement. +KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,) endif KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) @@ -845,9 +848,6 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include) # warn about C99 declaration after statement KBUILD_CFLAGS += -Wdeclaration-after-statement -# Warn about unmarked fall-throughs in switch statement. -KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,) - # Variable Length Arrays (VLAs) should not be used anywhere in the kernel KBUILD_CFLAGS += -Wvla
On Mon, Aug 19, 2019 at 01:43:08AM +0900, Masahiro Yamada wrote: > Hi. > > On Fri, Aug 16, 2019 at 7:59 AM Nathan Huckleberry <nhuck@google.com> wrote: > > > > Clang is updating to support -Wimplicit-fallthrough on C > > https://reviews.llvm.org/D64838. Since clang does not > > support the comment version of fallthrough annotations > > this update causes an additional 50k warnings. Most > > of these warnings (>49k) are duplicates from header files. > > > > This patch is intended to be reverted after the warnings > > have been cleaned up. > > > > Signed-off-by: Nathan Huckleberry <nhuck@google.com> > > Suggested-by: Nathan Chancellor <natechancellor@gmail.com> > > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > > --- > > Changes v1->v2 > > * Move code to preexisting ifdef > > scripts/Makefile.extrawarn | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > > index a74ce2e3c33e..95973a1ee999 100644 > > --- a/scripts/Makefile.extrawarn > > +++ b/scripts/Makefile.extrawarn > > @@ -70,5 +70,6 @@ KBUILD_CFLAGS += -Wno-initializer-overrides > > KBUILD_CFLAGS += -Wno-format > > KBUILD_CFLAGS += -Wno-sign-compare > > KBUILD_CFLAGS += -Wno-format-zero-length > > +KBUILD_CFLAGS += $(call cc-option,-Wno-implicit-fallthrough) > > endif > > endif > > -- > > 2.23.0.rc1.153.gdeed80330f-goog > > > > > Perhaps, is the following even cleaner? > > > > diff --git a/Makefile b/Makefile > index 1b23f95db176..cebc6bf5372e 100644 > --- a/Makefile > +++ b/Makefile > @@ -751,6 +751,9 @@ else > # These warnings generated too much noise in a regular build. > # Use make W=1 to enable them (see scripts/Makefile.extrawarn) > KBUILD_CFLAGS += -Wno-unused-but-set-variable > + > +# Warn about unmarked fall-throughs in switch statement. > +KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,) > endif > > KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > @@ -845,9 +848,6 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) > -print-file-name=include) > # warn about C99 declaration after statement > KBUILD_CFLAGS += -Wdeclaration-after-statement > > -# Warn about unmarked fall-throughs in switch statement. > -KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,) > - > # Variable Length Arrays (VLAs) should not be used anywhere in the kernel > KBUILD_CFLAGS += -Wvla > > > > -- > Best Regards > Masahiro Yamada I like this more than anything suggested so far. I think a comment should be added regarding why this is only enabled for GCC right now but that is pretty easy to revert once we have figured out the right course of action. Cheers, Nathan
On Mon, Aug 19, 2019 at 3:43 AM Nathan Chancellor <natechancellor@gmail.com> wrote: > > On Mon, Aug 19, 2019 at 01:43:08AM +0900, Masahiro Yamada wrote: > > Hi. > > > > On Fri, Aug 16, 2019 at 7:59 AM Nathan Huckleberry <nhuck@google.com> wrote: > > > > > > Clang is updating to support -Wimplicit-fallthrough on C > > > https://reviews.llvm.org/D64838. Since clang does not > > > support the comment version of fallthrough annotations > > > this update causes an additional 50k warnings. Most > > > of these warnings (>49k) are duplicates from header files. > > > > > > This patch is intended to be reverted after the warnings > > > have been cleaned up. > > > > > > Signed-off-by: Nathan Huckleberry <nhuck@google.com> > > > Suggested-by: Nathan Chancellor <natechancellor@gmail.com> > > > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > > > --- > > > Changes v1->v2 > > > * Move code to preexisting ifdef > > > scripts/Makefile.extrawarn | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > > > index a74ce2e3c33e..95973a1ee999 100644 > > > --- a/scripts/Makefile.extrawarn > > > +++ b/scripts/Makefile.extrawarn > > > @@ -70,5 +70,6 @@ KBUILD_CFLAGS += -Wno-initializer-overrides > > > KBUILD_CFLAGS += -Wno-format > > > KBUILD_CFLAGS += -Wno-sign-compare > > > KBUILD_CFLAGS += -Wno-format-zero-length > > > +KBUILD_CFLAGS += $(call cc-option,-Wno-implicit-fallthrough) > > > endif > > > endif > > > -- > > > 2.23.0.rc1.153.gdeed80330f-goog > > > > > > > > > Perhaps, is the following even cleaner? > > > > > > > > diff --git a/Makefile b/Makefile > > index 1b23f95db176..cebc6bf5372e 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -751,6 +751,9 @@ else > > # These warnings generated too much noise in a regular build. > > # Use make W=1 to enable them (see scripts/Makefile.extrawarn) > > KBUILD_CFLAGS += -Wno-unused-but-set-variable > > + > > +# Warn about unmarked fall-throughs in switch statement. > > +KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,) > > endif > > > > KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > > @@ -845,9 +848,6 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) > > -print-file-name=include) > > # warn about C99 declaration after statement > > KBUILD_CFLAGS += -Wdeclaration-after-statement > > > > -# Warn about unmarked fall-throughs in switch statement. > > -KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,) > > - > > # Variable Length Arrays (VLAs) should not be used anywhere in the kernel > > KBUILD_CFLAGS += -Wvla > > > > > > > > -- > > Best Regards > > Masahiro Yamada > > I like this more than anything suggested so far. I think a comment > should be added regarding why this is only enabled for GCC right now but > that is pretty easy to revert once we have figured out the right course > of action. Agree. This is well-explained in the commit log, but adding a short comment will be nice. BTW, I personally like the traditional comment version of fallthrough annotations. Is there a plan for Clang to support it as well as the attribute? Thanks.
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn index a74ce2e3c33e..95973a1ee999 100644 --- a/scripts/Makefile.extrawarn +++ b/scripts/Makefile.extrawarn @@ -70,5 +70,6 @@ KBUILD_CFLAGS += -Wno-initializer-overrides KBUILD_CFLAGS += -Wno-format KBUILD_CFLAGS += -Wno-sign-compare KBUILD_CFLAGS += -Wno-format-zero-length +KBUILD_CFLAGS += $(call cc-option,-Wno-implicit-fallthrough) endif endif