Theory of Broken Warnings

Original author: Yuriy Solodkyy
  • Transfer
" Theory of" broken "warnings"- this is a fictional theory, which claims that the team’s connivance towards minor warnings, such as“ inconsistencies with or without a sign, "“ the operator before the decimal point has no result, "" non-standard extension was used, "etc., directly provokes developers to connivance to similar or more serious warnings.The psychological mechanism of such provocation at the household level is illustrated by the phrase: "If others can, then why can not I?" - when the programmer sees that the warnings are in code other developers are not repaired, he ceases to consider the rules (and not only those violations of which he observed, but also any others) binding on himself. At the same time, the conditional average bar of the “permissible warning” in the team constantly decreases, sooner or later leading to an increase in the number serious bugs already.

Conversely, active work to prevent small (even the most insignificant) warnings in the code and punish the authors of this code (the so-called zero tolerance) creates an atmosphere of intolerance to warnings in general, and the activity to prevent small warnings allows you to “train” and significantly limit in the possibilities of repeat offenders, who usually neglect team rules.

The preliminary version of Visual Studio 2017 15.6 Preview 1 provides developers with tools to improve the quality of the code and clear it of warnings.

Description of the problem


Sweeping the jokes aside, it should be noted that not all warnings were created equal:

  • Some are accurate
  • Others - by efficiency
  • Still others - relevance
  • Fourth - high speed detection
  • Fifth - low impact on existing code bases

Alas, almost no warning has all five characteristics at once. Any diagnostics combines certain features from this list, which leads to endless discussions about which problems the programmer should be notified about and which not. Of course, different development teams use different criteria, and compiler developers try to create an extended classification of diagnostics to cater to all of these criteria. Clang and GCC try to differentiate the alerts more finely by grouping them together, and the MSVC uses a more crude separation of alert importance levels.

In our Diagnostics Improvements Survey, 15% of 270 respondents said they were collecting a code with the / Wall / WX keys, which indicates their zero tolerance for any warnings. Another 12% said that they started the assembly with the / Wall option , which includes the / W4 level plus all warnings turned off by default. Another 30% collect code at / W4 . These three disjoint groups make up 57% of users who have a stricter code quality than the default settings for Visual Studio (level / W3 ) or the compiler (level / W1 ). The separation of warnings by levels is to some extent arbitrary and does not in any way reflect our own approaches. For example, the MSVC library team is zealously clearing code at the / W4 level .

Despite the lack of consensus on what sets of warnings a developer should see, everyone agrees that no matter what set is adopted in a particular project, no active warnings should remain from it at the end: they should all be either fixed or suppressed. On the one hand, with this approach, each new warning serves as a threshold for distinguishing from the notorious Weber – Fechner law , and on the other hand, this is necessary in cross-platform code, since warnings issued on such code on the same platform / compiler often turn into errors and more serious failures to another, as has been repeatedly reported. Zero tolerance for warnings is easy to inculcate in relation to the internal code, but almost impossible in respect to the external code of third-party libraries, the authors of which can use a different set of issued / excluded warnings. The requirement that all libraries be cleared of all known warnings is both impractical (due to false positives and the lack of a standard recording form for suppressing them), and unattainable (as many warnings are continuously expanding). The second is due to the fact that the ecosystems of compilers and libraries are developing together, and improvements in some cause the need for improvements - and, therefore, the need to maintain a given pace - in others. As a result, developers have to deal either with compilers that are behind libraries, or with libraries that are behind compilers - and neither no others are under their control. In such circumstances, programmers (provided that they write in live and active languages ​​like C ++) want to determine for themselves what warnings they would like to see in code that is not under their control.

Proposed solution


We introduce a new group of compilation options / external: * , which works with "external" header files. We preferred the name " external header files " to the name " system header files ", adopted by other compilers, as it better reflects the diversity of existing third-party libraries. In addition, the C ++ standard already appeals to external header files in the [lex.header] section, so our choice is quite natural. We combined the new keys into a group, instead of describing them separately, because it will be easier for users to master them: The full syntax of new keys can be predicted by analogy with the already known keys. At the moment, the group consists of 5 parameters, divided into two categories (see the relevant sections below):

Parameters defining a set of external header files


  • / external: I
  • / external: anglebrackets
  • / external: env:

Parameters defining diagnostic behavior for external header files


  • / external: W
  • / external: templates-

The second group can be further supplemented with such parameters as / external: w, / external: Wall, / external: Wv:, / external: WX [-], / external: w, / external: wd, / external: we, / external: woetc. They can be used as analogues of warning levels or any other standard keys for external (as opposed to custom) header files. Please note that since this is an experimental function, to enable it, you will have to add the / experimental: external option until we debug it to the end. Let's see what the new keys do.

