The “broken warnings theory” is a fictional theory of the norm-setting and signaling effect of coding practices and bug-checking techniques in 3rd party libraries on new bugs and design anti-patterns. The theory states that maintaining and monitoring warning levels to prevent small problems such as “signed/unsigned mismatch”, “no effect before comma”, and “non-standard extension used” helps to create an atmosphere of order and lawfulness, thereby preventing more serious bugs, like buffer overruns, from happening.
Visual Studio 2017 version 15.6 Preview 1 comes with new ways to make your code more robust and warning free.
Problem Description
Jokes aside though, not all warnings have been made equal:
- Some are precise
- Some are useful
- Some are actionable
- Some are fast to detect
- Some have little effect on existing code bases
Virtually none have all 5 of these nice-to-have characteristics, so a particular warning would usually fall somewhere on the spectrum of these traits creating endless discussions on which should or should not be reported. Naturally, different teams would settle on different criteria as to what set of warnings should be emitted, while compiler developers would try to put them into some overapproximated taxonomy trying to satisfy those numerous criteria. Clang and GCC try to be more fine-grained by using warning families, MSVC is more coarse-grained with its use of warning levels.
In our Diagnostics Improvements Survey, 15% of 270 respondents indicated they build their code with /Wall /WX indicating they have a zero tolerance for any warnings. Another 12% indicated they build with /Wall, which implies /W4 with all off-by-default warnings enabled. Another 30% build with /W4. These were disjoint groups that altogether make 57% of users that have stricter requirements to code than the default of Visual Studio IDE – /W3 or the compiler by itself – /W1. These levels are somewhat arbitrary and in no way represent our own practices. The MSVC libraries team, for example, strives hard to have all our libraries be /W4 clean.
While everyone disagrees on which subset of the warnings should be reported, most agree there should be 0 warnings from the agreed upon set admitted in a project: all should be fixed or suppressed. On one hand, 0 makes any new warning a JND of the infamous Weber-Fechner law, but on another it is often a necessity in cross-platform code, where it’s been repeatedly reported that warnings on one platform/compiler can often manifest themselves as errors or worse – bugs on another. This zero-tolerance to warnings can be easily enforced for internal code, yet virtually unenforceable for external code of 3rd-party libraries, whose authors may have settled on a different set of [in]tolerable warnings. Requiring all libraries to be clean with regard to all known warnings is both impractical (due to false positives and absence of standard notation to suppress them) and impossible to achieve (as the set of all warnings is an ever-growing target). The latter one is a result of compilers and libraries ecosystems coevolution where improvements in one require improvements, and thus keeping up in the race, in the other. Because of this coevolution, a developer will often be dealing with compilers that haven’t caught up with their libraries or libraries that haven’t caught up with their compilers and neither of those would be under the developer’s control. The developers under such circumstances, which we’d argue are all the developers using an alive and vibrant language like C++, effectively want to have control over emission of warnings in the code they don’t have control over.
Proposed Solution
We offer a new compiler switch group: /external:* dealing with “external” headers. We chose the notion of “external header” over “system header” that other compilers use as it better represents the variety of 3rd party libraries in existence. Besides, the standard already refers to external headers in [lex.header], so it was only natural. We define a group instead of just new switches to ease discoverability by users, which would be able to guess the full syntax of the switch based on the switches they already know. For now, this group consists of 5 switches split into 2 categories (each described in its own section below):
Switches defining the set of external headers
/external:I <path>/external:anglebrackets/external:env:<var>
Switches defining diagnostics behavior on external headers
/external:W<n>/external:templates-
The 2nd group may later be extended to /external:w, /external:Wall, /external:Wv:<version>, /external:WX[-], /external:w<n><warning>, /external:wd<warning>, /external:we<warning>, /external:wo<warning> etc. which would constitute an equivalent of the corresponding warning switch when applied to an external (as opposed to user) header or any other switch when it would make sense to specialize it for external headers. Please note that since this is an experimental feature, you will have to additionally use /experimental:external switch to enable the feature until we finalize its functionality. Let’s see what those switches do.
External Headers
We currently offer 4 ways for users and library writers to define what constitutes an external header, which differ in the level of ease of adding to build scripts, intrusiveness and control.
/external:I <path>– a moral equivalent of -isystem, or just -i (lowercase) from GCC, Clang and EDG that defines which directories contain external headers. All recursive sub-directories of that path are considered external as well, but only the path itself is added to the list of directories searched for includes./external:env:<var>– specifies the name of an environment variable that holds a semicolon-separated list of directories with external headers. This is useful for build systems that rely on environment variables like INCLUDE and CAExcludePath to specify the list of external includes and those that shouldn’t be analyzed by/analyzerespectively. The user can simply use/external:env:INCLUDEand/external:env:CAExcludePathinstead of a long list of directories passed via/external:Iswitch./external:anglebrackets– a switch that allows a user to treat all headers included via#include <>(as opposed to#include "") as external headers#pragma system_header– an intrusive header marker that allows library writers to mark certain headers as external.
Warning Level for External Headers
The basic idea of /external:W<n> switch is to define the default warning level for external headers. We wrap those inclusions with a moral equivalent of:
#pragma warning (push, n) // the global warning level is now n here #pragma warning (pop)
Combined with your preferred way to define the set of external headers, /external:W0 is everything you need to do to entirely shut off any warnings emanating from those external headers.
Example:
External Header: some_lib_dir/some_hdr.hpp
template <typename T>
struct some_struct
{
static const T value = -7; // W4: warning C4245: 'initializing': conversion from 'int' to 'unsigned int', signed/unsigned mismatch
};
User code: my_prog.cpp
#include "some_hdr.hpp"
int main()
{
return some_struct<unsigned int>().value;
}
Compiling this code as:
cl.exe /I some_lib_dir /W4 my_prog.cpp
will emit a level-4 C4245 warning inside the header, mentioned in the comment. Running it with:
cl.exe /experimental:external /external:W0 /I some_lib_dir /W4 my_prog.cpp
has no effect as we haven’t specified what external headers are. Likewise, running it as:
cl.exe /experimental:external /external:I some_lib_dir /W4 my_prog.cpp
has no effect either as we haven’t specified what the warning level in external headers should be and by default it is the same as the level specified in /W switch, which is 4 in our case. To suppress the warning in external headers, we need to both specify which headers are external and what the warning level in those headers should be:
cl.exe /experimental:external /external:I some_lib_dir /external:W0 /W4 my_prog.cpp
This would effectively get rid of any warning inside some_hdr.hpp while preserving warnings inside my_prog.cpp.
Warnings Crossing an Internal/External Boundary
Simple setting of warning level for external headers would have been good enough if doing so wouldn’t hide some user-actionable warnings. The problem with doing just pragma push/pop around include directives is that it effectively shuts off all the warnings that would have been emitted on template instantiations originating from the user code, many of which could have been actionable. Such warnings might still indicate a problem in user’s code that only happens in instantiations with particular types (e.g. the user forgot to apply a type trait removing const or &) and the user should be aware of them. Before this update, the determination of warning level effective at warning’s program point was entirely lexical, while reasons that caused that warning could have originated from other scopes. With templates, it seems reasonable that warning levels in place at instantiation points should play a role in what warnings are and what aren’t permitted for emission.
In order to avoid silencing the warnings inside the templates whose definitions happen to be in external headers, we allow the user to exclude templates from the simplified logic for determining warning levels at a given program point by passing /external:templates- along with /external:W<n>. In this case, we look not only at the effective warning level at the program point where template is defined and warning occurred, but also at warning levels in place at every program point across template instantiation chain. Our warning levels form a lattice with respect to the set of messages emitted at each level (well not a perfect one, since we sometimes emit warnings at multiple levels). One over-approximation of what warnings should be allowed at a given program point with respect to this lattice would be to take the union of messages allowed at each program point across instantiation chain, which is exactly what passing /external:template- does. With this flag, you will be able to see warnings from external headers as long as they are emitted from inside a template and the template is instantiated from within user (non-external) code.
cl.exe /experimental:external /external:I some_lib_dir /external:W0 /external:templates- /W4 my_prog.cpp
This makes the warning inside the external header reappear, even though the warning is inside an external header with warning level set to 0.
Suppressing and Enforcing Warnings
The above mechanism does not by itself enable or disable any warnings, it only sets the default warning level for a set of files, and thus all the existing mechanisms for enabling, disabling and suppressing the warnings still work:
/wdNNNN, /w1NNNN, /weNNNN, /Wv:XX.YY.ZZZZetc.#pragma warning( disable : 4507 34; once : 4385; error : 4164 )#pragma warning( push[ ,n ] ) / #pragma warning( pop )
In addition to these, when /external:templates- is used, we allow a warning to be suppressed at the point of instantiation. In the above example, the user can explicitly suppress the warning that reappeared due to use of /external:templates- as following:
int main()
{
#pragma warning( suppress : 4245)
return some_struct<unsigned int>().value;
}
On the other side of developers continuum, the library writers can use the exact same mechanisms to enforce certain warnings or all the warnings at certain level if they feel those should never be silenced with /external:W<n>.
Example:
External Header: some_lib_dir/some_hdr.hpp
#pragma warning( push, 4 )
#pragma warning( error : 4245 )
template <typename T>
struct some_struct
{
static const T value = -7; // W4: warning C4245: 'initializing': conversion from 'int'
// to 'unsigned int', signed/unsigned mismatch
};
#pragma warning( pop )
With the above change to the library header the owner of the library now ensures that the global warning level in his header is going to be 4 no matter what the user specified in /external:W<n> and thus all level 4 and above warnings will be emitted. Moreover, like in the above example she can enforce that a certain warning will be always treated as error, disabled, suppressed or emitted once in her header, and, again, the user will not be able to override that deliberate choice.
Limitations
In the current implementation you will still occasionally get a warning through from an external header when that warning was emitted by the compiler’s back-end (as opposed to front-end). These warnings usually start with C47XX, though not all C47XX warnings are back-end warnings. A good rule of thumb is that if detection of a given warning may require data or control-flow analysis, then it is likely done by the back-end in our implementation and such a warning won’t be suppressed by the current mechanism. This is a known problem and the proper fix may not arrive until the next major release of Visual Studio as it requires breaking changes to our intermediate representation. You can still disable these warnings the traditional way with /wd47XX.
Besides, this experimental feature hasn’t been integrated yet with /analyze warnings as we try to gather some feedback from the users first. /analyze warnings do not have warning levels, so we are also investigating the best approach to integrate them with the current logic.
We currently don’t have a guidance on the use of this feature for SDL compliance, but we will be in contact with the SDL team to provide such guidance.
Conclusion
Coming back to the analogy with the Broken Windows Theory, we had mixed feelings about the net effect of this feature on the broader libraries ecosystem. On one hand it does a disservice to library writers by putting their users into “not my problem” mode and making them less likely to report or fix problems upstream. On the other hand, it gives them more control over their own code as they can now enforce stricter requirements over it by subduing rogue libraries that prevented such enforcement in the past.
While we agree that the secondary effect of this feature might limit contributions back to the library, fixing issues upstream is usually not a user’s top priority given the code she is working on, but fixing issues in her own code is her topmost priority and warnings from other libraries obstruct detection of warnings in it because she cannot enforce /WX on her code only. More importantly, we believe this will have a tertiary effect that would balance the net loss of the secondary effect.
By enabling a developer to abstract from 3rd party library warnings we encourage her to concentrate on her own code – make it cleaner, possibly even warning free on as large level as she possibly can. 3rd party library developers are also developers in this chain, so by allowing them to abstract from their 3rd party dependencies, we encourage them to clean up their code and make it compile with as large warning level as they possibly can, etc. etc. Why is this important? In essence, in the current world the warnings avalanche across the entire chain of library dependencies and the further you are on this chain, the more difficult it becomes to do something about them – the developer feels overwhelmed and gives up on any attempt to do so. On the other hand, in the world where we can distinguish our own code from 3rd party code, each developer in the chain has means to stop (block the effects of) the avalanche and is encouraged to minimize its impact, resulting in minimizing the overall impact to the entire chain. This is a speculation of course, but we think it is as plausible as the secondary effect we were concerned about.
In closing, we would like to invite you to try the feature out for yourself and let us know what you think. Please do tell us both what you like and what you don’t like about it as otherwise the vocal minority might decide for you. The feature is available as of Visual Studio 15.6 Preview 1. As always, we can be reached via the comments below, via email (visualcpp@microsoft.com) and you can provide feedback via Help -> Report A Problem in the product, or via Developer Community. You can also find us on Twitter (@VisualC) and Facebook (msftvisualcpp).
P.S. Kudos to Robert Schumacher for pointing to the analogy with the Broken Windows Theory!
Seems to not work with junction(s) in the include folder path.
It would be good with some environment variable support to reduce the cruft in the command line.