[cfe-commits] r151664 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp lib/StaticAnalyzer/Core/PathDiagnostic.cpp

[Ted Kremenek]

Author: kremenek
Date: Tue Feb 28 17:27:39 2012
New Revision: 151664

URL: http://llvm.org/viewvc/llvm-project?rev=151664&view=rev
Log:
[analyzer diagnostics] Refactor filtration for PathDiagnosticConsumers that don't support cross-file diagnostics
into a common place.  Currently enable this filtration for Plist diagnostics as well.

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
    cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=151664&r1=151663&r2=151664&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h Tue Feb 28 17:27:39 2012
@@ -68,6 +68,10 @@
   virtual bool supportsLogicalOpControlFlow() const { return false; }
   virtual bool supportsAllBlockEdges() const { return false; }
   virtual bool useVerboseDescription() const { return true; }
+  
+  /// Return true if the PathDiagnosticConsumer supports individual
+  /// PathDiagnostics that span multiple files.
+  virtual bool supportsCrossFileDiagnostics() const { return false; }
 
 protected:
   bool flushed;

Modified: cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp?rev=151664&r1=151663&r2=151664&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp Tue Feb 28 17:27:39 2012
@@ -145,35 +145,14 @@
   // First flatten out the entire path to make it easier to use.
   PathPieces path;
   flattenPath(path, D.path);
-  
-  const SourceManager &SMgr = (*path.begin())->getLocation().getManager();
-  FileID FID;
 
-  // Verify that the entire path is from the same FileID.
-  for (PathPieces::const_iterator I = path.begin(), E = path.end();
-       I != E; ++I) {
-    FullSourceLoc L = (*I)->getLocation().asLocation().getExpansionLoc();
-
-    if (FID.isInvalid()) {
-      FID = SMgr.getFileID(L);
-    } else if (SMgr.getFileID(L) != FID)
-      return; // FIXME: Emit a warning?
-
-    // Check the source ranges.
-    for (PathDiagnosticPiece::range_iterator RI = (*I)->ranges_begin(),
-                                             RE = (*I)->ranges_end();
-                                             RI != RE; ++RI) {
-      SourceLocation L = SMgr.getExpansionLoc(RI->getBegin());
-      if (!L.isFileID() || SMgr.getFileID(L) != FID)
-        return; // FIXME: Emit a warning?
-      L = SMgr.getExpansionLoc(RI->getEnd());
-      if (!L.isFileID() || SMgr.getFileID(L) != FID)
-        return; // FIXME: Emit a warning?
-    }
-  }
-
-  if (FID.isInvalid())
-    return; // FIXME: Emit a warning?
+  // The path as already been prechecked that all parts of the path are
+  // from the same file and that it is non-empty.
+  const SourceManager &SMgr = (*path.begin())->getLocation().getManager();
+  assert(!path.empty());
+  FileID FID =
+    (*path.begin())->getLocation().asLocation().getExpansionLoc().getFileID();
+  assert(!FID.isInvalid());
 
   // Create a new rewriter to generate HTML.
   Rewriter R(const_cast<SourceManager&>(SMgr), PP.getLangOptions());

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=151664&r1=151663&r2=151664&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Tue Feb 28 17:27:39 2012
@@ -13,6 +13,7 @@
 
 #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclObjC.h"
@@ -76,19 +77,49 @@
 }
 
 void PathDiagnosticConsumer::HandlePathDiagnostic(PathDiagnostic *D) {
-  if (!D)
-    return;
+  llvm::OwningPtr<PathDiagnostic> OwningD(D);
   
-  if (D->path.empty()) {
-    delete D;
+  if (!D || D->path.empty())
     return;
-  }
   
   // We need to flatten the locations (convert Stmt* to locations) because
   // the referenced statements may be freed by the time the diagnostics
   // are emitted.
   D->flattenLocations();
 
+  // If the PathDiagnosticConsumer does not support diagnostics that
+  // cross file boundaries, prune out such diagnostics now.
+  if (!supportsCrossFileDiagnostics()) {
+    // Verify that the entire path is from the same FileID.
+    FileID FID;
+    const SourceManager &SMgr = (*D->path.begin())->getLocation().getManager();
+
+    for (PathPieces::const_iterator I = D->path.begin(), E = D->path.end();
+         I != E; ++I) {
+      FullSourceLoc L = (*I)->getLocation().asLocation().getExpansionLoc();
+      
+      if (FID.isInvalid()) {
+        FID = SMgr.getFileID(L);
+      } else if (SMgr.getFileID(L) != FID)
+        return; // FIXME: Emit a warning?
+      
+      // Check the source ranges.
+      for (PathDiagnosticPiece::range_iterator RI = (*I)->ranges_begin(),
+           RE = (*I)->ranges_end();
+           RI != RE; ++RI) {
+        SourceLocation L = SMgr.getExpansionLoc(RI->getBegin());
+        if (!L.isFileID() || SMgr.getFileID(L) != FID)
+          return; // FIXME: Emit a warning?
+        L = SMgr.getExpansionLoc(RI->getEnd());
+        if (!L.isFileID() || SMgr.getFileID(L) != FID)
+          return; // FIXME: Emit a warning?
+      }
+    }
+    
+    if (FID.isInvalid())
+      return; // FIXME: Emit a warning?
+  }  
+
   // Profile the node to see if we already have something matching it
   llvm::FoldingSetNodeID profile;
   D->Profile(profile);
@@ -110,16 +141,14 @@
           shouldKeepOriginal = false;
       }
 
-      if (shouldKeepOriginal) {
-        delete D;
+      if (shouldKeepOriginal)
         return;
-      }
     }
     Diags.RemoveNode(orig);
     delete orig;
   }
   
-  Diags.InsertNode(D);
+  Diags.InsertNode(OwningD.take());
 }