Date: Mon, 15 Apr 2013 21:21:20 +0200 From: Ed Schouten <ed@80386.nl> To: freebsd-toolchain@freebsd.org Subject: [Patch] Add -Wmissing-variable-declarations to WARNS=6 Message-ID: <CAJOYFBDT2FkX-w940fZXiebbdaSuRMDMLdfoNJ=0mFDi6ig5iQ@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
Hi folks,
Quite some time ago I spent some time hacking on Clang to add support
for a new warning flag, called -Wmissing-variable-declarations. To
summarize, it lets the compiler trigger a warning if you write the
following piece of code (at the global scope):
int i;
Huh? Why should it do that? Well, people should typically write the
following instead:
extern int i;
...
int i;
Or:
static int i;
Essentially, it is the same as -Wmissing-prototypes, but then for
global variables. By enabling this compiler warning, I managed to
improve our code quality a bit:
- In some cases binaries became smaller as I added static keywords, as
either the number of symbols in the header decreased, or the compiler
could do some magic optimisation that was otherwise not possible. Also
in many cases, I observed that the compiler can now more accurately
derive which variables are not written to, meaning they are stored in
read-only sections.
- In some cases I found truly harmful bugs. For example, multiple C
source files in one binary would do a tentative definition of a
variable under the same name, which got shared, even though they had
to remain separated.
- In other cases I found harmless bugs. For example:
enum { FOO, BAR } baz;
Where the following was meant:
enum baz { FOO, BAR };
Or simply:
enum { FOO, BAR };
Because I think it's a nice compiler warning flag, I am thinking about
adding it to WARNS=6. Patch:
http://80386.nl/pub/wmissing-variable-declarations.txt
I thought I'd send this email, because I've got a couple of questions:
- Essentially any piece of code that uses Yacc/Lex won't build because
of this flag, simply because the C code that's printed out by these
tools is too horrible and cannot easily be fixed without changing the
consumers as well. To work around this, I've added a
NO_WMISSING_VARIABLE_DECLARATIONS=. Any objections?
- Is the logic I added to share/mk/bsd.sys.mk all right? Do I check
the right conditions?
- Is it okay if I push in the changes to the CDDL code? I already sent
a patch for this to the Illumos folks, but so far I haven't received a
reply yet:
https://www.illumos.org/issues/3700
Enabling this for the kernel would be awesome, but unfortunately this
would be pretty hard. We quite often have that we create non-static
global variables that are used elsewhere (e.g. through symbol lookups
or by placing them in different sections).
Cheers,
--
Ed Schouten <ed@80386.nl>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJOYFBDT2FkX-w940fZXiebbdaSuRMDMLdfoNJ=0mFDi6ig5iQ>