diff mbox series

[1/2] kbuild: refactor scripts/Makefile.extrawarn

Message ID 20190828055425.24765-1-yamada.masahiro@socionext.com
State Superseded
Headers show
Series [1/2] kbuild: refactor scripts/Makefile.extrawarn | expand

Commit Message

Masahiro Yamada Aug. 28, 2019, 5:54 a.m. UTC 
Instead of the warning-[123] magic, let's accumulate compiler options
to KBUILD_CFLAGS directly as the top Makefile does. I think this makes
easier to understand what is going on in this file.

This commit slightly changes the behavior, I think all of which are OK.

[1] Currently, cc-option calls are needlessly evaluated. For example,
      warning-3 += $(call cc-option, -Wpacked-bitfield-compat)
    needs evaluating only when W=3, but it is actually evaluated for
    W=1, W=2 as well. With this commit, only relevant cc-option calls
    will be evaluated. This is a slight optimization.

[2] Currently, unsupported level like W=4 is checked by:
      $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)
    This will no longer be checked, but I do not think it is a big
    deal.

[3] Currently, 4 Clang warnings (Winitializer-overrides, Wformat,
    Wsign-compare, Wformat-zero-length) are shown by any of W=1, W=2,
    and W=3. With this commit, they will be warned only by W=1. I
    think this is a more correct behavior since each warning belongs
    to only one warning level.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

 scripts/Makefile.extrawarn | 104 +++++++++++++++++++------------------
 1 file changed, 53 insertions(+), 51 deletions(-)

-- 
2.17.1

Comments

Sedat Dilek Aug. 28, 2019, 7:20 a.m. UTC | #1 
On Wed, Aug 28, 2019 at 7:55 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>

> Instead of the warning-[123] magic, let's accumulate compiler options

> to KBUILD_CFLAGS directly as the top Makefile does. I think this makes

> easier to understand what is going on in this file.

>

> This commit slightly changes the behavior, I think all of which are OK.

>

> [1] Currently, cc-option calls are needlessly evaluated. For example,

>       warning-3 += $(call cc-option, -Wpacked-bitfield-compat)

>     needs evaluating only when W=3, but it is actually evaluated for

>     W=1, W=2 as well. With this commit, only relevant cc-option calls

>     will be evaluated. This is a slight optimization.

>

> [2] Currently, unsupported level like W=4 is checked by:

>       $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)

>     This will no longer be checked, but I do not think it is a big

>     deal.

>


Hi Masahiro Yamada,

thanks for your patch series.

If KBUILD_ENABLE_EXTRA_GCC_CHECKS does extra(-warning)-checks for GCC and Clang,
please rename the Kconfig into...

KBUILD_ENABLE_EXTRA_CC_CHECKS

...or something similiar (and maybe with some notes in its Kconfig help-text?).

Regards,
- Sedat -

> [3] Currently, 4 Clang warnings (Winitializer-overrides, Wformat,

>     Wsign-compare, Wformat-zero-length) are shown by any of W=1, W=2,

>     and W=3. With this commit, they will be warned only by W=1. I

>     think this is a more correct behavior since each warning belongs

>     to only one warning level.

>

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> ---

>

>  scripts/Makefile.extrawarn | 104 +++++++++++++++++++------------------

>  1 file changed, 53 insertions(+), 51 deletions(-)

>

> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn

> index a74ce2e3c33e..1fa53968e292 100644

> --- a/scripts/Makefile.extrawarn

> +++ b/scripts/Makefile.extrawarn

> @@ -1,14 +1,6 @@

>  # SPDX-License-Identifier: GPL-2.0

>  # ==========================================================================

> -#

>  # make W=... settings

> -#

> -# W=1 - warnings that may be relevant and does not occur too often

> -# W=2 - warnings that occur quite often but may still be relevant

> -# W=3 - the more obscure warnings, can most likely be ignored

> -#

> -# $(call cc-option, -W...) handles gcc -W.. options which

> -# are not supported by all versions of the compiler

>  # ==========================================================================

>

>  KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)

> @@ -17,58 +9,68 @@ ifeq ("$(origin W)", "command line")

>    export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W)

>  endif

>

> -ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS

> -warning-  := $(empty)

> +#

> +# W=1 - warnings that may be relevant and does not occur too often

> +#

> +ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)

>

> -warning-1 := -Wextra -Wunused -Wno-unused-parameter

> -warning-1 += -Wmissing-declarations

> -warning-1 += -Wmissing-format-attribute

> -warning-1 += -Wmissing-prototypes

> -warning-1 += -Wold-style-definition

> -warning-1 += -Wmissing-include-dirs

> -warning-1 += $(call cc-option, -Wunused-but-set-variable)

> -warning-1 += $(call cc-option, -Wunused-const-variable)

> -warning-1 += $(call cc-option, -Wpacked-not-aligned)

> -warning-1 += $(call cc-option, -Wstringop-truncation)

> +KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter

> +KBUILD_CFLAGS += -Wmissing-declarations

> +KBUILD_CFLAGS += -Wmissing-format-attribute

> +KBUILD_CFLAGS += -Wmissing-prototypes

> +KBUILD_CFLAGS += -Wold-style-definition

> +KBUILD_CFLAGS += -Wmissing-include-dirs

> +KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable)

> +KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable)

> +KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned)

> +KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)

>  # The following turn off the warnings enabled by -Wextra

> -warning-1 += -Wno-missing-field-initializers

> -warning-1 += -Wno-sign-compare

> -

> -warning-2 += -Wcast-align

> -warning-2 += -Wdisabled-optimization

> -warning-2 += -Wnested-externs

> -warning-2 += -Wshadow

> -warning-2 += $(call cc-option, -Wlogical-op)

> -warning-2 += -Wmissing-field-initializers

> -warning-2 += -Wsign-compare

> -warning-2 += $(call cc-option, -Wmaybe-uninitialized)

