“Off By Default” Compiler Warnings in Visual C++

“Off By Default” Compiler Warnings in Visual C++

  • Comments 10

Jon Sturgeon

Greetings! My name is Jon Sturgeon; I’m a developer in the Forefront team here at Microsoft. I’m happy to be able to contribute to the Visual C++ Team Blog as a “guest blogger”.

One of my passions when writing C++ code has always been to use as many techniques as possible to find bugs earlier and earlier in the development process; obviously the earlier that bugs are found the easier and cheaper they generally are to fix. And if they can be found and corrected before the code is even built, everybody wins.


Compiler Warning Levels

I’m assuming that everybody is familiar with the compiler warning level in the Visual C++ compiler. It is exposed through the /W command-line switch and through the “Warning Level” item in the C/C++ projects’ properties page in the IDE:

clip_image002

The relevant part of the property page shown above is for a brand-new C++ project created in Visual Studio 2010. As you can see, the warning level is set to /W3 by default. However, I recommend you compile all your code at warning level 4 instead, since the increase in warning “noise” can be fairly minimal and there are some useful warnings that are only active with /W4 enabled. It is best-practice in many teams to ensure that all code compiles cleanly with /W4 enabled and the related option to treat all warnings as errors (/WX) is often also specified to ensure that code cannot be submitted without first addressing all the warnings identified by the compiler.


/Wall

One warning option that isn’t so well-known is /Wall, shown in the IDE below:

clip_image004

This option (obviously) enables all warnings in the compiler. This of course leads to the question of what warnings are not enabled when /W4 is specified? The answer can be found on this page on MSDN which lists all the warnings that are “off by default” in the compiler. I’m not going to walk through each and every warning in this list, but you can see that many of them are very trivial and “noisy” warnings that you indeed are unlikely to want enabled on any realistic codebase. For instance, C4061 (“enumerator in switch statement is not explicitly handled by a case label”) is unlikely to identify any actual bugs and you’d probably end up quickly disabling it if it was not off by default.

However, as you scan through the list, you can see that some of the warnings might identify potentially serious problems. A couple of my favorites relate to virtual functions: C4263 (member function does not override any base class virtual member function) and C4264 (no override available for virtual member function; function is hidden). In both of these cases, I want to know when my code triggers the warning, since there is quite likely to be an underlying bug that at the very least needs to be investigated.


/Wall in the Real World

Since just enabling the /Wall switch on a real-world codebase is going to be unrealistic due to all the noise from the trivial warnings, is there a way to gain the benefits of some of these more useful warnings without suffering the pain of the less useful ones? Yes there is, and here’s the way I’ve approached this in the past:

  1. Create a header file that every project in your codebase can #include.
  2. Force-include this header file in every translation unit in your codebase by using the /FI compiler option.
  3. Add #pragma warning(disable:<warning number>) lines to the header file to disable the individual warnings that produce nothing but noise for your codebase.
  4. Turn on the /Wall switch for your codebase.

Alternatively, you can use the same technique to selectively enable individual off-by-default compiler warnings and rebuild without adding the /Wall switch. However, I prefer the first approach, since that way I’m making explicit choices about which warnings I want disabled. Additionally, any future warnings that the compiler team adds in future releases will then be automatically enabled when the compiler is updated, at which time I can decide if it makes sense to keep them enabled.


Handling External Header Files

One thing that you are likely to find when trying to build your codebase with many of these off-by-default warnings enabled is that some of the compiler/SDK-supplied header files do not compile cleanly. I know that the Visual C++ & Windows SDK teams are very good at ensuring all the standard headers will compile cleanly at warning level 4, but there are no such guarantees when enabling /Wall. Because of this, if you want to gain the benefits of building with some of these warnings enabled, you’ll need a solution to this problem. Unfortunately the best option I’ve found so far is to wrap the offending header file an a header of your own which temporarily disables the offending warnings and arrange for your code to #include that instead. For instance, if your code uses ATL, you may need to use a header similar to this:

#pragma warning(push) #pragma warning(disable:4265) #pragma warning(disable:4625) #pragma warning(disable:4626) #include <atlbase.h> #pragma warning(pop)

This header file temporarily disables three of the off-by-default warnings that atlbase.h can generate, includes the header, then re-enables the warnings so that they are active again for your code. If you can arrange for this small header file to be found in your codebase’s include path before any of the system headers, you can name it atlbase.h so you won’t have to modify any of your actual source code’s #include directives.


False Positives

There are always false-positive scenarios with compiler warnings and the case is no different for the off-by-default warnings either. You’ll have to use your judgment for each warning that you want to enable to see if the number of false positives that are generated in your codebase outweigh the potential for catching a real bug. One example where I think it can be worth suffering a few false-positives is C4265 - class has virtual functions, but destructor is not virtual. This warning, when enabled, is triggered by code like this:

