Re: [PATCH 3/3] kbuild: add -Wno-unused-but-set-variable compiler flag unconditionally

[Nick Desaulniers]

On Mon, Oct 1, 2018 at 3:24 PM Masahiro Yamada
<yamada.masahiro@xxxxxxxxxxxxx> wrote:
>
> 2018å10æ2æ(ç) 4:58 Nick Desaulniers <ndesaulniers@xxxxxxxxxx>:
> >
> > On Mon, Oct 1, 2018 at 12:02 PM Masahiro Yamada
> > <yamada.masahiro@xxxxxxxxxxxxx> wrote:
> > >
> > > Hi Nick,
> > >
> > > 2018å10æ2æ(ç) 2:18 Nick Desaulniers <ndesaulniers@xxxxxxxxxx>:
> > > >
> > > > On Mon, Oct 1, 2018 at 2:45 AM Masahiro Yamada
> > > > <yamada.masahiro@xxxxxxxxxxxxx> wrote:
> > > > >
> > > > > We have raised the compiler requirement from time to time.
> > > > > With commit cafa0010cd51 ("Raise the minimum required gcc version
> > > > > to 4.6"), the minimum for GCC is 4.6 now.
> > > > >
> > > > > This flag was added by GCC 4.6, and it is recognized by Clang and
> > > > > ICC as well.
> > > >
> > > > This doesn't seem to be the case for Clang:
> > > > https://godbolt.org/z/qesF5o
> > > >
> > > > Nacked-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> > >
> > > Hmm, I tested this patch with pre-built Clang 6.0.1 / 7.0.0
> > > downloaded from http://releases.llvm.org/download.html
> > >
> > > Was this option dropped by clang 8 ?
> >
> > From the Godbolt link above, it seems all versions of Clang do not
> > recognize this flag.  It doesn't look like the kernel sets
> > -Wno-unknown-warning-option to prevent this warning.  Can you please
> > triple check that compiling with Clang and this patch causes no
> > warnings?  I suspect something might be amiss then if this patch
> > doesn't produce warnings with Clang, since on the smaller Godbolt
> > example it does.
>
>
> I got it.
>
> Clang does NOT recognize -Wno-unused-but-set-variable.
>
> But, the code I changed is not parsed for clang.
> That is why I did not see any problem with this patch.
>
> See the following code.
>
>
>
> ifeq ($(cc-name),clang)
> KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,)
> 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)
> # Quiet clang warning: comparison of unsigned expression < 0 is always false
> KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
> # CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the
> # source of a reference will be _MergedGlobals and not on of the
> whitelisted names.
> # See modpost pattern 2
> KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
> KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
> 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

Ah! I see, it's in the else part of an if clang check.  Ok, in that
case I think the commit message can be cleaned up with this detail,
then I'd be happy to sign off on it.  Sorry I didn't notice that
before.

> endif
>
>
>
>
>
> --
> Best Regards
> Masahiro Yamada



-- 
Thanks,
~Nick Desaulniers