Good approaches to enforce building with increased warning level for new C++ code

Question

I've inherited a large C++ codebase for several Windows applications that successfully is in use by many customers.

• The codebase is large, >1mill LOC.
• The codebase has a history of 15+ years.
• The codebase is in some areas dominated by C programming style and/or not very modern C++ style, e.g. not using Standard C++ collections and algorithms.
• The codebase has unfortunately only been compiled with warning level 2 (/W2 in Visual C++). I would like to increase to level 3 (/W3) to increase security and prepare for 64-bit.

The most problematic point in the increase to warning level 3 are the many warnings received involving signed/unsigned mismatches and I recognize that it will be a very large task to resolve all those for the existing codebase.

What would be a good approach to ensure and enforce that new code committed to the codebase are compiled with the increased warning level?

In more general terms the question could be rephrased to how you enforce increased programming quality into new committed code. If you don't do anything, new code has, in my experience, a tendency to be affected and styled similar to existing code, rather than being improved to more modern standards.

Donahoe

since you say this codebase is "successfully is in use by many customers", do you intend to re-release the "increased warning level" codebase to the customers? Have you considered the implications of doing that if at all? – Sauerkraut

Have a buildsystem with global compiler settings, such that the warning level can be changed easily. Search for grave warnings and fix them asap, lesser warnings can be (globally) disabled. I dont like enforcement, every developer should just fix any warnings in the code, he is currently working on. – Varlet

@MeThinks: I have thought about the implications. I do not intend to release a new version to customers for this sake alone. The increased warning level etc will be done in a development / non-production VCS branch and extensive tests needs to be planned and executed. – Donahoe

@Markus Kull: We already have a central build system. It is very easy to change warning level for all files in one setting. But it does not support different warning levels for different files. That is part of the problem. – Donahoe

Answer 1

I would even go as far as going to warning level 4 (/W4).

---

Since you're using Visual Studio, it's quite easy to suppress bothersome warnings like signed vs unsigned comparision:

#pragma warning(disable:NNNN)

Where NNNN is the number of your warning. Now put all those disabled warnings in a header file (say, "tedious_warnings.h") and force-include that header file everywhere - Project Properties -> C/C++ -> Advanced -> Forced Include File.
Later on, or better, ASAP, remove the force include and work your way through the warnings, since most of them are quite easy to fix (size_t instead if int, etc).

Sayre

Not a bad idea. But I believe the forced include file solution also would disable the high level warnings for new files. I would like a solution in where new files added by developers from start can be checked towards the higher standard, e.g. higher warning level. – Donahoe

Answer 2

Perhaps you could create new code in separate DLLs or Libraries. That way you can enforce your higher warning level (and I would say go for /W4 and be prepared to turn off a few of MS's dafter warnings rather than settle for /W3) without having to wade through 1000s of warnings from the old code.

Then you can start working on cleaning up the old code, a bit at a time, when there is time -- making sure you have suitable unit tests in place to avoid breaking it by accident of course.

Affine

Answer 3

you may not like the answer...

remove the warnings by correcting the issues.

i'm very picky about warning levels; even i ignore warnings which i don't need to correct, especially when the warning level is high and build times are high. meanwhile, new ones slip in (in large codebases). removing them incrementally doesn't work very well, in my experience -- they tend to get ignored if the noise is too high, or it is not enforced.

you need to reduce the warning noise so people can see the warnings they add (at the warning level you desire).

to reach the compliance level you want/need, make it a priority.

if you don't know whether the conversions/comparisons are valid, you can always use a template function with an error action (assert, throw, log) to perform the logic when in doubt.

it can be a slow/tedious process, but it's also a good way to learn the codebase.

i typically start at the libraries highest in the tree, or those which are reused most often. once a library meets a standard, maintain that standard.

Homovec

of course. the task could either be done hastily, or as a benefit to the program's correctness. changes are a good time to introduce static/runtime checks, and may be backed by unit tests. many programs do not have enough of either. if one takes the right approach, and doesn't reimplement portions of the program, this can be a good way to to verify correctness and improve error detection. of course, human error can introduce new issues, but the tests/checks employed in the process should ultimately reduce the total number of issues in the program. (cont) – Homovec

(cont) many classes of warnings can be eliminated without change in a program's behaviour. – Homovec

@Justin: I mean, if the code already works, fixing warnings is not a priority. – Spurt

@Alexandre C. i agree in some cases. if you have to deal directly with the older programs and their warnings (e.g. you have to maintain, extend, or they are simply in your translations), then they can't be ignored unless you correct (or individually disable) them. if they exist in libraries which are not being maintained (for example), they could be ignored for new development. in many cases, one can move the definitions to a single TU, so the messages do not appear all over the place. that requires no modification. the OP also mentioned that security and 64 bit compatibility were (cont) – Homovec

(cont) important. in this case, some classes of warnings should not be ignored since the importance of those improvements has been stated. some bugs will appear only in 64 bit builds. oftentimes, the original author did not consider or know everything needed to make the right choices at the time the program was written. the OP could at least try it (initially for a few libraries), in order to determine whether correcting them is a good approach. again, depending on the codebase and usage: you may be correct. we just don't have enough information to determine that for the case at hand. – Homovec

@Justin: I know all this, but writing new code which conforms to your higher standards and not trying to fix immediately the thousands of warnings from old code is much sounder in terms of business. Your goal is to deliver software that works, period. A long term goal should never have higher priority than the main business. – Spurt

Answer 4

If you are going to make code modifications as a result of the new stricter warning level, write adequate tests that protect against introducing new problems/bugs. Write the tests using the new warning level. Do this before you start to change the codebase and verify correct functionality. Then you can rerun the updated code against the same test case.

Sauerkraut

Answer 5

I would use an incremental approach.

The first step would be to modify the old files and add the required pragma action to deactivate the warning in the code.

The second step is to build a commit-hook that will refuse any committed file that contain the specific pragma pattern that discards all those "old" warnings.

This means that any modified file should be warning free.

However, let us be frank, developers always find ways to game the system.

Gyneco

Answer 6

My approach has been to go with the highest warning level you can and fix all the warnings that come up - you may even find some bugs in the process.

You should set this up using vsprops files so that all projects are compiled with the same warning level and any changes you make to these settings change in all projects.

A more incremental approach is to go with the highest warning level you can and then disable almost all warnings, leaving you with only a small number of warnings to consider at once - fix those and then switch on another warning, and so on until you are free of warnings.

Orbiculate