Use of -Wextra

[isaacwoods]

I already use the -Wall flag when compiling any C or C++ code. For one particular project, I have no warnings at all with -Wall, so decided to use -Wextra as well to see how many that would create. To my surprise, I had over 1000 new warnings, most of which seemed highly pedantic and not really worth following, to me anyway. From more experienced programmers, is it worth using -Wextra, or is it just too pedantic for words?

[bluemoon]

It depends on what those 1000 new warnings are, and they might indeed worth noticed. Can you show us?
I use -Wall -Wextra and some other flags and happy to see zero warning on my code.

[kzinti]

-Wall -Wextra -Werror is where it's at.

[Rusky]

-std=c11 -Wpedantic -pedantic-errors -Wall -Wextra -Werror

[kzinti]

Rusky: not a big fan of -Wpedantic, but i'd rather have it than no -Wextra =)

[Combuster]

Wextra of its own contains the following bug detections:
-Wtype-limits for comparisons that can never be true.
-Wempty-body for the following:

Code: Select all

while(condition);
{
...
}

-Wmissing-field-initializers for when you initialise the wrong struct
-Woverride-init when you initialise parts of a struct twice.

And style detections
-Wmissing-parameter-type to enforce you to type everything properly.
-Wignored-qualifiers for when your function prototypes look silly
-Wold-style-declaration for when your function prototypes might break in the next C standard
-Wsign-compare when you're too careless about signed and unsigned variables.
-Wunused-parameter when your functions take more variables than they really need (i.e., you have API defects)
-Wunused-but-set-parameter when you have dead code.


In particular -Wsign-compare can be very verbose, especially for less experienced developers, and it's an indicator that you don't think about your types well enough and as a consequence, your C style is not up to par with what it should be. They're also very easily cleaned in the vast majority of cases where they don't actually point to a bug, and there's no reason to allow them to remain. GCC is a very good learning guide in this respect.

[eisdt]

Some warnings are useful, like -Wtype-limits, some are a bit less because, most of the times, they have no real consequences but just signal a possibly faulty logic. -Wextra shows why this code never ends

Code: Select all

for (unsigned i = 0xff; i >= 0; --i) 
               ;

clang warns about that even at -Wall.

[Rusky]

If you're willing to tweak your coding style a little, that warning is also helpful. It detects situations like this:

Code: Select all

while (condition);
{
    ...
}

And you can get rid of false positives by using {} instead of ; for empty loop bodies.

[Candy]

For GCC and Clang you can think of the warning levels as follows:

- No extra warnings: Only the absolutely required warnings. For example (C++), having a function return an object and then not returning anything is not even a warning. It will guaranteed crash if you ever call it, but you don't even get a warning.
- Wall: The "everybody thinks this is a good idea" set of warnings. Very occasionally a warning is added that's only valid 99% of the times and then it's usually removed a release later. If you do not use this, you're pretty much an idiot.
- Wextra: The "These things are highly suspect and should be fixed" set of warnings. You are doing something that, for a healthy code base, is stupid. Fix it. Examples are arguments to functions that you just don't use, local variables that are never used and so on.
- Wpedantic: The "Standard says this should be a warnings, so it is" set of warnings. Personally I like this being on, but I know many people don't want to spend the time to fix these things. Contains warnings that are technically places to improve, but not necessarily a major thing if your code base is not thousands of files.

Source: Lots of work experience, recently enabled -Wextra on a multi-million line of code legacy code base and fixed the +- 6000 warnings coming out of it. Found at least 4 gross errors with it - uninitialized variables, const-reference to temporary being stored as a member, stuff like this.

[willedwards]

I really want to love -Wpedantic -Wall -Wextra and friends but you invariably find yourself trying bumping into this warning:

warning: unused parameter 'args' [-Wunused-parameter]

And so you try and remove the name of the parameter, and:

error: parameter name omitted

There are whole forum topics across the web where people share tips on how to get rid of just this error. And there are others. I cannot remember the details but I once struggled with trying to use %z in printf and get that through some pedantic combination of complaining works-in-c-but-not-c++ thing.

Really every single warning should be suppressible with a pragma. Many are on new compilers, but if you're targeting a variety of compilers all bets are off.

[kscguru]

I really want to love -Wpedantic -Wall -Wextra and friends but you invariably find yourself trying bumping into this warning:

warning: unused parameter 'args' [-Wunused-parameter]

Very true. -Wextra -Wno-error=unused-parameter can help. That -Wno-error= option is the best thing from gcc since -Werror...