External Header Files


At the moment, library authors and users can specify the location of header files in four ways - they differ in the complexity of adding to assembly scripts, their embeddability, and degree of control.

  • / external: I - the ideological analogue of -isystem, or simply -i (in lower case), in the GCC, Clang and EDG compilers, with the help of which a directory with external header files is set. All recursive subdirectories in the path are also considered external, but only the path itself is added to the list of directories that search for included files;
  • / external: env:- sets the name of the environment variable in which the list of directories with external header files, separated by semicolons, is stored. This method is useful when using build systems that rely on environment variables such as INCLUDE and CAExcludePath, which sets the list of external include files and files that should not be checked with the / analyze switch , respectively. The user can simply write / external: env: INCLUDE and / external: env: CAExcludePath and not pass a long list of directories with the / external: I parameter ;
  • / external: anglebrackets - allows you to interpret all header files included with the #include <> command (as opposed to #include "" ) as external;
  • #pragma system_header - an embedded label for header files - allows library authors to mark certain header files as external.

Warning levels for external header files


/ External parameter : WAllows the user to set the default alert level for external header files. We wrap such inclusions in an analogue of the design:

#pragma warning (push, n)
// уровень предупреждений n в этом коде
#pragma warning (pop)

Using this key in combination with the preferred method of specifying external header files, you can completely disable any warnings issued to these files.

Example:


External header file: some_lib_dir / some_hdr.hpp

template 
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().value;
}

If you compile this code as follows:

cl.exe / I some_lib_dir / W4 my_prog.cpp

, the warning message C4245 of the 4th level mentioned in the comment will be issued on the header file. Compilation with the parameters:

cl.exe / experimental: external / external: W0 / I some_lib_dir / W4 my_prog.cpp

will have no effect, since we did not specify external header files. Compilation with parameters:

cl.exe / experimental: external / external: I some_lib_dir / W4 my_prog.cpp

also will not give any effect, since the warning level for external header files is not set and by default it corresponds to the level specified in the / W parameter (in our case 4). To suppress the warning in the external header files, we must specify both the path to these files and the warning level for them:

cl.exe / experimental: external / external: I some_lib_dir / external: W0 / W4 my_prog.cpp

This command will get rid of all warnings to the some_hdr.hpp file, leaving only warnings to the my_prog.cpp file.

Warnings affecting both internal and external code


It would be great if you could just set the alert level for external header files, however, we risk filtering out some messages that are also relevant for internal user files. If you use only push / pop pragma directives in conjunction with include, many useful warnings when instantiating templates in user code can disappear. Such warnings may indicate the presence of a problem precisely on the user's side, and it manifests itself when substituting only certain types into the template (for example, when you forgot to apply type conversion from, removing const or &), and then this problem should be reported. Prior to this version, the warning level at the time the message was issued was determined solely on the basis of lexical analysis, while the problem area may be in a different scope. Apparently, it makes sense to compare warning levels at the instantiation points of the templates to determine which warnings should be issued and which should not.

In order not to accidentally drown out warnings in templates whose definitions are in external header files, we allowed users to exclude templates from the simplified logic described above - this can be done by passing the / external: templates- option along with / external: W. At the same time, we look not only at the current warning level at the point where the template definition is contained and a warning is issued, but also at the warning levels at all points in the pattern instantiation sequence. Our warning levels form a lattice in relation to the entire set of messages of all levels (however, it is not ideal, because sometimes we issue warnings at several levels at once). A superset defining warnings that should be allowed at a given program point with respect to this lattice would be obtained by combining messages allowed at each program point through a chain of instantiations. This is exactly what the / external: template- key is for, allowing you to display warnings on templates stored in external header files and implemented in user (i.e., internal) code.

cl.exe / experimental: external / external: I some_lib_dir / external: W0 / external: templates- / W4 my_prog.cpp

This command allows you to display a warning even though it has a warning level of 0 in the external header file.

Suppress and Force Warnings


The mechanism described above does not turn on and off warnings on its own - it only sets the default level for the specified set of files, and then standard mechanisms for turning on, turning off and suppressing warnings come into play:

  • / wdNNNN, / w1NNNN, / weNNNN, /Wv:XX.YY.ZZZZ etc.
  • #pragma warning (disable: 4507 34; once: 4385; error: 4164)
  • #pragma warning (push [, n]) / #pragma warning (pop)