> -warning-2 += $(call cc-option, -Wunused-macros)

> -

> -warning-3 := -Wbad-function-cast

> -warning-3 += -Wcast-qual

> -warning-3 += -Wconversion

> -warning-3 += -Wpacked

> -warning-3 += -Wpadded

> -warning-3 += -Wpointer-arith

> -warning-3 += -Wredundant-decls

> -warning-3 += -Wswitch-default

> -warning-3 += $(call cc-option, -Wpacked-bitfield-compat)

> -

> -warning := $(warning-$(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))

> -warning += $(warning-$(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))

> -warning += $(warning-$(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))

> -

> -ifeq ("$(strip $(warning))","")

> -        $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)

> -endif

> +KBUILD_CFLAGS += -Wno-missing-field-initializers

> +KBUILD_CFLAGS += -Wno-sign-compare

>

> -KBUILD_CFLAGS += $(warning)

>  else

>

> +# W=1 also stops suppressing some warnings

> +

>  ifdef CONFIG_CC_IS_CLANG

>  KBUILD_CFLAGS += -Wno-initializer-overrides

>  KBUILD_CFLAGS += -Wno-format

>  KBUILD_CFLAGS += -Wno-sign-compare

>  KBUILD_CFLAGS += -Wno-format-zero-length

>  endif

> +

> +endif

> +

> +#

> +# W=2 - warnings that occur quite often but may still be relevant

> +#

> +ifneq ($(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)

> +

> +KBUILD_CFLAGS += -Wcast-align

> +KBUILD_CFLAGS += -Wdisabled-optimization

> +KBUILD_CFLAGS += -Wnested-externs

> +KBUILD_CFLAGS += -Wshadow

> +KBUILD_CFLAGS += $(call cc-option, -Wlogical-op)

> +KBUILD_CFLAGS += -Wmissing-field-initializers

> +KBUILD_CFLAGS += -Wsign-compare

> +KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized)

> +KBUILD_CFLAGS += $(call cc-option, -Wunused-macros)

> +

> +endif

> +

> +#

> +# W=3 - the more obscure warnings, can most likely be ignored

> +#

> +ifneq ($(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)

> +

> +KBUILD_CFLAGS += -Wbad-function-cast

> +KBUILD_CFLAGS += -Wcast-qual

> +KBUILD_CFLAGS += -Wconversion

> +KBUILD_CFLAGS += -Wpacked

> +KBUILD_CFLAGS += -Wpadded

> +KBUILD_CFLAGS += -Wpointer-arith

> +KBUILD_CFLAGS += -Wredundant-decls

> +KBUILD_CFLAGS += -Wswitch-default

> +KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat)

> +

>  endif

> --

> 2.17.1

>

> --

> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.

> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20190828055425.24765-1-yamada.masahiro%40socionext.com.
Sedat Dilek Aug. 28, 2019, 2:18 p.m. UTC | #2 
On Wed, Aug 28, 2019 at 9:20 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>

> On Wed, Aug 28, 2019 at 7:55 AM Masahiro Yamada

> <yamada.masahiro@socionext.com> wrote:

> >

> > Instead of the warning-[123] magic, let's accumulate compiler options

> > to KBUILD_CFLAGS directly as the top Makefile does. I think this makes

> > easier to understand what is going on in this file.

> >

> > This commit slightly changes the behavior, I think all of which are OK.

> >

> > [1] Currently, cc-option calls are needlessly evaluated. For example,

> >       warning-3 += $(call cc-option, -Wpacked-bitfield-compat)

> >     needs evaluating only when W=3, but it is actually evaluated for

> >     W=1, W=2 as well. With this commit, only relevant cc-option calls

> >     will be evaluated. This is a slight optimization.

> >

> > [2] Currently, unsupported level like W=4 is checked by:

> >       $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)

> >     This will no longer be checked, but I do not think it is a big

> >     deal.

> >

>

> Hi Masahiro Yamada,

>

> thanks for your patch series.

>

> If KBUILD_ENABLE_EXTRA_GCC_CHECKS does extra(-warning)-checks for GCC and Clang,

> please rename the Kconfig into...

>

> KBUILD_ENABLE_EXTRA_CC_CHECKS

>

> ...or something similiar (and maybe with some notes in its Kconfig help-text?).

>


I have tested both patches against recent kbuild-next and can boot on
bare metal with clang.

I have *not* passed any W= to my make, but I see that clang's W=1
kbuild-cflags are active.

[ scripts/Makefile.extrawarn ]

ifeq ("$(origin W)", "command line")
  export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W)
endif

#
# W=1 - warnings that may be relevant and does not occur too often
#
ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
[ ... ]
KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1

else

# W=1 also stops suppressing some warnings

ifdef CONFIG_CC_IS_CLANG
KBUILD_CFLAGS += -Wno-initializer-overrides
KBUILD_CFLAGS += -Wno-format
KBUILD_CFLAGS += -Wno-sign-compare
KBUILD_CFLAGS += -Wno-format-zero-length
endif # CONFIG_CC_IS_CLANG

endif # KBUILD_ENABLE_EXTRA_GCC_CHECKS

These clang KBUILD_CFLAGS are active independently of passing W=1.

$ grep '\-Wno-initializer-overrides'
build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt | wc -l
27195

So the above comment is misleading?

Is W=1 activated by default?

Or do I miss something?

[ Documentation/kbuild/kbuild.rst ]

KBUILD_ENABLE_EXTRA_GCC_CHECKS
------------------------------
If enabled over the make command line with "W=1", it turns on additional
gcc -W... options for more extensive build-time checking.

What about?

KBUILD_CC_EXTRA_CHECKS (or KBUILD_EXTRA_CC_CHECKS)
------------------------------
If enabled over the make command line with "W=...", it turns on additional
compiler warning options like -Wmissing-declarations for more extensive
build-time checking. For more details see <Documentation/kbuild/kbuild.rst>.

