Re: [PATCH] kbuild: clang: Disable -Wunused-const-variable warnings
2018-01-30 2:08 GMT+09:00 Prasad Sodagudi : > Currently -Wunused-variable warnings are disabled with > clang and with gcc -Wunused-variable warnings are > enabled, with this setting all unused local variables > would be warned in clang as well. > > Disable -Wunused-const-variable warnings instead of > disabling -Wunused-variable warnings, So that in both > clang and GCC -Wunused-const-variable gets disabled. > > Signed-off-by: Prasad Sodagudi The code is OK, but I'd like to make the log even clearer. The commit subject "kbuild: clang: Disable -Wunused-const-variable warnings" sounds confusing. -Wunused-const-variable was already disabled for clang because it was implied by Wno-unused-variable. So, this patch is effectively enabling -Wunused-variable, right? How about something like follows? ->8--- kbuild: clang: disable unused variable warnings only when constant Currently, GCC disables -Wunused-const-variable, but not -Wunused-variable, so warns unused variables if they are non-constant. While, Clang does not warn unused variables at all regardless of the const qualifier because -Wno-unused-const-variable is implied by the stronger option -Wno-unused-variable. Disable -Wunused-const-variable instead of -Wunused-variable so that GCC and Clang work in the same way. --->8-- If it is tedious to resend, shall I reword the log locally? Let me your thought. > --- > Makefile | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 339397b..4b6c8e2 100644 > --- a/Makefile > +++ b/Makefile > @@ -698,7 +698,6 @@ KBUILD_CFLAGS += $(stackp-flag) > > ifeq ($(cc-name),clang) > KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) > KBUILD_CFLAGS += $(call cc-disable-warning, gnu) > KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) > @@ -716,9 +715,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 += $(call cc-disable-warning, unused-but-set-variable) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > endif > > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > ifdef CONFIG_FRAME_POINTER > KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls > else > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na > Linux Foundation Collaborative Project > -- Best Regards Masahiro Yamada
Re: [PATCH] kbuild: clang: Disable -Wunused-const-variable warnings
Hi Prasad, On Mon, Jan 29, 2018 at 09:08:00AM -0800, Prasad Sodagudi wrote: > --- a/Makefile > +++ b/Makefile > @@ -698,7 +698,6 @@ KBUILD_CFLAGS += $(stackp-flag) > > ifeq ($(cc-name),clang) > KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) > KBUILD_CFLAGS += $(call cc-disable-warning, gnu) > KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) > @@ -716,9 +715,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 += $(call cc-disable-warning, unused-but-set-variable) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > endif > > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > ifdef CONFIG_FRAME_POINTER > KBUILD_CFLAGS+= -fno-omit-frame-pointer -fno-optimize-sibling-calls > else Ah, much more obvious now, thanks :-) Segher
Re: [PATCH] kbuild: clang: Disable -Wunused-const-variable warnings
On 2018-01-28 08:22, Segher Boessenkool wrote: On Fri, Jan 26, 2018 at 04:59:46PM -0800, Prasad Sodagudi wrote: Disable -Wunused-const-variable warnings instead of disabling -Wunused-variable warnings, So that in both clang and GCC -Wunused-const-variable gets disabled. Why would you disable -Wunused-const-variable on GCC? You do not explain why. Hi Segher, Please check below discussion with Greg and Masahiro about "unused-variable". https://lkml.org/lkml/2017/12/18/697 Please check this link, "unused-const-variable" warning is already disabled for GCC. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Makefile?h=v4.15#n719 Below commit is disabling the "unused-const-variable" for GCC. commit - c9c6837d39311b0c - "kbuild: move -Wunused-const-variable to W=1 warning level" Currently "unused-variable" warnings are disabled for clang and not for the gcc. Disabling of "unused-variable" warning also disables "unused-const-variable" warning. To match same settings between clang and gcc, disabling "unused-const-variable" warnings for clang as well. So with this patch, keeping the same settings for GCC and CLANG with respect to "unused-const-variable". -Thanks, Prasad Segher -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, Linux Foundation Collaborative Project
Re: [PATCH] kbuild: clang: Disable -Wunused-const-variable warnings
On Fri, Jan 26, 2018 at 04:59:46PM -0800, Prasad Sodagudi wrote: > Disable -Wunused-const-variable warnings instead of > disabling -Wunused-variable warnings, So that in both > clang and GCC -Wunused-const-variable gets disabled. Why would you disable -Wunused-const-variable on GCC? You do not explain why. Segher