In addition, when using / external: templates-, you can suppress a warning at the instantiation point. In the example discussed earlier, the user can explicitly suppress the warning issued due to the / external: templates- key :

int main()
{
#pragma warning( suppress : 4245)
  return some_struct().value;
}

Library authors working on the other side can use the same mechanisms to force the inclusion of some or all warnings of a certain level, if they believe that these warnings are critical enough and should not be suppressed with the / external: W switch.

Example:


External header file: some_lib_dir / some_hdr.hpp

#pragma warning( push, 4 )
#pragma warning( error : 4245 )
template 
struct some_struct
{
  static const T value = -7; // W4: warning C4245: 'initializing':
                             // conversion from 'int' to
                             // 'unsigned int', signed/unsigned
                             // mismatch
};
#pragma warning( pop )

By changing the library header file as shown above, the author of the library can be sure that this file will be checked with level 4 no matter what level the user specified in the / external: W parameter, - the compiler will still issue all warnings of level 4 and higher. Moreover, as shown there, you can force a particular warning to be set so that it will always be considered an error, turn off, suppress, or issue once for this header file - and, again, the user will not be able to bypass this setting.

Limitations


In the current version, warnings can be triggered on external header files in those cases when they are issued by the compiler optimizer (and not by the scanner). Such warnings are usually in C47XX format, but not all C47XX messages are issued by the optimizer. As a rule, if the detection of a warning requires analysis of the data flow or control, then most likely it comes from the optimizer in our implementation and so far cannot be suppressed using the new mechanism. We know about this problem, but the solution is unlikely to appear before the next major Visual Studio update, since it involves major changes in the presentation of the intermediate code. However, such warnings can still be disabled in the standard way - using the / wd47XX switch.

In addition, this experimental functionality has not yet been integrated into / analyze warnings , since we want to first examine user feedback. / Analyze warnings are not divided by level, so we are also looking for the best way to combine them with the current logic.

We can’t say yet how this function will work with SDL alerts , but we will keep in touch with the SDL team and let you know as soon as we know.

Conclusion


Returning to the analogy with the theory of broken windows, it should be noted that we ambiguously evaluate the impact of our innovation on the ecosystem of libraries as a whole. On the one hand, it can harm them, since users will no longer consider errors in libraries as their problems, which means they will become less likely to report them or edit them themselves. On the other hand, users will have more control over their own code and will be able to make more stringent requirements on it. Previously, libraries beyond their control prevented this, but now they can be pacified.

Although we recognize the possibility of the mentioned side effect, correcting errors in third-party code is still not the main task of the programmer, since he has his own code that he needs to work on and which should clear out errors, while warnings to third-party libraries prevent this process, because it cannot enable the / WX option only for its files. But, more importantly, we are sure that the negative impact on libraries is offset by another useful consequence.

By allowing developers to hide warnings on third-party libraries, we encourage them to pay more attention to their own code, improve it, and even completely clear warnings at the maximum possible level for them. The authors of third-party libraries are the same developers, another link in this chain, which means that giving them the opportunity to not be distracted by dependencies in third-party code, we also encourage them to closely monitor the quality of their own code and achieve compilation at the maximum warning level possible for them . Etc. Why is it important? The fact is that with the current structure of the software development process, the number of warnings grows like an avalanche as you move along the dependency chain, and the farther you are in this chain, the more difficult it becomes to cope with warnings; developers “break down” under their weight and stop any attempts to rectify the situation. When it is possible to separate your code from someone else’s code, each developer in the chain gets at his disposal funds to stop (block the consequences) of this avalanche of warnings; he has an incentive to minimize its impact on his site, and thereby on the whole chain . This, of course, is a speculative conclusion, but we believe that this effect is no less likely than a negative effect on libraries.

In conclusion, we invite you to experience the new functionality yourself and share your experiences with us. We kindly request: tell us not only about what you liked, but also about what you didn’t like, otherwise an active minority will decide for you. The new mechanism is available in the preview version of Visual Studio 15.6 Preview 1 . As usual, you can contact us by leaving a comment below or by writing an email to us at visualcpp@microsoft.com ; Feedback can be sent using the menu command Help -> Report A Problem in the product or left in the developer community . Follow us on Twitter ( @VisualC ) and Facebook (msftvisualcpp ).

PS Special thanks to Robert Schumacher - he pointed out the similarity of our theory with the theory of broken windows !

Translator Notes


  1. The article is published with the consent of the author. The original article was published on the Visual C ++ Team Blog in English: Broken Warnings Theory .
  2. GitHub author profile: solodon4 .
  3. Discussion of an article on the Reddit website.

Also popular now: