Date: Wed, 6 Jan 2021 13:34:30 -0800 From: Ryan Libby <rlibby@freebsd.org> To: Warner Losh <imp@bsdimp.com> Cc: FreeBSD Hackers <freebsd-hackers@freebsd.org> Subject: Re: clang vs gcc warning flags Message-ID: <CAHgpiFw7rsqAQ-M%2BnsXzvFCLPgEasqnMjkEyMjw1zaAAgQrsGA@mail.gmail.com> In-Reply-To: <CANCZdfpHC6ovmP9syA=8V-92AjdYyRngO0o8m44JwqEtMkNzDA@mail.gmail.com> References: <CAHgpiFwK77o3J6Bm_3GAQRGAJz70=n8Z8bqqLirahL_gqXCM3w@mail.gmail.com> <CANCZdfpHC6ovmP9syA=8V-92AjdYyRngO0o8m44JwqEtMkNzDA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Jan 6, 2021 at 12:38 PM Warner Losh <imp@bsdimp.com> wrote:
>
>
>
> On Wed, Jan 6, 2021 at 12:53 PM Ryan Libby <rlibby@freebsd.org> wrote:
>>
>> One of the more annoying things about keeping the gcc build going is the
>> set of warnings that gcc acts on but clang only recognizes for
>> compatibility. As a common example, -Wredundant-decls has no effect
>> in clang, but will break the gcc build. There are a couple dozen such
>> flags [1][2], and a few of them are in our default set of warnings, such
>> as in sys/conf/kern.mk, these ones in CWARNFLAGS:
>>
>> - Wredundant-decls
>> - Wnested-externs
>> - Wmissing-include-dirs
>>
>> additionally some warnings are explicitly disabled for clang, but not
>> for gcc in CWARNEXTRA:
>>
>> - Wempty-body
>> - Wunused-function
>>
>> Similarly, in share/mk/bsd.sys.mk:
>>
>> - Winline (although, Wno-error'd)
>> - Wnested-externs
>> - Wredundant-decls
>> - Wold-style-definition
>>
>> So I suggest we harmonize these somewhat.
>>
>> - Wnested-externs I just do not understand. We have specified this
>> warning flag for some 25 years but to me it seems completely without
>> value. I suggest we just delete it.
>
>
>
> It's to prevent declaring functions inside of functions. why is that bad?=
It only has scope of the function, and we've decided that's not something =
we want in the tree... I'd say that the fact we don't have any in the tree =
shows that it's useful at its intended purpose.
>
> What code is causing issues here?
>
Well, but do we really care specifically about declaring extern
functions (or variables) inside of functions? Is that worse than the
usual fix, moving them just outside the function to file scope?
Regarding _defining_ functions inside of functions, I agree we don't
want that. But -Wnested-externs doesn't warn about that; gcc produces
no warning for this with -Wnested-externs:
void f(void) { void g(void){}; g(); }
(Gcc produces a warning for that with -pedantic, which we don't use; and
clang produces an error even with no warning flags.)
How I've actually seen "nested externs" used is usually to make
variables visible without including headers, for one reason or another.
Here are a couple examples:
https://cgit.freebsd.org/src/commit/?id=3Dd576ccdf01a4c6f8f02e8ed7e72290c72=
9d68de6
https://cgit.freebsd.org/src/tree/sys/contrib/openzfs/module/zfs/arc.c#n472=
1
As far as I understand, this actually does limit the scope of the
visibility of the extern declaration. I am not an expert in the
standard, I could be mistaken.
I don't want to overstate the problem though. As I said, these are
annoyances, the solutions aren't hard.
>>
>> - Wredundant-decls, I'm not sure about. I have never seen this detect
>> anything that will cause misbehavior, but most of the time that it
>> fires it does indicate some kind of genuine--but harmless--mistake.
>
>
> It also tends to prevent declarations that are the same on some platforms=
, but different on others.
>
>>
>> - Wmissing-include-dirs doesn't seem to occur often and usually
>> indicates a genuine (but again harmless) mistake in a makefile. I
>> think we should keep it.
>
>
> I do too.
>
>>
>> - Wempty-body, Wunused-function. I'm not sure. These are proscriptive
>> about things that are not necessarily problems. We are apparently
>> already clean for them in the kernel gcc build, so perhaps we should
>> enable them for the kernel clang build. In any case, I think we
>> should bring these into agreement between clang and gcc, one way or
>> the other.
>
>
> We do not remove it. We include the warning, but don't make it fatal for =
clang.
>
Yes, "clean" was maybe not the right word. The end result is build
errors with gcc, no build errors with clang. The suggestion is just
that we do the same thing with both compilers, whether that be an error,
or a warning without an error.
> Most of the clang neutering was due to bugs in the early days of clang. I=
'm not sure anybody has systematically reviewed them since then.
>
>> Another sticking point may be contrib software. I think we generally
>> don't want to fail builds of contrib software for things that are
>> ultimately harmless. For bsd.sys.mk this could be accomplished by
>> enabling such warnings only at WARNS >=3D 6. For the kernel, we could
>> come up with some other mechanism.
>
>
> The kernel is different. 'Ultimately harmless' is a value judgement.
>
I largely agree. What I'm hoping to break out of is situations where we
import contrib software and it breaks the gcc build for reasons that
don't affect the functionality of the built code, and then it's a pain
to fix because it's contrib software.
>>
>> I'll put up a review soon for deleting -Wnested-externs unless there are
>> objections. If there is agreement about the others, I'll include those
>> too.
>>
>> Ryan
>>
>> [1] https://clang.llvm.org/docs/DiagnosticsReference.html
>> [2] https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
>> _______________________________________________
>> freebsd-hackers@freebsd.org mailing list
>> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
>> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.or=
g"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAHgpiFw7rsqAQ-M%2BnsXzvFCLPgEasqnMjkEyMjw1zaAAgQrsGA>