W=1 - warnings that may be relevant and does not occur too often
W=1 - also stops suppressing some warnings
W=2 - warnings that occur quite often but may still be relevant
W=3 - the more obscure warnings, can most likely be ignored

- Sedat -
Sedat Dilek Aug. 28, 2019, 2:21 p.m. UTC | #3 
> build-time checking. For more details see <Documentation/kbuild/kbuild.rst>.


Grrr.

s/ Documentation/kbuild/kbuild.rst / scripts/Makefile.extrawarn

- Sedat -
Sedat Dilek Aug. 28, 2019, 3:59 p.m. UTC | #4 
Something like that...

[PATCH 1/2] kbuild: Improve extrawarn documentation
[PATCH 2/2] kbuild: Rename extrawarn Kconfig to KBUILD_EXTRA_CC_CHECKS

- Sedat -
From 1275ec0f1d31c4ac57b73b318bdc45151d99e8dc Mon Sep 17 00:00:00 2001
From: Sedat Dilek <sedat.dilek@credativ.de>
Date: Wed, 28 Aug 2019 16:27:13 +0200
Subject: [PATCH 1/2] kbuild: Improve extrawarn documentation

---
 Documentation/kbuild/kbuild.rst | 10 ++++++++--
 scripts/Makefile.extrawarn      |  2 +-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
index 62f9d86c082c..f0f1c475d7fa 100644
--- a/Documentation/kbuild/kbuild.rst
+++ b/Documentation/kbuild/kbuild.rst
@@ -243,8 +243,14 @@ To get all available archs you can also specify all. E.g.::
 
 KBUILD_ENABLE_EXTRA_GCC_CHECKS
 ------------------------------
-If enabled over the make command line with "W=1", it turns on additional
-gcc -W... options for more extensive build-time checking.
+If enabled over the make command line with "W=...", it turns on additional
+compiler warning options like -Wmissing-declarations for more extensive
+build-time checking. For more details see <scripts/Makefile.extrawarn>.
+
+W=1 - warnings that may be relevant and does not occur too often
+W=1 - also stops suppressing some warnings
+W=2 - warnings that occur quite often but may still be relevant
+W=3 - the more obscure warnings, can most likely be ignored
 
 KBUILD_BUILD_TIMESTAMP
 ----------------------
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 3af1770497fd..6770f8da4e6d 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -32,7 +32,7 @@ KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1
 
 else
 
-# W=1 also stops suppressing some warnings
+# W=1 - also stops suppressing some warnings
 
 ifdef CONFIG_CC_IS_CLANG
 KBUILD_CFLAGS += -Wno-initializer-overrides
Nathan Chancellor Aug. 28, 2019, 6:17 p.m. UTC | #5 
On Wed, Aug 28, 2019 at 02:54:24PM +0900, Masahiro Yamada wrote:
> Instead of the warning-[123] magic, let's accumulate compiler options

> to KBUILD_CFLAGS directly as the top Makefile does. I think this makes

> easier to understand what is going on in this file.

> 

> This commit slightly changes the behavior, I think all of which are OK.

> 

> [1] Currently, cc-option calls are needlessly evaluated. For example,

>       warning-3 += $(call cc-option, -Wpacked-bitfield-compat)

>     needs evaluating only when W=3, but it is actually evaluated for

>     W=1, W=2 as well. With this commit, only relevant cc-option calls

>     will be evaluated. This is a slight optimization.

> 

> [2] Currently, unsupported level like W=4 is checked by:

>       $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)

>     This will no longer be checked, but I do not think it is a big

>     deal.

> 

> [3] Currently, 4 Clang warnings (Winitializer-overrides, Wformat,

>     Wsign-compare, Wformat-zero-length) are shown by any of W=1, W=2,

>     and W=3. With this commit, they will be warned only by W=1. I

>     think this is a more correct behavior since each warning belongs

>     to only one warning level.

> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>


Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Nick Desaulniers Aug. 28, 2019, 6:26 p.m. UTC | #6 
On Wed, Aug 28, 2019 at 12:20 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>

> On Wed, Aug 28, 2019 at 7:55 AM Masahiro Yamada

> <yamada.masahiro@socionext.com> wrote:

> >

> > Instead of the warning-[123] magic, let's accumulate compiler options

> > to KBUILD_CFLAGS directly as the top Makefile does. I think this makes

> > easier to understand what is going on in this file.

> >

> > This commit slightly changes the behavior, I think all of which are OK.

> >

> > [1] Currently, cc-option calls are needlessly evaluated. For example,

> >       warning-3 += $(call cc-option, -Wpacked-bitfield-compat)

> >     needs evaluating only when W=3, but it is actually evaluated for

> >     W=1, W=2 as well. With this commit, only relevant cc-option calls

> >     will be evaluated. This is a slight optimization.

> >

> > [2] Currently, unsupported level like W=4 is checked by:

> >       $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)

> >     This will no longer be checked, but I do not think it is a big

> >     deal.

> >

>

> Hi Masahiro Yamada,

>

> thanks for your patch series.

>

> If KBUILD_ENABLE_EXTRA_GCC_CHECKS does extra(-warning)-checks for GCC and Clang,

> please rename the Kconfig into...

>

> KBUILD_ENABLE_EXTRA_CC_CHECKS

>

> ...or something similiar (and maybe with some notes in its Kconfig help-text?).


I too would like to see that changed.

-- 
Thanks,
~Nick Desaulniers
Nick Desaulniers Aug. 28, 2019, 10:38 p.m. UTC | #7 
On Tue, Aug 27, 2019 at 10:54 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>

> Instead of the warning-[123] magic, let's accumulate compiler options

> to KBUILD_CFLAGS directly as the top Makefile does. I think this makes

> easier to understand what is going on in this file.

>

> This commit slightly changes the behavior, I think all of which are OK.

>

> [1] Currently, cc-option calls are needlessly evaluated. For example,

