One reason you should clean up warnings

by Michael S. Kaplan, published on 2006/01/30 22:01 -05:00, original URI: http://blogs.msdn.com/b/michkap/archive/2006/01/30/519993.aspx


(The following post is reprinted from a blog that is no longer around. Done so with the permission of the author -- special thanks to Mike Dolenga, for that permission and for having a cynical side that I find quite comforting since it makes me look like an optimist!)

Compiler warnings can sometimes be more aggravating than useful.  We've all encountered them, and learn which ones to ignore in our code.  I think it's best to clean them up, either by fixing the code or suppressing the warning itself.  Suppression should be done with caution and should be clearly documented, of course.

I was finally convinced of the need to clean up warnings at a previous employer a few years ago.  This company has since gone bankrupt, and what you're about to read is one of the reasons why.  It provided a database application to a specific industry, let's just imagine it was widget manufacturers.  The application had a lot of flexibility to it, too much in fact, and was sold to specific widget makers with customizations defined by them.

One day, one of the widget makers called and said they were hitting a bug in the program which caused some of their data to be lost.  We investigated and found the bug.  The developer who owned the code checked in a fix, and we eagerly told the widget maker we would soon have an update ready for them, which we would courier overnight on a CD.

The client received the CD, installed our program and called right away to tell us the desktop shortcut didn't work.  Luckily there were a few technically savvy people at the client's office who pointed out that our program was nowhere to be found on the system where they had just installed it.  Our used car salesman turned CEO was red faced and reflexively told the client the disc we sent must have been defective.  I found out later the client knew better, but it did buy us another day.

We did some digging, and found out what happened.  First, the checkin made by the developer caused a build break.  This also meant he never tested his fix, but that's a separate issue.  The builder kick off a build and as was his habit, ignored the error, because there were so many warnings in the code base.  He never noticed that our main application failed to build.

The next guy in the chain, who owned the installer package, also ignored the warning that the application did not exist when building his package.  Why?  Again, there were too many warnings and he just assumed all those lines flying by were the usual noise.  And finally, after burning the CD, the test team installed the build on a machine which already had an installation.  They didn't test the fix; instead, they simply logged in and made sure the application launched.  Why?  Management routinely refused to buy them the tools to quickly re-image an OS and have a clean system to install, and insisted this CD get out the door before the FedEx office closed for the day.  The same management which refused to buy additional build hardware so that we could build on something other than a Pentium 90 (400s were standard at the time).

While the failures here were numerous, a key component was all the warnings in the code.  They made everyone sloppy and complacent.  If the builder had noticed the application never built, that could have avoided all the problems.  If you go from zero errors and warnings to some, that's a lot easier to notice than an extra message out of thousands.

As a footnote, I have no idea whether the fix worked.  Not only did I quit the next week (I already had an offer elsewhere), but the client cancelled the contract.  The ensuing litigation was one of the things which finally put an end to the software company.


# Larry Osterman [MSFT] on 30 Jan 2006 11:32 PM:

At some point, I need to write up "Warning Level 3 considered harmful", which is a recycled rant from one of the early C compiler developers (not at Microsoft).

# Michael S. Kaplan on 30 Jan 2006 11:39 PM:

Well, harmful if people don't fix the warnings!

I was quite pleased every time I had a project compiling with /WX /W4 and no warnings.... :-)

# Ben Cooke on 31 Jan 2006 2:43 AM:

I encountered this just the other week, in fact. I was working on a C# codebase that I inherited, and it had lots of those "Unreachable code detected" warnings and a few other things like private variables that are never used. Several times during the few days I was working on that code I ran the build script and some errors got concealed in the mess. The result was an unmodified assembly, which then proceeded to do exactly the same thing as it had just done and leading me to believe that the bug I was attempting to fix was still present.

Of course, the best fix is just not to make mistakes in the first place, but I'm not perfect. :)

# Mo on 31 Jan 2006 5:57 PM:

