Tools: Comments that outlived errors

Introduction ## What's under the hood ## Cross-platform compatibility ## Avoiding source file modification as long as possible ## How to use it ## What markers should be removed ## How to collect information about redundant markers ## How to automatically delete false alarm markers ## How to merge reports ## Preview ## Our little study ## PVS-Studio check results ## Unreal Engine check results ## Summary table ## Markers we found: the Unreal Engine example ## Recommendation ## Conclusion Many static analyzers enable suppressing individual warnings directly in code via special comments. Over time, the number of such comments in projects increases. Some of them lose their relevance yet remain in the code, like dusty magnets on a fridge. In this article, we'll examine the scope of the issue. We selected a few projects and checked them using the new PVS-Studio feature, which can detect redundant false alarm markers. Let's take a look at the results. There are several reasons why false alarm markers can appear in code. Sometimes, developers choose not to address a warning right away. They think, "I'll look at it later when I have time." Picture 2. Avengers: Endgame / Marvel Studios Another possibility is that the analyzer got it wrong. This does happen: the tool detects an issue that doesn't actually exist, developers check the code and confirm that everything is fine, then, they add a false alarm marker to suppress the warning. However, analyzers are evolving, old false positives are disappearing, while false alarm markers remain. So, the marker that once worked so well may now be hiding real warnings. Over time, developers end up with a whole collection of these artifacts, and if they aren't natural collectors, it's better to get rid of them. Our task was to create a utility that, based on an analysis report, would locate special markers in the codebase and delete the irrelevant ones. We considered two approaches for its implementation. The first one relied on a utility that would just recursively scan the specified directory, find comments in all files, and delete them based on the analysis report. At first glance, it sounded simple. Still, there were some complications, of course. Firstly, the code can contain false alarm markers in its various sections: 1. At the end of a string: 2. Among other markers: 4. In a string literal: In other words, the utility should understand the context of each text file, including the programming language used. Secondly, is it necessary to check all files? At first glance, it might seem enough to scan only text files, but which of them end up in the actual program? Projects can be cross-platform, and files involved in compilation on one platform may not be used at all on another. These two details alone made it clear that we would have to take a different path. We wanted to create a lightweight utility that simply removes comments from source code. We approached the task from a different angle. The analyzer already displays the warnings necessary for the utility to work. It already collects information about false alarm comments. And it fully understands the context of the programming language, meaning it can distinguish between a string literal containing a comment and the control comment itself. So, if we let the analyzer decide which comments to delete? Here's what it gives us: This approach has just a couple of drawbacks: In the end, the pros outweighed the cons. Next, the utility simply reads the report and deletes the markers at the specified "coordinates." So far, we've assumed that the project being analyzed runs on a single platform. In reality, most projects are cross-platform, and warnings may differ between platforms. A marker may be irrelevant on one platform but still suppress a warning on another. To handle this, we added a report merge mode to the utility. If all reports contain information about a redundant marker, you can still delete it. Otherwise, the marker should remain in the source code. Now that everything is ready and the information about redundant markers has been collected, the file can be edited. Or not? First of all, it's better to make sure that the file hasn't changed since the last analysis run. Fortunately, we've added the checksum for this purpose. What if something goes wrong, though? Nobody wants to mess up their source files. That's why a copy of the source file is created, the modifications are applied to this copy according to the input data, and the original file is then replaced automatically. If the process is interrupted, the source file remains unchanged. To address the issue described above, we added a special tool to PVS-Studio—pvs-fp-cleaner. To suppress false positive warnings, PVS-Studio supports the following control comments: The warning-number is the diagnostic rule number. If you're using a hash code in the line to suppress warnings (the suppression is removed if the original line changes), the utility also handles these markers: The hash is the hash code of a line excluding comments. The first step is to analyze the project in a special mode that collects data on potentially redundant false alarm markers: The result is a report containing information about redundant false alarm markers based on data collected during the analysis. Once the reports are ready, we can move on to the most important part: cleaning up the codebase. To do this, we'll use pvs-fp-cleaner. Since warnings may differ across platforms in cross-platform projects, we need to specify the reports received for each platform to ensure the utility works correctly. If multiple reports are generated for a cross-platform project, and it's necessary to work with a single set of data, use the merge mode: Before removing false alarm markers, it may be useful to assess the scale of changes in the code. For this reason, we've added a preview mode for generating reports: The report can be opened in the IDE: We tested the utility on three different projects: Now, let's see how many false alarm markers are still kicking, and how many should've been retired long ago. We'll start with our tool. Here are the results: In other words, about half of the markers in the project were redundant. This clearly demonstrates the analyzer's progress over the years. Moving on to Unreal Engine. The percentage of redundant markers here is much lower than in PVS-Studio, but in absolute terms, the number still amounts to several hundreds. UE also has a complex codebase with extensive use of macros and templates, which results in more false positives. In most cases, everything is quite predictable. For example, here's a single marker: And here are multiple markers in a row: In such cases, it's pretty simple: if warnings stop popping up, delete the marker. Sometimes, however, there are more abstract cases, for example: The marker is inside the comment. Technically, it's redundant, but contextually, it's part of the text. This sparked a lively debate in our team: some felt the marker should be removed since suppression was no longer relevant, while others argued it was a "comment within a comment" and should be left untouched. Eventually, the first group prevailed, and now the utility removes such markers. What do you think? Share your thoughts in the comments! Here's another similar case: The utility removes the marker while leaving the rest of the text intact: Given these cases, we recommend running pvs-fp-cleaner under the supervision of a version control system. In ambiguous situations, it's better to check the diffs beforehand, consider the changes, and only then hit commit. The results show that hundreds of redundant markers can accumulate in actively developed projects. This illustrates the analyzer evolution effect: tools grow smarter, but redundant false alarm markers persist. The automated cleanup of such markers reduces noise, simplifies code maintenance, and lowers the risk that a real warning will be obscured by a long-forgotten marker. The feature for detecting and removing redundant false alarm markers is available in PVS-Studio starting with the 7.41 version. Templates let you quickly answer FAQs or store snippets for re-use. Are you sure you want to hide this comment? It will become hidden in your post, but will still be visible via the comment's permalink. Hide child comments as well For further actions, you may consider blocking this person and/or reporting abuse CODE_BLOCK: int a1; //-V861 Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: int a1; //-V861 CODE_BLOCK: int a1; //-V861 CODE_BLOCK: int a2; //-V561 //-V861 //-V773 Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: int a2; //-V561 //-V861 //-V773 CODE_BLOCK: int a2; //-V561 //-V861 //-V773 CODE_BLOCK: int a3; /* comment //-V861 */ Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: int a3; /* comment //-V861 */ CODE_BLOCK: int a3; /* comment //-V861 */ CODE_BLOCK: std::string a4 = "//-V861"; //-V861 std::string a5 = "hello //-V861 \" world"; //-V861 Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: std::string a4 = "//-V861"; //-V861 std::string a5 = "hello //-V861 \" world"; //-V861 CODE_BLOCK: std::string a4 = "//-V861"; //-V861 std::string a5 = "hello //-V861 \" world"; //-V861 CODE_BLOCK: { "version": 1, "files": [ { "path": "/path/to/file1", "hash": "d82b7bac944b7da5c9a13d0c48d285345368790f", "lines": [ { "line": 1, "markers": [ { "code": 567, "columns": { "begin": 1, "end": 2 }, "offsets": { "begin": 1, "end": 2 } }, { "code": 730, "columns": { "begin": 34, "end": 43 }, "offsets": { "begin": 34, "end": 47 } } ] }, { "line": 10, "markers": [ { "code": 568, "columns": { "begin": 3, "end": 7 }, "offsets": { "begin": 3, "end": 7 } }, { "code": 609, "columns": { "begin": 9, "end": 15 }, "offsets": { "begin": 9, "end": 20 } } ] } ] }, { "path": "/path/to/file2", "hash": "9bc0ced475c2df7103f34db6c1b407b7db79b4a4", "lines": [ { "line": 1, "markers": [ { "code": 557, "columns": { "begin": 5, "end": 8 }, "offsets": { "begin": 5, "end": 8 } }, { "code": 777, "columns": { "begin": 63, "end": 79 }, "offsets": { "begin": 63, "end": 84 } } ] }, { "line": 10, "markers": [ { "code": 501, "columns": { "begin": 20, "end": 25 }, "offsets": { "begin": 20, "end": 27 } }, { "code": 509, "columns": { "begin": 33, "end": 39 }, "offsets": { "begin": 33, "end": 42 } } ] } ] } ] } Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: { "version": 1, "files": [ { "path": "/path/to/file1", "hash": "d82b7bac944b7da5c9a13d0c48d285345368790f", "lines": [ { "line": 1, "markers": [ { "code": 567, "columns": { "begin": 1, "end": 2 }, "offsets": { "begin": 1, "end": 2 } }, { "code": 730, "columns": { "begin": 34, "end": 43 }, "offsets": { "begin": 34, "end": 47 } } ] }, { "line": 10, "markers": [ { "code": 568, "columns": { "begin": 3, "end": 7 }, "offsets": { "begin": 3, "end": 7 } }, { "code": 609, "columns": { "begin": 9, "end": 15 }, "offsets": { "begin": 9, "end": 20 } } ] } ] }, { "path": "/path/to/file2", "hash": "9bc0ced475c2df7103f34db6c1b407b7db79b4a4", "lines": [ { "line": 1, "markers": [ { "code": 557, "columns": { "begin": 5, "end": 8 }, "offsets": { "begin": 5, "end": 8 } }, { "code": 777, "columns": { "begin": 63, "end": 79 }, "offsets": { "begin": 63, "end": 84 } } ] }, { "line": 10, "markers": [ { "code": 501, "columns": { "begin": 20, "end": 25 }, "offsets": { "begin": 20, "end": 27 } }, { "code": 509, "columns": { "begin": 33, "end": 39 }, "offsets": { "begin": 33, "end": 42 } } ] } ] } ] } CODE_BLOCK: { "version": 1, "files": [ { "path": "/path/to/file1", "hash": "d82b7bac944b7da5c9a13d0c48d285345368790f", "lines": [ { "line": 1, "markers": [ { "code": 567, "columns": { "begin": 1, "end": 2 }, "offsets": { "begin": 1, "end": 2 } }, { "code": 730, "columns": { "begin": 34, "end": 43 }, "offsets": { "begin": 34, "end": 47 } } ] }, { "line": 10, "markers": [ { "code": 568, "columns": { "begin": 3, "end": 7 }, "offsets": { "begin": 3, "end": 7 } }, { "code": 609, "columns": { "begin": 9, "end": 15 }, "offsets": { "begin": 9, "end": 20 } } ] } ] }, { "path": "/path/to/file2", "hash": "9bc0ced475c2df7103f34db6c1b407b7db79b4a4", "lines": [ { "line": 1, "markers": [ { "code": 557, "columns": { "begin": 5, "end": 8 }, "offsets": { "begin": 5, "end": 8 } }, { "code": 777, "columns": { "begin": 63, "end": 79 }, "offsets": { "begin": 63, "end": 84 } } ] }, { "line": 10, "markers": [ { "code": 501, "columns": { "begin": 20, "end": 25 }, "offsets": { "begin": 20, "end": 27 } }, { "code": 509, "columns": { "begin": 33, "end": 39 }, "offsets": { "begin": 33, "end": 42 } } ] } ] } ] } CODE_BLOCK: //-Vwarning-number Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: //-Vwarning-number CODE_BLOCK: //-Vwarning-number CODE_BLOCK: if (a == b && a == b && 0 / 0 == 0) //-V501 //-V609 Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: if (a == b && a == b && 0 / 0 == 0) //-V501 //-V609 CODE_BLOCK: if (a == b && a == b && 0 / 0 == 0) //-V501 //-V609 CODE_BLOCK: //-VH"hash" Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: //-VH"hash" CODE_BLOCK: //-VH"hash" CODE_BLOCK: if (a == b && a == b && 0 / 0 == 0) //-V501 //-V609 //-VH"12345678" Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: if (a == b && a == b && 0 / 0 == 0) //-V501 //-V609 //-VH"12345678" CODE_BLOCK: if (a == b && a == b && 0 / 0 == 0) //-V501 //-V609 //-VH"12345678" CODE_BLOCK: pvs-studio-analyzer analyze --redundant-false-alarms=/path/to/report.json \ --sourcetree-root=/path/to/project-root \ .... Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: pvs-studio-analyzer analyze --redundant-false-alarms=/path/to/report.json \ --sourcetree-root=/path/to/project-root \ .... CODE_BLOCK: pvs-studio-analyzer analyze --redundant-false-alarms=/path/to/report.json \ --sourcetree-root=/path/to/project-root \ .... CODE_BLOCK: pvs-fp-cleaner cleanup \ --sourcetree-root=/path/to/project \ PATH... Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: pvs-fp-cleaner cleanup \ --sourcetree-root=/path/to/project \ PATH... CODE_BLOCK: pvs-fp-cleaner cleanup \ --sourcetree-root=/path/to/project \ PATH... CODE_BLOCK: pvs-fp-cleaner cleanup \ --sourcetree-root=/home/user/project \ /home/user/windows_report.json \ /home/user/linux_report.json \ /home/user/macOS_report.json Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: pvs-fp-cleaner cleanup \ --sourcetree-root=/home/user/project \ /home/user/windows_report.json \ /home/user/linux_report.json \ /home/user/macOS_report.json CODE_BLOCK: pvs-fp-cleaner cleanup \ --sourcetree-root=/home/user/project \ /home/user/windows_report.json \ /home/user/linux_report.json \ /home/user/macOS_report.json CODE_BLOCK: pvs-fp-cleaner merge \ --sourcetree-root=/path/to/project \ --output-file=/path/to/merged_report.json \ PATH... Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: pvs-fp-cleaner merge \ --sourcetree-root=/path/to/project \ --output-file=/path/to/merged_report.json \ PATH... CODE_BLOCK: pvs-fp-cleaner merge \ --sourcetree-root=/path/to/project \ --output-file=/path/to/merged_report.json \ PATH... CODE_BLOCK: pvs-fp-cleaner report \ --sourcetree-root=/home/user/project \ --output-file=/home/user/report_for_IDE_plugin.json \ PATH... Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: pvs-fp-cleaner report \ --sourcetree-root=/home/user/project \ --output-file=/home/user/report_for_IDE_plugin.json \ PATH... CODE_BLOCK: pvs-fp-cleaner report \ --sourcetree-root=/home/user/project \ --output-file=/home/user/report_for_IDE_plugin.json \ PATH... CODE_BLOCK: if ( Name.Contains(CtrlPrefix) && !DynamicHierarchy->Contains(....)) //-V1051 Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: if ( Name.Contains(CtrlPrefix) && !DynamicHierarchy->Contains(....)) //-V1051 CODE_BLOCK: if ( Name.Contains(CtrlPrefix) && !DynamicHierarchy->Contains(....)) //-V1051 CODE_BLOCK: for (....) //-V621 //-V654 Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: for (....) //-V621 //-V654 CODE_BLOCK: for (....) //-V621 //-V654 CODE_BLOCK: // The warning disable comment can can't be used in a macro: //-V501 Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: // The warning disable comment can can't be used in a macro: //-V501 CODE_BLOCK: // The warning disable comment can can't be used in a macro: //-V501 CODE_BLOCK: LinkerSave.AdditionalDataToAppend.Add(....); \ // -V595 PVS believes that LinkerSave can potentially be nullptr at Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: LinkerSave.AdditionalDataToAppend.Add(....); \ // -V595 PVS believes that LinkerSave can potentially be nullptr at CODE_BLOCK: LinkerSave.AdditionalDataToAppend.Add(....); \ // -V595 PVS believes that LinkerSave can potentially be nullptr at CODE_BLOCK: LinkerSave.AdditionalDataToAppend.Add(....); \ // PVS believes that LinkerSave can potentially be nullptr at Enter fullscreen mode Exit fullscreen mode CODE_BLOCK: LinkerSave.AdditionalDataToAppend.Add(....); \ // PVS believes that LinkerSave can potentially be nullptr at CODE_BLOCK: LinkerSave.AdditionalDataToAppend.Add(....); \ // PVS believes that LinkerSave can potentially be nullptr at - easy implementation: the utility just needs to read the information about redundant comments and delete them; - access to analysis configuration information, which lets us adjust the output—for example, keep markers for disabled diagnostic rules; - extensibility: the utility is independent of any specific language syntax. If the analyzer learns to work with a new language and transfer data in a standard format, the utility will be able to clean up comments without requiring any code modifications. - It requires the full project analysis. If the analysis is integrated into CI/CD, so the report is available only after an overnight run. - It's impossible to automatically delete comments from files not included in the analysis. - We grouped the information by file. This allows us to handle them all at once. - We added a checksum for each file. These steps helped resolve the following issues: if the utility is run twice on the same report, the source files won't change in later runs; if the utility is interrupted while running, but a user wants it to continue until it finishes. - if the utility is run twice on the same report, the source files won't change in later runs; - if the utility is interrupted while running, but a user wants it to continue until it finishes. - We saved marker positions via byte shifts. This enabled the utility to ignore file encodings and language syntax. - if the utility is run twice on the same report, the source files won't change in later runs; - if the utility is interrupted while running, but a user wants it to continue until it finishes. - --redundant-false-alarms is a path to the report; - --sourcetree-root is the project root for correct relative path generation. - cleanup is the cleanup mode; - --sourcetree-root is a root project directory; - PATH... is a report list. - merge is the merge mode; - --output-file is a final report. - Of course, PVS-Studio—after all, if we don't check it ourselves, who will? - Unreal Engine, which is a huge project with a rich history. - Total number of false alarm markers: 744 - Redundant markers: 324 - Total number of false alarm markers: 2 215 - Redundant markers: 270