>       warning-3 += $(call cc-option, -Wpacked-bitfield-compat)

>     needs evaluating only when W=3, but it is actually evaluated for

>     W=1, W=2 as well. With this commit, only relevant cc-option calls

>     will be evaluated. This is a slight optimization.

>

> [2] Currently, unsupported level like W=4 is checked by:

>       $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)

>     This will no longer be checked, but I do not think it is a big

>     deal.

>

> [3] Currently, 4 Clang warnings (Winitializer-overrides, Wformat,

>     Wsign-compare, Wformat-zero-length) are shown by any of W=1, W=2,

>     and W=3. With this commit, they will be warned only by W=1. I

>     think this is a more correct behavior since each warning belongs

>     to only one warning level.

>

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> ---

>

>  scripts/Makefile.extrawarn | 104 +++++++++++++++++++------------------

>  1 file changed, 53 insertions(+), 51 deletions(-)

>

> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn

> index a74ce2e3c33e..1fa53968e292 100644

> --- a/scripts/Makefile.extrawarn

> +++ b/scripts/Makefile.extrawarn

> @@ -1,14 +1,6 @@

>  # SPDX-License-Identifier: GPL-2.0

>  # ==========================================================================

> -#

>  # make W=... settings

> -#

> -# W=1 - warnings that may be relevant and does not occur too often

> -# W=2 - warnings that occur quite often but may still be relevant

> -# W=3 - the more obscure warnings, can most likely be ignored

> -#

> -# $(call cc-option, -W...) handles gcc -W.. options which

> -# are not supported by all versions of the compiler

>  # ==========================================================================

>

>  KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)

> @@ -17,58 +9,68 @@ ifeq ("$(origin W)", "command line")

>    export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W)

>  endif

>

> -ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS

> -warning-  := $(empty)

> +#

> +# W=1 - warnings that may be relevant and does not occur too often

> +#

> +ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)

>

> -warning-1 := -Wextra -Wunused -Wno-unused-parameter

> -warning-1 += -Wmissing-declarations

> -warning-1 += -Wmissing-format-attribute

> -warning-1 += -Wmissing-prototypes

> -warning-1 += -Wold-style-definition

> -warning-1 += -Wmissing-include-dirs

> -warning-1 += $(call cc-option, -Wunused-but-set-variable)

> -warning-1 += $(call cc-option, -Wunused-const-variable)

> -warning-1 += $(call cc-option, -Wpacked-not-aligned)

> -warning-1 += $(call cc-option, -Wstringop-truncation)

> +KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter

> +KBUILD_CFLAGS += -Wmissing-declarations

> +KBUILD_CFLAGS += -Wmissing-format-attribute

> +KBUILD_CFLAGS += -Wmissing-prototypes

> +KBUILD_CFLAGS += -Wold-style-definition

> +KBUILD_CFLAGS += -Wmissing-include-dirs

> +KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable)

> +KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable)

> +KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned)

> +KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)

>  # The following turn off the warnings enabled by -Wextra

> -warning-1 += -Wno-missing-field-initializers

> -warning-1 += -Wno-sign-compare

> -

> -warning-2 += -Wcast-align

> -warning-2 += -Wdisabled-optimization

> -warning-2 += -Wnested-externs

> -warning-2 += -Wshadow

> -warning-2 += $(call cc-option, -Wlogical-op)

> -warning-2 += -Wmissing-field-initializers

> -warning-2 += -Wsign-compare

> -warning-2 += $(call cc-option, -Wmaybe-uninitialized)

> -warning-2 += $(call cc-option, -Wunused-macros)

> -

> -warning-3 := -Wbad-function-cast

> -warning-3 += -Wcast-qual

> -warning-3 += -Wconversion

> -warning-3 += -Wpacked

> -warning-3 += -Wpadded

> -warning-3 += -Wpointer-arith

> -warning-3 += -Wredundant-decls

> -warning-3 += -Wswitch-default

> -warning-3 += $(call cc-option, -Wpacked-bitfield-compat)

> -

> -warning := $(warning-$(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))

> -warning += $(warning-$(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))

> -warning += $(warning-$(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))

> -

> -ifeq ("$(strip $(warning))","")

> -        $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)

> -endif

> +KBUILD_CFLAGS += -Wno-missing-field-initializers

> +KBUILD_CFLAGS += -Wno-sign-compare

>

> -KBUILD_CFLAGS += $(warning)

>  else

>

> +# W=1 also stops suppressing some warnings

> +

>  ifdef CONFIG_CC_IS_CLANG

>  KBUILD_CFLAGS += -Wno-initializer-overrides

>  KBUILD_CFLAGS += -Wno-format

>  KBUILD_CFLAGS += -Wno-sign-compare

>  KBUILD_CFLAGS += -Wno-format-zero-length

>  endif


I find this part of the patch exceedingly confusing, and I think it
mistakenly changes the behavior of W=2, W=3, and W=4.  If W != 1 && CC
== clang, then disable some flags?  What?  So W=2,3,4 those are
disabled, but at W=1 are not?  Didn't the previous version set these
unless any W= was set?

> +

> +endif

> +

> +#

> +# W=2 - warnings that occur quite often but may still be relevant

> +#

> +ifneq ($(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)

> +

> +KBUILD_CFLAGS += -Wcast-align

> +KBUILD_CFLAGS += -Wdisabled-optimization

> +KBUILD_CFLAGS += -Wnested-externs

> +KBUILD_CFLAGS += -Wshadow

> +KBUILD_CFLAGS += $(call cc-option, -Wlogical-op)

> +KBUILD_CFLAGS += -Wmissing-field-initializers

> +KBUILD_CFLAGS += -Wsign-compare

> +KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized)

> +KBUILD_CFLAGS += $(call cc-option, -Wunused-macros)

> +

> +endif

> +

> +#

> +# W=3 - the more obscure warnings, can most likely be ignored

> +#

