Is it a good idea to eliminate compiler warnings?

Question

In the past I've worked with -Wall and other switches for gcc to eliminate every compiler warning for projects I've been involved in. Similarly, in Perl, I always program with use strict and use warnings (and often -T as well) to try to achieve the best code quality I can. I understand that a few years ago, the Perl porters group worked hard to make perl itself (the Perl interpreter) compile cleanly under gcc with all warnings enabled. Obviously they felt that was a good idea for code quality. I also understand that nowadays Perl programmers have been adding even more warnings to their code with Perl::Critic, which warns them when they violate best practices found in Damian Conway's Perl Best Practices book (and from other sources, I believe).

I always had a good feeling about code that I had cleaned up this way, but sometimes I couldn't avoid the feeling that some of the work was a little wasted. For example, in my intro C classes over a decade ago, I was taught to start my main() function like this:

void main(void) {

This was minimal and could only be used when you weren't returning a value and weren't accessing your arguments. It works just fine in that case, but gcc warnings would let you know that this function really ought to look like:

int main(int args, char* argv) {

I must've typed a couple of hundred unused int args, char* argv lines back in the day. Did I really make my code better, or just wear my fingers down shorter?

Nowadays I'm programming in Java in Eclipse, and our project has tens of thousands of warnings. I'd like to clean them up. Some of them are especially difficult to understand and eliminate, but slowly I'm learning. A few of these I've had to handle with compiler directives to suppress warnings (usually in tiny minimal methods to factor out the bad practice of ignoring warnings), but I'm finding ways to handle those, as well.

Is this worth a programmer's time? Will a project really be much better if you track down every single compiler warning?

If nothing else, it seems like it'd be nice to reduce the number of warnings to zero so that serious warnings wouldn't get lost in the mess.

Note: Duplicate of this question

Answer 1

Yes, definitely. A warning is an indication that something may be wrong in your code. Either you fix it, or you should disable the warning -- if you're not going to act on it, having the warning is just preventing you from detecting when new warnings appear.

Easiest way to get and keep the code warning-free is simply to always compile with -Werror (/WX in Visual Studio) or equivalent.

Answer 2

It will save the compiler the time of writing out the warnings, and make it easier to find potentially hazardous warnings if you reduce the number of frivolous warnings in your code. Is that worth the time it takes to hunt down every warning and fix it? Not usually... but it's worth fixing warnings when they're obvious and easy to fix, or when you need to take the time and change the logic containing the warnings for another reason.

Answer 3

Is this worth a programmer's time? Will a project really be much better if you track down every single compiler warning?

Yes. Even the trivial ones about sign mismatch can have profound impact on code generation and optimizations.

Answer 4

Eclipse, IntelliJ and other modern code quality systems have huge numbers of available warnings.

Cleaning up warnings is a useful practice, if for no other reason than to quiet down the noise. Your code probably already has some of these that are real bugs.

However, many of the warnings are probably bogus. Some warnings from Eclipse are things like micro-optimizations or stylistic nitpicking. This means that some of your cleanups must be tweaking the settings of your tool, rather than code changes or localized suppression directives.

Answer 5

My opinion. Yes.

Like you said at the end. It helps make the real errors more prominent.

When you run a Perl cgi script that outputs warnings on an Apache Server, the warnings get logged in error.log. Why waste the space. Fix the warnings.

Also I think it's a learning experience to better understand the language, and compiler. I didn't realize dynamic scoping was even a feature of Perl until I started using strict.

Answer 6

There are two compliant prototypes for main in C.

int main(void)

and

int main(int argc, char **argv)

void main(void) is not technically correct, although it may be supported by some compilers as an extension to the standard.

So, in your particular case, you can use a short declaration of main and, if compliant, it won't trigger the warning.

Answer 7

Yes, do so..

Just yesterday I did that for a project at work. While removing the warnings I came about two issues that turned out to be real bugs.

It was a C-project, and in one source the sqrt (square root) was called. I got the following warning:

test.c:4: warning: implicit declaration of function 'sqrt'

Turned out that the coder forgot to include the correct header-file and the square root was called with integer arguments, not floats. This was obviously wrong and not what he intended.

The warning has been there for quite a while, but noone has seen it because it was lost in the other 1000 harmless warnings that just scrolled over the screen. Warnings are there for a reason.

Btw - fixing the warnings took me like ... 4 hours ... That's nothing compared to the time it took to write all that stuff.

Answer 8

Yes. Even the trivial ones are worth fixing. Here's why. Some, like the example of main you mention, probably aren't worth fixing on their own, but they are in aggregate. Most compiler warnings will save you direct pain in the long term. They are bugs waiting to happen (or even happening now). In order to find those, you need an easy way to notice them. If you fix all of the warnings, then each new warning is a red flag and easy to notice. If you fix all of the critical ones but leave some like the "main" issue alone, you will miss the new critical ones.

Which is easier to remember? That any warning needs to be fixed or that you had 23 irrelevant warnings in this code base yesterday and so if you see 24 you need to go take a look?

In the code base I work on we tell the compiler to generate errors on all warnings. This forces us to fix them and keeps the code in much better shape. If there's ever a warning that truly isn't worth fixing, there is always #pragma to make it disappear. That way you still have a bright line for failure.

Answer 9

Your example is a perfect illustration why warnings shouldn't be ignored. void main(void) is an invalid prototype (but int main(void) works!)) and your code may break on some compilers. Your compiler was nice enough to point this out.

I've found that almost every warning actually pointed to a real problem, even if I failed to understand the cause at the time. Basically, I always did something and intended something else. The effect might have been achieved anyway, but only by pure coincidence.

Treat warnings as errors. Exceptions exist (there's one in the VB.NET compiler concerning uninitialized value types) but are exceedingly rare. And if you ever stumble upon one of these exceptions, you'll know it.

Answer 10

I agree with all the answers that say "fix them all", but I'd still like to present an opposing view:

There are legacy systems where some (many or all) components are without a dedicated maintainer and large parts of the system are basically unknown. Fixing compiler warnings in such unknown code can take a considerable amount of code (since for each warning the code together with its context needs to be understood) and introduces a possible source of errors (some code depends on undefined behaviour). And since such legacy systems rarely have extensive test coverage you can't even rely on the tests to notify you of regressions.

Answer 11

I'm going to move against the pack here and say that there may be some compiler warnings that aren't worth fixing. Example follows:

We have a great deal of code that was written prior to Java 1.5 and generics. It still works. Eclipse also generates literally thousands of warnings about unparametrized collections. Practically all of these collections are limited in scope; they're never going to break. But wait, you may ask; what about the few collections which aren't limited in scope? Aren't you risking a collection somewhere containing an instance of something that should never go in there?

The response is: a lot of that code is also unchanged. It's legacy-ish; no new code is modifying those collections, and that code has been used for years without problems. It's still in our code base, and we would have to maintain it if a bug manifested, but in practice, it's done its job. Spending the time necessary to parametrize all of those generic collections would take time out of other needed projects.

Note the caveats here, then:

• The code has been used for a long time with no bugs reported.
• The code is not undergoing any substantial editing.

If either of these stops being true - say, for instance, a bug suddenly does manifest, and we have to go in and edit - the warnings suddenly become more worthwhile to fix. (Incidentally, they're very inexpensive to fix at this point, too; we can fix the relevant blocks along the way as we fix the bug.)

Answer 12

You should always try to have no warnings come up on a compile. That way, new warnings get attention. (My most frustrating experience ever with this was the AIX C++ compiler back in 2002, which spat out hundreds of warnings for code that wasn't even heavily templated.)

Know what each warning means. In your case, you should have typed the standard int main() rather than the incorrect void main(void) or the clumsier int main(int argc, char **argv). Eliminate what you can, and suppress the warnings you have deemed acceptable.

You may not want to fix every warning. As saua says, there are legacy systems where fixing warnings is downright dangerous, and complex systems like CGAL where you don't want to muck with the code either. In converting our apps to run under 64 bits, I found cases where I could eliminate a warning only with a cast, which would have potentially caused other problems. We suppressed that warning.

Answer 13

Just to add warnings, here is what a gnome developer use in his code:

http://blogs.gnome.org/otte/2008/12/22/warning-options/

Answer 14

Another reason to ensure that your code compiles with no warnings is that, if someone else* has to make changes to the code at some point in the future, a good place to start is to make sure that they can compile the code in its current form.

However, if they do this, and see lots of warnings, are these warnings that have always existed, or have they installed the compiler incorrectly on their PC?

At least if there's a rule (and a box to tick on the checklist, if you use one) that the code must compile without warnings, then you avoid this ambiguity.

(* Someone else may include your future self).

Answer 15

Warnings are broken windows. If you don't fix the ones you've got, you'll end up with more.