Speaking with a background of professional OS codebase ... -Wextra and -Wpedantic don't work at scale. Once you need to target multiple compilers (e.g. gcc, clang, icc, Microsoft, or different versions thereof) with different warnings, you quickly get into situations where the warning workarounds for one compiler induce warnings in other compilers (I'm looking at you, -Wunused-result). But more importantly, professional code tends to build for multiple configurations (release / debug or multi-arch being common), which results in lots of "#ifdef DEBUG / sanity_check(foo); / #endif" clauses[1] - and parameters used only in one configuration are by FAR the most common -Wextra warning. Then you get into a debate about whether you should have configuration-specific APIs with no unused params, or the general API should be a superset of all configurations but some configurations will have unused params, and trying to get both to work simultaneously leads down a path of increasingly opaque workarounds.

Where I'm trying to go with this is that you can definitely make it work when you target only a single thing. -Wextra or -Wpedantic covers things that are usually errors but are correct behavior often enough (e.g. depending on preprocessing) that the cost of silencing warnings for the "correct" cases exceeds the value of fixing the "error" cases. That cost and tradeoff varies tremendously between hobbyist (single configuration / single compiler) and massive professional codebase (many configurations / many compilers), and there is no right answer beyond being as aggressive about not having warnings as you can tolerate.

[1] Many but not all of these can be done via optimizer instead of preprocessor: gcc -DDEBUG=0 or gcc -DDEBUG=1 and "if (DEBUG) sanity_check(foo);"

[thomasloven]

I do

Code: Select all

(void)args;

at the top of my unfinished stub functions.

[kzinti]

#define UNUSED_PARAM(x) (void)x

[Candy]

Speaking with a background of professional OS codebase ... -Wextra and -Wpedantic don't work at scale. Once you need to target multiple compilers (e.g. gcc, clang, icc, Microsoft, or different versions thereof) with different warnings, you quickly get into situations where the warning workarounds for one compiler induce warnings in other compilers (I'm looking at you, -Wunused-result). But more importantly, professional code tends to build for multiple configurations (release / debug or multi-arch being common), which results in lots of "#ifdef DEBUG / sanity_check(foo); / #endif" clauses[1] - and parameters used only in one configuration are by FAR the most common -Wextra warning.

That project I was referring to? It's a 200-man multi-decade project, targeting 3 out of 4 of those compilers, with a stretch of versions for each (currently GCC 4.4 to 5.2, MSVC 2008 to 2013 and Clang 3.2 to 3.. We've recently been working on getting the -Wextra warnings fixed, which lead to a lot of bugs being found. The biggest problems are:

-Wno-unused-local-typedefs for typedefs used as static assert

-Wno-unused-variable, for your aforementioned debug/release (or a second build variation point we have, which also leads to this). This should be fixed though, but it's just a lot of work.

-Wno-unused-parameter, again, should be fixed but it's I think 10000 occurrences, so perhaps when we have time.

-Wno-missing-field-initializers, as it complains about initializing a struct with = {0}. That's just plain not wrong.

Pedantic is something I would not turn on unless all developers believe in it. In short, that only happens on personal projects for me.

Where I'm trying to go with this is that you can definitely make it work when you target only a single thing. -Wextra or -Wpedantic covers things that are usually errors but are correct behavior often enough (e.g. depending on preprocessing) that the cost of silencing warnings for the "correct" cases exceeds the value of fixing the "error" cases. That cost and tradeoff varies tremendously between hobbyist (single configuration / single compiler) and massive professional codebase (many configurations / many compilers), and there is no right answer beyond being as aggressive about not having warnings as you can tolerate.

Yes. If you have a code base you should consider how long you want it to last and how certain you want to be that it works. Typically, when you have technically minded people, they recognize that -Wextra results in many bugs being found and fixed early, and that the investment way outweights the immediate costs of introducing it.

Same thing happened when porting to C++11, or porting to 64-bit. Both resulted in many places being found (but typically only affecting 2-4% of the code base) where bugs had been present, but now are fixed.

[eisdt]

-Wno-unused-variable, for your aforementioned debug/release (or a second build variation point we have, which also leads to this). This should be fixed though, but it's just a lot of work.

-Wno-unused-parameter, again, should be fixed but it's I think 10000 occurrences, so perhaps when we have time.

You can cast to void to shut up those warnings; I consider it pretty ugly though, also because fixing the actual problem is easy.

-Wno-missing-field-initializers, as it complains about initializing a struct with = {0}. That's just plain not wrong.

That's a known and fixed bug. It's perfectly allowed in the standard.