> +ifneq ($(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)

> +

> +KBUILD_CFLAGS += -Wbad-function-cast

> +KBUILD_CFLAGS += -Wcast-qual

> +KBUILD_CFLAGS += -Wconversion

> +KBUILD_CFLAGS += -Wpacked

> +KBUILD_CFLAGS += -Wpadded

> +KBUILD_CFLAGS += -Wpointer-arith

> +KBUILD_CFLAGS += -Wredundant-decls

> +KBUILD_CFLAGS += -Wswitch-default

> +KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat)

> +

>  endif

> --

> 2.17.1

>



-- 
Thanks,
~Nick Desaulniers
Sedat Dilek Aug. 29, 2019, 8:49 a.m. UTC | #8 
On Thu, Aug 29, 2019 at 12:38 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>

> On Tue, Aug 27, 2019 at 10:54 PM Masahiro Yamada

> <yamada.masahiro@socionext.com> wrote:

> >

> > Instead of the warning-[123] magic, let's accumulate compiler options

> > to KBUILD_CFLAGS directly as the top Makefile does. I think this makes

> > easier to understand what is going on in this file.

> >

> > This commit slightly changes the behavior, I think all of which are OK.

> >

> > [1] Currently, cc-option calls are needlessly evaluated. For example,

> >       warning-3 += $(call cc-option, -Wpacked-bitfield-compat)

> >     needs evaluating only when W=3, but it is actually evaluated for

> >     W=1, W=2 as well. With this commit, only relevant cc-option calls

> >     will be evaluated. This is a slight optimization.

> >

> > [2] Currently, unsupported level like W=4 is checked by:

> >       $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)

> >     This will no longer be checked, but I do not think it is a big

> >     deal.

> >

> > [3] Currently, 4 Clang warnings (Winitializer-overrides, Wformat,

> >     Wsign-compare, Wformat-zero-length) are shown by any of W=1, W=2,

> >     and W=3. With this commit, they will be warned only by W=1. I

> >     think this is a more correct behavior since each warning belongs

> >     to only one warning level.

> >

> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> > ---

> >

> >  scripts/Makefile.extrawarn | 104 +++++++++++++++++++------------------

> >  1 file changed, 53 insertions(+), 51 deletions(-)

> >

> > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn

> > index a74ce2e3c33e..1fa53968e292 100644

> > --- a/scripts/Makefile.extrawarn

> > +++ b/scripts/Makefile.extrawarn

> > @@ -1,14 +1,6 @@

> >  # SPDX-License-Identifier: GPL-2.0

> >  # ==========================================================================

> > -#

> >  # make W=... settings

> > -#

> > -# W=1 - warnings that may be relevant and does not occur too often

> > -# W=2 - warnings that occur quite often but may still be relevant

> > -# W=3 - the more obscure warnings, can most likely be ignored

> > -#

> > -# $(call cc-option, -W...) handles gcc -W.. options which

> > -# are not supported by all versions of the compiler

> >  # ==========================================================================

> >

> >  KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)

> > @@ -17,58 +9,68 @@ ifeq ("$(origin W)", "command line")

> >    export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W)

> >  endif

> >

> > -ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS

> > -warning-  := $(empty)

> > +#

> > +# W=1 - warnings that may be relevant and does not occur too often

> > +#

> > +ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)

> >

> > -warning-1 := -Wextra -Wunused -Wno-unused-parameter

> > -warning-1 += -Wmissing-declarations

> > -warning-1 += -Wmissing-format-attribute

> > -warning-1 += -Wmissing-prototypes

> > -warning-1 += -Wold-style-definition

> > -warning-1 += -Wmissing-include-dirs

> > -warning-1 += $(call cc-option, -Wunused-but-set-variable)

> > -warning-1 += $(call cc-option, -Wunused-const-variable)

> > -warning-1 += $(call cc-option, -Wpacked-not-aligned)

> > -warning-1 += $(call cc-option, -Wstringop-truncation)

> > +KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter

> > +KBUILD_CFLAGS += -Wmissing-declarations

> > +KBUILD_CFLAGS += -Wmissing-format-attribute

> > +KBUILD_CFLAGS += -Wmissing-prototypes

> > +KBUILD_CFLAGS += -Wold-style-definition

> > +KBUILD_CFLAGS += -Wmissing-include-dirs

> > +KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable)

> > +KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable)

> > +KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned)

> > +KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)

> >  # The following turn off the warnings enabled by -Wextra

> > -warning-1 += -Wno-missing-field-initializers

> > -warning-1 += -Wno-sign-compare

> > -

> > -warning-2 += -Wcast-align

> > -warning-2 += -Wdisabled-optimization

> > -warning-2 += -Wnested-externs

> > -warning-2 += -Wshadow

> > -warning-2 += $(call cc-option, -Wlogical-op)

> > -warning-2 += -Wmissing-field-initializers

> > -warning-2 += -Wsign-compare

> > -warning-2 += $(call cc-option, -Wmaybe-uninitialized)

> > -warning-2 += $(call cc-option, -Wunused-macros)

> > -

> > -warning-3 := -Wbad-function-cast

> > -warning-3 += -Wcast-qual

> > -warning-3 += -Wconversion

> > -warning-3 += -Wpacked

> > -warning-3 += -Wpadded

> > -warning-3 += -Wpointer-arith

> > -warning-3 += -Wredundant-decls

> > -warning-3 += -Wswitch-default

> > -warning-3 += $(call cc-option, -Wpacked-bitfield-compat)

> > -

> > -warning := $(warning-$(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))

> > -warning += $(warning-$(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))

> > -warning += $(warning-$(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))

> > -

> > -ifeq ("$(strip $(warning))","")

> > -        $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)

> > -endif

> > +KBUILD_CFLAGS += -Wno-missing-field-initializers

> > +KBUILD_CFLAGS += -Wno-sign-compare

> >

> > -KBUILD_CFLAGS += $(warning)