class Animal { public: Animal(); ~Animal(); //not virtual virtual void MakeNoise(); };

As you can see, the class has a virtual function, but the destructor is not declared virtual. This can be a real bug that can lead to memory leaks if objects that implement the Animal base class are expected to be created on the heap and later deleted through a pointer of Animal * type. Or this can be a false-positive (i.e. not a problem) if these objects have an alternative destruction mechanism, such as the virtual IUnknown::Release() method that all COM interfaces employ. Consequently, if you choose to build your code with this warning enabled, you’ll have false-positive warnings in any code that implements COM interfaces. At this point you’ll have to make a decision about whether the warning is likely to catch real bugs in your code; if you want to keep the warning enabled, you’ll have to temporarily disable it around the false-positive locations, using the #pragma warning method shown in the atlbase.h example above.


Summary

To sum up, here’s my recommendations:

  • If you are not already building (cleanly) at warning level 4, start today!
  • Consider using the techniques above to enable some of the off-by-default compiler warnings

As a starting point, I’d consider enabling these off-by-default warnings:

  • C4191 - unsafe conversion from 'type of expression' to 'type required'
  • C4242 - conversion from 'type1' to 'type2', possible loss of data
  • C4263 - member function does not override any base class virtual member function
  • C4264 - no override available for virtual member function from base 'class'; function is hidden
  • C4265 - class has virtual functions, but destructor is not virtual
  • C4266 - no override available for virtual member function from base 'type'; function is hidden
  • C4302 - truncation from 'type 1' to 'type 2'
  • C4826 - conversion from 'type1' to 'type2' is sign-extended. This may cause unexpected runtime behavior
  • C4905 - wide string literal cast to 'LPSTR'
  • C4906 - string literal cast to 'LPWSTR'
  • C4928 - illegal copy-initialization; more than one user-defined conversion has been implicitly applied

If you enable these warnings on your code and they help you to find real bugs, I’d love to hear about them in the comments.

Finally, I’d like to thank the Visual C++ team for allowing me to guest on their blog. Thanks!

  • I don't like the /FI approach.

    I rather alter the command line options in the project properties and here's what i'm using:

    /wd4820 /we4289 /wd4342 /wd4347 /wd4514 /we4545 /we4546 /we4547 /we4548 /we4549 /we4619 /we4623 /we4625 /we4626 /wd4710 /we4836 /we4905 /we4906 /we4928 /we4946 /wd4986

  • Is there any way to add a warning for "member variable 'foo' is not initialised in constructor of class bar" for POD types?  I've fixed many many bugs because someone forgot to initialize a bool or other POD (anything without a default constructor) in a constructor.  Surely this doesn't require a static analyzer to figure out does it?

  • This is all dandy, but if you could fix/remove the warnings in your own header files it would make our lives easier.

    Max.

  • When I ask people about this they say "Well we tried using /W4 here, but reverted the change when we started getting tons of warnings from Microsoft's own header files..." and "it'd be nice if you could tell VC to disable warnings when generated from files in the standard include path. Though, I guess that may be problematic for templates."

  • when i want debug  program  istream and iostream do  not known by c++

    please helpe me

  • @Max, Jonathan: Amen to that

  • I use more than one compiler.  One of them is absolutely free, comes with complete source code, and compiles on and for every operating system I've ever used.  It also has a -Wall option, and never complains about system header files.  Amazing, isn't it?  

    Jon, I don't mean to beat up on you; you at least provided a workaround.  But, seriously, instead of revamping the GUI every few years, breaking the Help system, and dithering with the command-line tools, why doesn't Microsoft undertake the basic engineering of patrolling its own header files to make them warning-free?  It's got to be in the interest of both Microsoft and its customers: Either the warnings produce useful information that can be used to fix the header files, or they don't and are a disservice to everyone who would use them.  Both the warning system and the header files are bound to be improved by the effort.  

  • [James K. Lowden]

    > It also has a -Wall option

    VC's /Wall and GCC's -Wall are completely different. VC's /Wall literally means "all warnings". GCC's -Wall means "some useful warnings". GCC also has -Wextra, which means "more useful warnings". Even combined, -Wall -Wextra leaves out several useful warnings (e.g. -Wshadow). See gcc.gnu.org/.../Warning-Options.html for more info.

    > why doesn't Microsoft undertake the basic engineering of patrolling its own header files to make them warning-free?

    I can speak for our C++ Standard Library implementation. We aim to compile cleanly at /W4 /analyze, which contains very many useful warnings and very few obnoxious warnings. We don't attempt to compile cleanly at /Wall, because it contains pure noise warnings like "function not inlined" and "struct contains padding". (The idea behind those warnings is that they can be enabled for individual compilations of individual translation units, so developers can figure out what's getting inlined, etc., not that they should be enabled for regular builds. Our /Wall enables them because it does exactly what it says on the box.)

    In the long run, I agree that we should identify /Wall's useful warnings, document them and probably bundle them in a switch like /W5, and make our headers compile cleanly at that level. While that would be valuable, we currently have more important things to do with our limited time. (Namely, correctness, performance, and C++0x features. Dinkumware and I are library developers, and we don't work on stuff like the IDE.)

  • I thought I asked this, but it seems like it got swallowed somewhere along the way.

    Is it a bug that C4265 doesn't warn when the destructor isn't explicitly declared? There has been a couple of times that that has bit me in the past.

  • What approach is better for classes that implement COM interfaces:

    1) use pragma around the class to disable C4265

    2) make the destructor virtual but protected

    What do you think?

Page 1 of 1 (10 items)