This reminds of a few past employers.

I have a set of GCC warning flags which I use (‘all the useful ones’), and my code is normally warning-free with those flags—though when warnings do appear they get looked into and I determine whether to fix the cause of the warning, or fix the warning. There's only really one thing which falls into the latter category, and that's unused parameter warnings (I occasionally leave these be as a deliberate reminder in places where code isn't fully implemented); I consider explicit casts a case of the former (after all, I'm telling the compiler precisely what I want to do having considered it carefully).

There's nothing worse than code which will only compile quietly when there are no warning flags turned on, except perhaps code which is full of issues and warning-free because the programmer fixed the warnings instead of the problems.

# bombaytv on 1 Feb 2006 5:11 AM:

Was there a way to block W4786 in VC++ 6.0 other than placing #pragma warning in source file? It used to drove me crazy.

# Michael S. Kaplan on 1 Feb 2006 9:43 AM:

-------------------
Compiler Warning (level 3) C4786

The identifier string exceeded the maximum allowable length and was truncated.

The debugger cannot debug code with symbols longer than 255 characters. In the debugger, you cannot view, evaluate, update, or watch the truncated symbols.

This limitation can be overcome by shortening identifier names. The example code below demonstrates this method.

A trace mechanism can also be used to solve this problem. A trace mechanism is like the printf statements in the code. It keeps track of what is going on in an application during the debugging process. The _ASSERT, _ASSERTE, _RPTn and _RPTFn macros provide concise and flexible ways to perform the trace. These macros are not defined when _DEBUG is not defined. See Using Macros for Verification and Reporting for more information.

This warning is off by default. See Compiler Warnings That Are Off by Default for more information.
-------------------

Hmmmm... seems like the answer is in the question? Use shorter identifiers!


# bombaytv on 1 Feb 2006 12:24 PM:

Use shorter identifiers? How do I shorten std::map, which yields the following bunch of warnings: d:\test.cpp(12) : warning C4786: 'std::reverse_bidirectional_iterator,std::map,std::allocator >::_Kfn,std::less,std::allocator >::iterator,std::pair,std::pair &,std::pair *,int>' : identifier was truncated to '255' characters in the debug information d:\test.cpp(12) : warning C4786: 'std::reverse_bidirectional_iterator,std::map,std::allocator >::_Kfn,std::less,std::allocator >::const_iterator,std::pa ir,std::pair const &,std::pair const *,int>' : identifier was truncated to '255' characters in the debug information d:\test.cpp(12) : warning C4786: 'std::pair,std::map,std::allocator >::_Kfn,std::less,std::allocator >::iterator,std::_Tree,std::map,std::allocator >::_Kfn,std::less,std::allocator >::iterator>' : identifier was truncated to '255' characters in the debug information d:\test.cpp(12) : warning C4786: 'std::pair,std::map,std::allocator >::_Kfn,std::less,std::allocator >::const_iterator,std::_Tree,std::map,std::allocator >::_Kfn,std::less,std::allocator >::const_iterator>' : identifier was truncated to '255' characters in the debug information c:\vstudio\vc98\include\xtree(183) : warning C4786: 'std::_Tree,std::map,std::allocator >::_Kfn,std::less,std::allocator >::~_Tree,std::map,std::allocator >::_Kfn,std::less,std::allocator >' : identifier was truncated to '255' characters in the debug information c:\vstudio\vc98\include\xtree(160) : warning C4786: 'std::_Tree,std::map,std::allocator >::_Kfn,std::less,std::allocator >::_Tree,std::map,std::allocator >::_Kfn,std::less,std::allocator >' : identifier was truncated to '255' characters in the debug information

Please consider a donation to keep this archive running, maintained and free of advertising.
Donate €20 or more to receive an offline copy of the whole archive including all images.

referenced by

2006/04/10 Getting all you can out of a keyboard layout, Part #8

go to newer or older post, or back to index or month or day