> >  else

> >

> > +# W=1 also stops suppressing some warnings

> > +

> >  ifdef CONFIG_CC_IS_CLANG

> >  KBUILD_CFLAGS += -Wno-initializer-overrides

> >  KBUILD_CFLAGS += -Wno-format

> >  KBUILD_CFLAGS += -Wno-sign-compare

> >  KBUILD_CFLAGS += -Wno-format-zero-length

> >  endif

>

> I find this part of the patch exceedingly confusing, and I think it

> mistakenly changes the behavior of W=2, W=3, and W=4.  If W != 1 && CC

> == clang, then disable some flags?  What?  So W=2,3,4 those are

> disabled, but at W=1 are not?  Didn't the previous version set these

> unless any W= was set?

>

> > +

> > +endif

> > +

> > +#

> > +# W=2 - warnings that occur quite often but may still be relevant

> > +#

> > +ifneq ($(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)

> > +

> > +KBUILD_CFLAGS += -Wcast-align

> > +KBUILD_CFLAGS += -Wdisabled-optimization

> > +KBUILD_CFLAGS += -Wnested-externs

> > +KBUILD_CFLAGS += -Wshadow

> > +KBUILD_CFLAGS += $(call cc-option, -Wlogical-op)

> > +KBUILD_CFLAGS += -Wmissing-field-initializers

> > +KBUILD_CFLAGS += -Wsign-compare

> > +KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized)

> > +KBUILD_CFLAGS += $(call cc-option, -Wunused-macros)

> > +

> > +endif

> > +

> > +#

> > +# W=3 - the more obscure warnings, can most likely be ignored

> > +#

> > +ifneq ($(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)

> > +

> > +KBUILD_CFLAGS += -Wbad-function-cast

> > +KBUILD_CFLAGS += -Wcast-qual

> > +KBUILD_CFLAGS += -Wconversion

> > +KBUILD_CFLAGS += -Wpacked

> > +KBUILD_CFLAGS += -Wpadded

> > +KBUILD_CFLAGS += -Wpointer-arith

> > +KBUILD_CFLAGS += -Wredundant-decls

> > +KBUILD_CFLAGS += -Wswitch-default

> > +KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat)

> > +

> >  endif

> > --

> > 2.17.1

> >

>

>

> --

> Thanks,

> ~Nick Desaulniers


Just a quick test...

...with clang-9 (built with tc-build from llvm-project.git#release/9.x)

$ mycompiler --version
ClangBuiltLinux clang version 9.0.0
(git://github.com/llvm/llvm-project
e82a53603ae3fed2215a44b5ac603db00a780c02) (based on LLVM 9.0.0)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/sdi/src/llvm-toolchain/install/bin

$ mylinker --version
LLD 9.0.0 (git://github.com/llvm/llvm-project
e82a53603ae3fed2215a44b5ac603db00a780c02) (compatible with GNU
linkers)

With each run (changing W=...), I stopped my build-script manually,
that's why the numbers differ.

[ NO W-N ]

sdi@iniza:~/src/linux-kernel$ for i in Wno-initializer-overrides
Wno-format Wno-sign-compare Wno-format-zero-length ; do echo [ $i ] ;
grep $i build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt_NO-W-N | wc -l ;
done
[ Wno-initializer-overrides ]
178
[ Wno-format ]
178
[ Wno-sign-compare ]
178
[ Wno-format-zero-length ]
178

[ W=1 ]

sdi@iniza:~/src/linux-kernel$ for i in Wno-initializer-overrides
Wno-format Wno-sign-compare Wno-format-zero-length ; do echo [ $i ] ;
grep $i build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt_W-1 | wc -l ; done
[ Wno-initializer-overrides ]
0
[ Wno-format ]
169
[ Wno-sign-compare ]
169
[ Wno-format-zero-length ]
0

[ W=2 ]

sdi@iniza:~/src/linux-kernel$ for i in Wno-initializer-overrides
Wno-format Wno-sign-compare Wno-format-zero-length ; do echo [ $i ] ;
grep $i build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt_W-2 | wc -l ; done
[ Wno-initializer-overrides ]
129
[ Wno-format ]
129
[ Wno-sign-compare ]
129
[ Wno-format-zero-length ]
129

[ W=3 ]

sdi@iniza:~/src/linux-kernel$ for i in Wno-initializer-overrides
Wno-format Wno-sign-compare Wno-format-zero-length ; do echo [ $i ] ;
grep $i build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt_W-3 | wc -l ; done
[ Wno-initializer-overrides ]
114
[ Wno-format ]
114
[ Wno-sign-compare ]
114
[ Wno-format-zero-length ]
114

[ W=4 ]

sdi@iniza:~/src/linux-kernel$ for i in Wno-initializer-overrides
Wno-format Wno-sign-compare Wno-format-zero-length ; do echo [ $i ] ;
grep $i build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt_W-4 | wc -l ; done
[ Wno-initializer-overrides ]
495
[ Wno-format ]
495
[ Wno-sign-compare ]
495
[ Wno-format-zero-length ]
495

W=1 is not passing -Wno-initializer-overrides and -Wno-format-zero-length.
Unsure if other KBUILD_CFLAGS  in W=1 disables them.

So, if it is desired to pass the CLANG extrawarn compiler-options to
all W=... then I ask myself why the CLANG block is in the W=1 block
only?
So if CLANG extrawarn options are independent of any W=... make-option
then I prefer to put it in a seperate block with an appropriate
comment.

According to the commit message W=4 is unsupported, but I can do a
'make V=1 W=4'.
If I pass an unsupported value to W=... then I expect an warning or
info or exit or whatever.

My €0,02.

- Sedat -
Sedat Dilek Aug. 29, 2019, 9:54 a.m. UTC | #9 
> So, if it is desired to pass the CLANG extrawarn compiler-options to

> all W=... then I ask myself why the CLANG block is in the W=1 block

> only?

> So if CLANG extrawarn options are independent of any W=... make-option

> then I prefer to put it in a seperate block with an appropriate

> comment.

>


Maybe something like that (on top of the two patches I had sent).

From: Sedat Dilek <sedat.dilek@credativ.de>

Date: Thu, 29 Aug 2019 11:35:21 +0200
Subject: [PATCH 3/3] kbuild: Move extra warnings for Clang

---
 Documentation/kbuild/kbuild.rst |  5 +++--
 scripts/Makefile.extrawarn      | 21 ++++++++++-----------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
index 3e65d32af875..fa9772ae2367 100644
--- a/Documentation/kbuild/kbuild.rst
+++ b/Documentation/kbuild/kbuild.rst
@@ -245,10 +245,11 @@ KBUILD_EXTRA_CC_CHECKS
 ------------------------------
 If enabled over the make command line with "W=...", it turns on additional
 compiler warning options like -Wmissing-declarations for more extensive
-build-time checking. For more details see <scripts/Makefile.extrawarn>.
+build-time checking.
+Some extra warning options are set for all W=... settings when using Clang.
+For more details see <scripts/Makefile.extrawarn>.

 W=1 - warnings that may be relevant and does not occur too often
-W=1 - also stops suppressing some warnings
 W=2 - warnings that occur quite often but may still be relevant
 W=3 - the more obscure warnings, can most likely be ignored

diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 72677ee9f202..86c0f8ae7e35 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -5,6 +5,16 @@

 KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)

