[PATCH] D24278: [analyzer] Extend bug reports with extra notes.

Aleksei Sidorin via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 9 11:25:51 PDT 2016


a.sidorin added a comment.

This definitely seems to be useful. However, this patch is pretty big. Some of its parts are not directly related with the feature being introduced (for example, changes for copypaste/sub-sequences.cpp). Is it possible to split this patch? Moreover, as I understand, you may not even need a review for refactoring-only changes. Or you can make a review for them which will be done much faster.
There is a temporary dump of some stylish comments after brief look.


================
Comment at: lib/StaticAnalyzer/Core/BugReporter.cpp:3449
@@ +3448,3 @@
+    // we may optionally convert those to path notes.
+    for (auto I = exampleReport->getExtraNotes().rbegin(),
+              E = exampleReport->getExtraNotes().rend(); I != E; ++I) {
----------------
Reverse iteration on array and push the corresponding element to the front (always O(n)) seems a bit strange to me. I suggest using a combination of resize + move existing elements to the end + copy/transform from start. What do you think? Or am I missing something?

================
Comment at: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:230
@@ +229,3 @@
+    unsigned NumExtraPieces = 0;
+    for (auto I = path.begin(), E = path.end(); I != E; ++I) {
+      if (const auto *P = dyn_cast<PathDiagnosticExtraNotePiece>(I->get())) {
----------------
const auto &Piece : path?

================
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:117
@@ +116,3 @@
+      // First, add extra notes, even if paths should not be included.
+      for (PathPieces::const_iterator PI = PD->path.begin(),
+                                      PE = PD->path.end(); PI != PE; ++PI) {
----------------
Range-for loop may look nicer here. What do you think?

================
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:119
@@ +118,3 @@
+                                      PE = PD->path.end(); PI != PE; ++PI) {
+        if (!isa<PathDiagnosticExtraNotePiece>(PI->get()))
+          continue;
----------------
if (isa<...>()) {
  ...
  ...
}
?


https://reviews.llvm.org/D24278





More information about the cfe-commits mailing list