top | item 15400396

Useful GCC warning options not enabled by -Wall -Wextra

288 points| rbanffy | 8 years ago |kristerw.blogspot.com | reply

166 comments

order
[+] vog|8 years ago|reply
Nice summary. Out of curiosity:

Why are those warnings not automatically included in "-Wall -Wextra"? Is there any issue with enabling them? Are these somewhat experimental?

How do these GCC warning compare to Clang warnings? Do these warnings exist there, too?

[+] jcelerier|8 years ago|reply
> Why are those warnings not automatically included in "-Wall -Wextra"? Is there any issue with enabling them? Are these somewhat experimental?

Because tons of people keep complaining "muh my build breaks if they enable new warnings". Dammit, idiots, if your build break there's a good chance your software is already broken too; that's the point of breaking the build: making bugs visible so that you have to fix them.

[+] asveikau|8 years ago|reply
I'll bite. It seems like a number of these would complain about perfectly benign code.

Here's an example I can think of:

    b = (a == 0) ? 42 : 42;
A lot of readers are probably thinking this line is absurd and why on earth would a compiler not yell at you and call you a bad human being for writing this?

But let's be creative ... What if each of those 42s are expansions of different macros that vary based on an ifdef? You could have one platform or configuration where it expands to ? 43 : 42, and another where it expands to ? 42 : 42. Since the preprocessor does those expansions early, your compiler doesn't know the difference at the time of generating the warning. I just confirmed this in a small program, replacing the first 42 with FOO, the 2nd with BAR, and #defining them both to 42:

    warn.c:9:23: warning: this condition has identical branches [-Wduplicated-branches]
        b = (a == 0) ? FOO : BAR;
Overall I would say items in this list are not equally bad. Some of them unambiguously you want to flag. Others are things that people might do intentionally.
[+] wongarsu|8 years ago|reply
Most of them sound like they might have a large amount of false positives if turned on for an existing code base (a "false positive" in this case would be a case where you would dismiss the warning after investigation). A large amount of warnings that don't point to "actual problems" causes people to ignore or overlook the actually important warnings.

(I'm not saying that these warnings are pointless, but you can have a bug-free code base that triggers a lot of these)

[+] dmm|8 years ago|reply
> Why are those warnings not automatically included

People who do production builds with -Wall and -Werror ruin it for the rest of us.

[+] wyldfire|8 years ago|reply
It's a design goal to be flag-compatible with gcc, so in general all these warnings are or will be supported in clang.
[+] breadbox|8 years ago|reply
> Why are those warnings not automatically included in "-Wall -Wextra"?

Not all of them are actually errors, maybe depending on your style. For example, shadowing a variable is a well-defined feature of the C language, and so some code patterns take advantage of that fact. Other people avoid shadowing like the plague.

[+] richardwhiuk|8 years ago|reply
I suspect macro expansion causes too many false positives for many of these.
[+] gciruelos|8 years ago|reply
some have a lot of false positives (even some in -Wextra have had false positives due to bugs), so if you compile with -Werror (as you should do) it can be troublesome.
[+] minexew|8 years ago|reply
My absolute favorite for modern C++ is

-Werror=switch

Using this, you can enforce that a switch statement over an enum value is complete (covers all cases) simply by omitting the default case.

[+] asveikau|8 years ago|reply
You're not the first person I've seen suggest that this condition is worth flagging, but I don't completely understand why. People who conform to such a style then go on to add a "default" case that is a no-op, which I guess is more explicit, but also feels a little tedious and verbose. Switch statements in C can do all sorts of funky things (vide: Duff's device) so I think omitting a case label for an enum value feels like a much lesser crime. What if you just don't have meaningful things to do for that enum value?
[+] usernam|8 years ago|reply
Warnings are good, but it's a pity there's no good warning suppression method which is compact (and does not require to disable the warning on the entire file).

People generally "fix" warnings by writing equally useless statements like "x = x" (common for -Wunused). I've seen codebases which aim for zero warnings, but with -Wextra/-Weverything it's just silly. Warnings are supposed to be just that: warnings.

I'd rather want a clean way to suppress the warning and mark it as such instead, so that's hidden by default when the programmer takes note of it.

We have push/pop pragmas, but they are even worse readability-wise. I'd rather have a neighboring comment with dedicated syntax instead.

[+] chocolatebunny|8 years ago|reply
Most warnings are just warnings. A warning like "implicit cast from pointer to integer" is a red flag (I got bit by this one). I think there are a few others that are always worth taking a look at.

I like your idea of marking warnings. Warnings should be treated like the messages from any static analysis tool. A lot of times they are worth at least looking at but you should be able to mark them as "not a problem" if you think you know better. That way you will always look at new warnings and not end up ignoring all warnings because there are too many that are irrelevant.

[+] Asooka|8 years ago|reply
For -Wunused specifically, the correct idiom is "(void)x;". It's a no-op that marks x as having been used. Yes, in an ideal world you wouldn't have to do that, but sometimes you're overriding a virtual method and genuinely don't need to use one of the parameters.
[+] zakk|8 years ago|reply
I wonder why -Wnull-dereference is not default.
[+] maxlybbert|8 years ago|reply
I would guess "because people complain when it gives warnings related to macros." That's almost always the answer, especially for C.
[+] rocqua|8 years ago|reply
I wonder the same. Especially because this might be the same code that deletes null-checks as an 'optimization due to undefined behavior' as discussed here [1].

I recall a bug report with quite the flame war in it, but couldn't find it.

[1] https://kb.isc.org/article/AA-01167