+#
+# W=... - stops suppressing some warnings when using Clang
+#
+ifdef CONFIG_CC_IS_CLANG
+KBUILD_CFLAGS += -Wno-initializer-overrides
+KBUILD_CFLAGS += -Wno-format
+KBUILD_CFLAGS += -Wno-sign-compare
+KBUILD_CFLAGS += -Wno-format-zero-length
+endif
+
 ifeq ("$(origin W)", "command line")
   export KBUILD_EXTRA_CC_CHECKS := $(W)
 endif
@@ -30,17 +40,6 @@ KBUILD_CFLAGS += -Wno-sign-compare

 KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1

-else
-
-# W=1 - also stops suppressing some warnings
-
-ifdef CONFIG_CC_IS_CLANG
-KBUILD_CFLAGS += -Wno-initializer-overrides
-KBUILD_CFLAGS += -Wno-format
-KBUILD_CFLAGS += -Wno-sign-compare
-KBUILD_CFLAGS += -Wno-format-zero-length
-endif
-
 endif

 #
-- 
2.20.1

- Sedat -
Masahiro Yamada Aug. 29, 2019, 5:56 p.m. UTC | #10 
On Wed, Aug 28, 2019 at 11:19 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>

> On Wed, Aug 28, 2019 at 9:20 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:

> >

> > On Wed, Aug 28, 2019 at 7:55 AM Masahiro Yamada

> > <yamada.masahiro@socionext.com> wrote:

> > >

> > > Instead of the warning-[123] magic, let's accumulate compiler options

> > > to KBUILD_CFLAGS directly as the top Makefile does. I think this makes

> > > easier to understand what is going on in this file.

> > >

> > > This commit slightly changes the behavior, I think all of which are OK.

> > >

> > > [1] Currently, cc-option calls are needlessly evaluated. For example,

> > >       warning-3 += $(call cc-option, -Wpacked-bitfield-compat)

> > >     needs evaluating only when W=3, but it is actually evaluated for

> > >     W=1, W=2 as well. With this commit, only relevant cc-option calls

> > >     will be evaluated. This is a slight optimization.

> > >

> > > [2] Currently, unsupported level like W=4 is checked by:

> > >       $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)

> > >     This will no longer be checked, but I do not think it is a big

> > >     deal.

> > >

> >

> > Hi Masahiro Yamada,

> >

> > thanks for your patch series.

> >

> > If KBUILD_ENABLE_EXTRA_GCC_CHECKS does extra(-warning)-checks for GCC and Clang,

> > please rename the Kconfig into...


You repeatedly mentioned "Kconfig" in your posts,
where there is nothing related to Kconfig.


> >

> > KBUILD_ENABLE_EXTRA_CC_CHECKS


You missed the fact this is already used
not only for C compilers, but also for Device Tree compiler.
(see scripts/Makefile.lib)

One more thing, this is the environment variable
that Kbuild officially supports.
Keeping the backward compatibility is must.


When I mentioned to rename this before,
Arnd suggested to keep it as is.
https://patchwork.kernel.org/patch/10172331/#21385013

I do not know whether he is still planning that rework, though.


> > ...or something similiar (and maybe with some notes in its Kconfig help-text?).


What did you mean by "Kconfig help-text" ?



> >

>

> I have tested both patches against recent kbuild-next and can boot on

> bare metal with clang.

>

> I have *not* passed any W= to my make, but I see that clang's W=1

> kbuild-cflags are active.

>

> [ scripts/Makefile.extrawarn ]

>

> ifeq ("$(origin W)", "command line")

>   export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W)

> endif

>

> #

> # W=1 - warnings that may be relevant and does not occur too often

> #

> ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)

> [ ... ]

> KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1

>

> else

>

> # W=1 also stops suppressing some warnings

>

> ifdef CONFIG_CC_IS_CLANG

> KBUILD_CFLAGS += -Wno-initializer-overrides

> KBUILD_CFLAGS += -Wno-format

> KBUILD_CFLAGS += -Wno-sign-compare

> KBUILD_CFLAGS += -Wno-format-zero-length

> endif # CONFIG_CC_IS_CLANG

>

> endif # KBUILD_ENABLE_EXTRA_GCC_CHECKS

>

> These clang KBUILD_CFLAGS are active independently of passing W=1.

>

> $ grep '\-Wno-initializer-overrides'

> build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt | wc -l

> 27195

>

> So the above comment is misleading?

>

> Is W=1 activated by default?

>

> Or do I miss something?



I won't comment back to your long analysis.

Instead, I will post v2.
I hope you will notice something.





> [ Documentation/kbuild/kbuild.rst ]

>

> KBUILD_ENABLE_EXTRA_GCC_CHECKS

> ------------------------------

> If enabled over the make command line with "W=1", it turns on additional

> gcc -W... options for more extensive build-time checking.

>

> What about?

>

> KBUILD_CC_EXTRA_CHECKS (or KBUILD_EXTRA_CC_CHECKS)

> ------------------------------

> If enabled over the make command line with "W=...", it turns on additional

> compiler warning options like -Wmissing-declarations for more extensive

> build-time checking. For more details see <Documentation/kbuild/kbuild.rst>.

>

> W=1 - warnings that may be relevant and does not occur too often

> W=1 - also stops suppressing some warnings

> W=2 - warnings that occur quite often but may still be relevant

> W=3 - the more obscure warnings, can most likely be ignored

>

> - Sedat -




--
Best Regards

Masahiro Yamada
diff mbox series

Patch

diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index a74ce2e3c33e..1fa53968e292 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -1,14 +1,6 @@ 
 # SPDX-License-Identifier: GPL-2.0
 # ==========================================================================
-#
 # make W=... settings
-#
-# W=1 - warnings that may be relevant and does not occur too often
-# W=2 - warnings that occur quite often but may still be relevant
-# W=3 - the more obscure warnings, can most likely be ignored
-#
-# $(call cc-option, -W...) handles gcc -W.. options which
-# are not supported by all versions of the compiler
 # ==========================================================================
 
 KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)
@@ -17,58 +9,68 @@  ifeq ("$(origin W)", "command line")
   export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W)
 endif
 
-ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS
-warning-  := $(empty)
+#
+# W=1 - warnings that may be relevant and does not occur too often
+#
+ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
 
-warning-1 := -Wextra -Wunused -Wno-unused-parameter
-warning-1 += -Wmissing-declarations
-warning-1 += -Wmissing-format-attribute
-warning-1 += -Wmissing-prototypes
-warning-1 += -Wold-style-definition
-warning-1 += -Wmissing-include-dirs
-warning-1 += $(call cc-option, -Wunused-but-set-variable)
-warning-1 += $(call cc-option, -Wunused-const-variable)
-warning-1 += $(call cc-option, -Wpacked-not-aligned)
-warning-1 += $(call cc-option, -Wstringop-truncation)
+KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter
+KBUILD_CFLAGS += -Wmissing-declarations
+KBUILD_CFLAGS += -Wmissing-format-attribute
+KBUILD_CFLAGS += -Wmissing-prototypes
+KBUILD_CFLAGS += -Wold-style-definition
+KBUILD_CFLAGS += -Wmissing-include-dirs
+KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable)
+KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable)
+KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned)
+KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)
 # The following turn off the warnings enabled by -Wextra
-warning-1 += -Wno-missing-field-initializers
-warning-1 += -Wno-sign-compare
-
-warning-2 += -Wcast-align
-warning-2 += -Wdisabled-optimization
-warning-2 += -Wnested-externs
-warning-2 += -Wshadow
-warning-2 += $(call cc-option, -Wlogical-op)
-warning-2 += -Wmissing-field-initializers
-warning-2 += -Wsign-compare
-warning-2 += $(call cc-option, -Wmaybe-uninitialized)
-warning-2 += $(call cc-option, -Wunused-macros)
-
-warning-3 := -Wbad-function-cast
-warning-3 += -Wcast-qual
-warning-3 += -Wconversion
-warning-3 += -Wpacked
-warning-3 += -Wpadded
-warning-3 += -Wpointer-arith
-warning-3 += -Wredundant-decls
-warning-3 += -Wswitch-default
-warning-3 += $(call cc-option, -Wpacked-bitfield-compat)
-
-warning := $(warning-$(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
-warning += $(warning-$(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
-warning += $(warning-$(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
-
-ifeq ("$(strip $(warning))","")
-        $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)
-endif
+KBUILD_CFLAGS += -Wno-missing-field-initializers
+KBUILD_CFLAGS += -Wno-sign-compare
 
-KBUILD_CFLAGS += $(warning)
 else
 
+# W=1 also stops suppressing some warnings
+
 ifdef CONFIG_CC_IS_CLANG
 KBUILD_CFLAGS += -Wno-initializer-overrides
 KBUILD_CFLAGS += -Wno-format
 KBUILD_CFLAGS += -Wno-sign-compare
 KBUILD_CFLAGS += -Wno-format-zero-length
 endif
+
+endif
+
+#
+# W=2 - warnings that occur quite often but may still be relevant
+#
+ifneq ($(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+
+KBUILD_CFLAGS += -Wcast-align
+KBUILD_CFLAGS += -Wdisabled-optimization
+KBUILD_CFLAGS += -Wnested-externs
+KBUILD_CFLAGS += -Wshadow
+KBUILD_CFLAGS += $(call cc-option, -Wlogical-op)
+KBUILD_CFLAGS += -Wmissing-field-initializers
+KBUILD_CFLAGS += -Wsign-compare
+KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized)
+KBUILD_CFLAGS += $(call cc-option, -Wunused-macros)
+
+endif
+
+#
+# W=3 - the more obscure warnings, can most likely be ignored
+#
+ifneq ($(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+
+KBUILD_CFLAGS += -Wbad-function-cast
+KBUILD_CFLAGS += -Wcast-qual
+KBUILD_CFLAGS += -Wconversion
+KBUILD_CFLAGS += -Wpacked
+KBUILD_CFLAGS += -Wpadded
+KBUILD_CFLAGS += -Wpointer-arith
+KBUILD_CFLAGS += -Wredundant-decls
+KBUILD_CFLAGS += -Wswitch-default
+KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat)
+
 endif