[+] kazinator|8 years ago|reply
I usually add these old standbys:

-Wimplicit-function-declaration: warn when a function is used without being declared (not needed in -std=c99 mode, but I write in c90). Catches functions not being declared in header files, which would result in broken type checking.

-Wmissing-prototypes: external function is defined without a previous prototype declaration. This catches functions that should be static: you fix it by making the function static, or else by prototyping it (in a header) so that it is properly public.

-Wstrict-prototypes: catches accidental old-style definitions or declarations, like void foo(), which isn't a prototype.

[+] wyldfire|8 years ago|reply
> Catches functions not being declared in header files, which would result in broken type checking.

Actually it could be much more interesting than that -- the implicit function decl will be `int foo(int)` so you risk your compiler pushing the wrong args on the stack.

[+] chocolatebunny|8 years ago|reply
I'm confused by -Wrestrict. Isn't the whole point of the restrict keyword aliasing like that? what's the point of restrict if there is no enforcement by default?
[+] zuzun|8 years ago|reply
The restrict keyword is supposed to enable optimizations, not to increase safety. You promise you won't access the memory location through different pointers and the compiler throws some precautions overboard.
[+] jerf|8 years ago|reply
Honest question: Are these warnings smart enough to fire only against "raw" source code? Many of the first few I could see happening from macro expansion and it would be nice to just let those go without warning and let the optimizations hopefully pick up the pieces. Or is that some of the false positives these bring up that were referenced?
[+] Ded7xSEoPKYNsDd|8 years ago|reply
Unless you enable -Wsystem-headers, GCC doesn't warn about things in system headers, so for noisy headers it's easiest to mark them as such (with -isystem <dir>).
[+] mrich|8 years ago|reply
clang implements it that way, most (all?) warnings from macro expansion are omitted. Maybe gcc does the same for some warnings, but I've only noticed it for clang.

I find clang's behavior particularly dangerous for warnings referring to removal of null pointer checks, which are undefined behavior (if (this != nullptr)...)

There should perhaps be an option to control this.

[+] conistonwater|8 years ago|reply
I think one of the most amazing things I ever found out about clang is that you can change warning settings directly inside the file with special #pragma's, without having to modify anything in the build process itself.
[+] Shorel|8 years ago|reply
I find more useful to make the program also warning free in clang instead of adding such options.
[+] jhasse|8 years ago|reply
clang just adds more stuff to -Wextra but you can get most of the warnings in GCC too.
[+] rurban|8 years ago|reply
Easier would be to use AX_COMPILER_FLAGS from https://www.gnu.org/software/autoconf-archive/ax_compiler_fl...

This probes for:

    checking whether C compiler accepts -Werror=unknown-warning-option... yes
    checking whether C compiler accepts -Wno-suggest-attribute=format... yes
    checking whether C compiler accepts -fno-strict-aliasing... yes
    checking whether C compiler accepts -Wall... yes
    checking whether C compiler accepts -Wextra... yes
    checking whether C compiler accepts -Wundef... yes
    checking whether C compiler accepts -Wnested-externs... yes
    checking whether C compiler accepts -Wwrite-strings... yes
    checking whether C compiler accepts -Wpointer-arith... yes
    checking whether C compiler accepts -Wmissing-declarations... yes
    checking whether C compiler accepts -Wmissing-prototypes... yes
    checking whether C compiler accepts -Wstrict-prototypes... yes
    checking whether C compiler accepts -Wredundant-decls... yes
    checking whether C compiler accepts -Wno-unused-parameter... yes
    checking whether C compiler accepts -Wno-missing-field-initializers... yes
    checking whether C compiler accepts -Wdeclaration-after-statement... yes
    checking whether C compiler accepts -Wformat=2... yes
    checking whether C compiler accepts -Wold-style-definition... yes
    checking whether C compiler accepts -Wcast-align... yes
    checking whether C compiler accepts -Wformat-nonliteral... yes
    checking whether C compiler accepts -Wformat-security... yes
    checking whether C compiler accepts -Wsign-compare... yes
    checking whether C compiler accepts -Wstrict-aliasing... yes
    checking whether C compiler accepts -Wshadow... yes
    checking whether C compiler accepts -Winline... yes
    checking whether C compiler accepts -Wpacked... yes
    checking whether C compiler accepts -Wmissing-format-attribute... yes
    checking whether C compiler accepts -Wmissing-noreturn... yes
    checking whether C compiler accepts -Winit-self... yes
    checking whether C compiler accepts -Wredundant-decls... (cached) yes
    checking whether C compiler accepts -Wmissing-include-dirs... yes
    checking whether C compiler accepts -Wunused-but-set-variable... no
    checking whether C compiler accepts -Warray-bounds... yes
    checking whether C compiler accepts -Wimplicit-function-declaration... yes
    checking whether C compiler accepts -Wreturn-type... yes
    checking whether C compiler accepts -Wswitch-enum... yes
    checking whether C compiler accepts -Wswitch-default... yes
    checking whether C compiler accepts -Wno-suggest-attribute=format... no
    checking whether C compiler accepts -Wno-error=unused-parameter... yes
    checking whether C compiler accepts -Wno-error=missing-field-initializers... yes
    checking whether C compiler accepts -Werror=unknown-warning-option... (cached) yes
    checking whether the linker accepts -Wl,--no-as-needed... no
    checking whether the linker accepts -Wl,--fatal-warnings... no
    checking whether the linker accepts -Wl,-fatal_warnings... yes
    checking whether the linker accepts -Wl,-fatal_warnings... yes