[analyzer] Refactor: make PathDiagnosticLocation responsible for validation of Source...

 - Get rid of PathDiagnosticLocation(SourceRange r,..) constructor by providing a bunch of create methods.
 - The PathDiagnosticLocation(SourceLocation L,..), which is used by crate methods, will eventually become private.
 - Test difference is in the case when the report starts at the beginning of the function. We used to represent that point as a range of the very first token in the first statement. Now, it's just a single location representing the first character of the first statement.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@139932 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
index e24544a1a95f721b12ca855ee34214444e908ac5..0c21788723e4f57a3c7ed010171f76685d9c5755 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -23,6 +23,8 @@
 
 namespace clang {
 
+class BinaryOperator;
+class CompoundStmt;
 class Decl;
 class LocationContext;
 class ProgramPoint;
@@ -98,21 +100,48 @@ public:
   PathDiagnosticLocation(FullSourceLoc L)
     : K(SingleLocK), R(L, L), S(0), D(0), SM(&L.getManager()), LC(0) {}
 
-  /// Constructs a location when no specific statement is available.
-  /// Defaults to end of brace for the enclosing function body.
-  PathDiagnosticLocation(const LocationContext *lc, const SourceManager &sm);
-  
+  PathDiagnosticLocation(SourceLocation L, const SourceManager &sm,
+                         Kind kind = SingleLocK)
+    : K(kind), R(L, L), S(0), D(0), SM(&sm), LC(0) {}
+
   PathDiagnosticLocation(const Stmt *s,
                          const SourceManager &sm,
                          const LocationContext *lc)
     : K(StmtK), S(s), D(0), SM(&sm), LC(lc) {}
 
-  PathDiagnosticLocation(SourceRange r, const SourceManager &sm)
-    : K(RangeK), R(r), S(0), D(0), SM(&sm), LC(0) {}
-
   PathDiagnosticLocation(const Decl *d, const SourceManager &sm)
     : K(DeclK), S(0), D(d), SM(&sm), LC(0) {}
 
+  // Create a location for the beginning of the statement.
+  static PathDiagnosticLocation createBeginStmt(const Stmt *S,
+                                                const SourceManager &SM,
+                                                const LocationContext *LC);
+
+  /// Create the location for the operator of the binary expression.
+  /// Assumes the statement has a valid location.
+  static PathDiagnosticLocation createOperatorLoc(const BinaryOperator *BO,
+                                                  const SourceManager &SM);
+
+  /// Create a location for the beginning of the compound statement.
+  /// Assumes the statement has a valid location.
+  static PathDiagnosticLocation createBeginBrace(const CompoundStmt *CS,
+                                                 const SourceManager &SM);
+
+  /// Create a location for the end of the compound statement.
+  /// Assumes the statement has a valid location.
+  static PathDiagnosticLocation createEndBrace(const CompoundStmt *CS,
+                                               const SourceManager &SM);
+
+  /// Create a location for the beginning of the enclosing declaration body.
+  /// Defaults to the beginning of the first statement in the declaration body.
+  static PathDiagnosticLocation createDeclBegin(const LocationContext *LC,
+                                                const SourceManager &SM);
+
+  /// Constructs a location for the end of the enclosing declaration body.
+  /// Defaults to the end of brace.
+  static PathDiagnosticLocation createDeclEnd(const LocationContext *LC,
+                                                   const SourceManager &SM);
+
   /// Create a location corresponding to the given valid ExplodedNode.
   PathDiagnosticLocation(const ProgramPoint& P, const SourceManager &SMng);
 
@@ -144,6 +173,16 @@ public:
     *this = PathDiagnosticLocation();
   }
 
+  /// Specify that the object represents a single location.
+  void setSingleLocKind() {
+    if (K == SingleLocK)
+      return;
+
+    SourceLocation L = asLocation();
+    K = SingleLocK;
+    R = SourceRange(L, L);
+  }
+
   void flatten();
 
   const SourceManager& getManager() const { assert(isValid()); return *SM; }

diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp
index 166b6fdd09e05102dd6c5d92bb7ff666d7d10861..9022a1e6f9d8b210b724476aaade9177c78880d5 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -175,8 +175,8 @@ PathDiagnosticBuilder::ExecutionContinues(const ExplodedNode *N) {
   if (const Stmt *S = GetNextStmt(N))
     return PathDiagnosticLocation(S, getSourceManager(), getLocationContext());
 
-  return FullSourceLoc(N->getLocationContext()->getDecl()->getBodyRBrace(),
-                       getSourceManager());
+  return PathDiagnosticLocation::createDeclEnd(N->getLocationContext(),
+                                               getSourceManager());
 }
 
 PathDiagnosticLocation
@@ -665,7 +665,8 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
             if (*(Src->succ_begin()+1) == Dst) {
               os << "false";
               PathDiagnosticLocation End(B->getLHS(), SMgr, LC);
-              PathDiagnosticLocation Start(B->getOperatorLoc(), SMgr);
+              PathDiagnosticLocation Start =
+                PathDiagnosticLocation::createOperatorLoc(B, SMgr);
               PD.push_front(new PathDiagnosticControlFlowPiece(Start, End,
                                                                os.str()));
             }
@@ -691,7 +692,8 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
             else {
               os << "true";
               PathDiagnosticLocation End(B->getLHS(), SMgr, LC);
-              PathDiagnosticLocation Start(B->getOperatorLoc(), SMgr);
+              PathDiagnosticLocation Start =
+                PathDiagnosticLocation::createOperatorLoc(B, SMgr);
               PD.push_front(new PathDiagnosticControlFlowPiece(Start, End,
                                                                os.str()));
             }
@@ -879,7 +881,7 @@ class EdgeBuilder {
     }
 
     if (firstCharOnly)
-      L = PathDiagnosticLocation(L.asLocation());
+      L.setSingleLocKind();
 
     return L;
   }
@@ -908,17 +910,14 @@ public:
 
   ~EdgeBuilder() {
     while (!CLocs.empty()) popLocation();
-
+    
     // Finally, add an initial edge from the start location of the first
     // statement (if it doesn't already exist).
-    // FIXME: Should handle CXXTryStmt if analyser starts supporting C++.
-    if (const CompoundStmt *CS =
-          dyn_cast_or_null<CompoundStmt>(PDB.getCodeDecl().getBody()))
-      if (!CS->body_empty()) {
-        SourceLocation Loc = (*CS->body_begin())->getLocStart();
-        rawAddEdge(PathDiagnosticLocation(Loc, PDB.getSourceManager()));
-      }
-
+    PathDiagnosticLocation L = PathDiagnosticLocation::createDeclBegin(
+                                                       PDB.getLocationContext(),
+                                                       PDB.getSourceManager());
+    if (L.isValid())
+      rawAddEdge(L);
   }
 
   void addEdge(PathDiagnosticLocation NewLoc, bool alwaysAdd = false);
@@ -1117,6 +1116,7 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
                                             PathDiagnosticBuilder &PDB,
                                             const ExplodedNode *N) {
   EdgeBuilder EB(PD, PDB);
+  const SourceManager& SM = PDB.getSourceManager();
 
   const ExplodedNode *NextNode = N->pred_empty() ? NULL : *(N->pred_begin());
   while (NextNode) {
@@ -1132,8 +1132,7 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
 
         // Are we jumping to the head of a loop?  Add a special diagnostic.
         if (const Stmt *Loop = BE->getDst()->getLoopTarget()) {
-          PathDiagnosticLocation L(Loop, PDB.getSourceManager(),
-                                         PDB.getLocationContext());
+          PathDiagnosticLocation L(Loop, SM, PDB.getLocationContext());
           const CompoundStmt *CS = NULL;
 
           if (!Term) {
@@ -1151,9 +1150,8 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
           PD.push_front(p);
 
           if (CS) {
-            PathDiagnosticLocation BL(CS->getRBracLoc(),
-                                      PDB.getSourceManager());
-            BL = PathDiagnosticLocation(BL.asLocation());
+            PathDiagnosticLocation BL =
+              PathDiagnosticLocation::createEndBrace(CS, SM);
             EB.addEdge(BL);
           }
         }

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index baef8cc9c042fc1a5d82c50dc9442aad3e33d6a1..3356ed4eac7d5f87e9898e832834bb18e4b4cb2c 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -90,8 +90,8 @@ BugReporterVisitor::getDefaultEndPath(BugReporterContext &BRC,
   if (const BlockEntrance *BE = dyn_cast<BlockEntrance>(&PP)) {
     const CFGBlock *block = BE->getBlock();
     if (block->getBlockID() == 0) {
-      L = PathDiagnosticLocation(PP.getLocationContext(),
-                                 BRC.getSourceManager());
+      L = PathDiagnosticLocation::createDeclEnd(PP.getLocationContext(),
+                                                BRC.getSourceManager());
     }
   }
 

diff --git a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
index 495451d261e49c63564f65b1757722f1e003b12c..3c798e0facb2ccfe3bce8d43b0a0fc2a645a7a9a 100644 (file)
--- a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -130,11 +130,71 @@ void PathDiagnosticClient::HandlePathDiagnostic(const PathDiagnostic *D) {
 // PathDiagnosticLocation methods.
 //===----------------------------------------------------------------------===//
 
-PathDiagnosticLocation::PathDiagnosticLocation(const LocationContext *lc,
-                                               const SourceManager &sm)
-  : K(SingleLocK), S(0), D(0), SM(&sm), LC(lc) {
+static SourceLocation getValidSourceLocation(const Stmt* S,
+                                             const LocationContext *LC) {
+  assert(LC);
+  SourceLocation L = S->getLocStart();
+
+  // S might be a temporary statement that does not have a location in the
+  // source code, so find an enclosing statement and use it's location.
+  if (!L.isValid()) {
+    ParentMap & PM = LC->getParentMap();
+
+    while (!L.isValid()) {
+      S = PM.getParent(S);
+      L = S->getLocStart();
+    }
+  }
+
+  return L;
+}
+
+PathDiagnosticLocation
+  PathDiagnosticLocation::createBeginStmt(const Stmt *S,
+                                          const SourceManager &SM,
+                                          const LocationContext *LC) {
+  return PathDiagnosticLocation(getValidSourceLocation(S, LC), SM, SingleLocK);
+}
+
+PathDiagnosticLocation
+  PathDiagnosticLocation::createOperatorLoc(const BinaryOperator *BO,
+                                            const SourceManager &SM) {
+  return PathDiagnosticLocation(BO->getOperatorLoc(), SM, SingleLocK);
+}
+
+PathDiagnosticLocation
+  PathDiagnosticLocation::createBeginBrace(const CompoundStmt *CS,
+                                           const SourceManager &SM) {
+  SourceLocation L = CS->getLBracLoc();
+  return PathDiagnosticLocation(L, SM, SingleLocK);
+}
+
+PathDiagnosticLocation
+  PathDiagnosticLocation::createEndBrace(const CompoundStmt *CS,
+                                         const SourceManager &SM) {
+  SourceLocation L = CS->getRBracLoc();
+  return PathDiagnosticLocation(L, SM, SingleLocK);
+}
+
+PathDiagnosticLocation
+  PathDiagnosticLocation::createDeclBegin(const LocationContext *LC,
+                                          const SourceManager &SM) {
+  // FIXME: Should handle CXXTryStmt if analyser starts supporting C++.
+  if (const CompoundStmt *CS =
+        dyn_cast_or_null<CompoundStmt>(LC->getDecl()->getBody()))
+    if (!CS->body_empty()) {
+      SourceLocation Loc = (*CS->body_begin())->getLocStart();
+      return PathDiagnosticLocation(Loc, SM, SingleLocK);
+    }
+
+  return PathDiagnosticLocation();
+}
+
+PathDiagnosticLocation
+  PathDiagnosticLocation::createDeclEnd(const LocationContext *LC,
+                                             const SourceManager &SM) {
   SourceLocation L = LC->getDecl()->getBodyRBrace();
-  R = SourceRange(L, L);
+  return PathDiagnosticLocation(L, SM, SingleLocK);
 }
 
 PathDiagnosticLocation::PathDiagnosticLocation(const ProgramPoint& P,
@@ -153,9 +213,9 @@ PathDiagnosticLocation::PathDiagnosticLocation(const ProgramPoint& P,
     invalidate();
 }
 
-PathDiagnosticLocation PathDiagnosticLocation::createEndOfPath(
-                                                      const ExplodedNode* N,
-                                                      const SourceManager &SM) {
+PathDiagnosticLocation
+  PathDiagnosticLocation::createEndOfPath(const ExplodedNode* N,
+                                          const SourceManager &SM) {
   assert(N && "Cannot create a location with a null node.");
 
   const ExplodedNode *NI = N;
@@ -174,26 +234,7 @@ PathDiagnosticLocation PathDiagnosticLocation::createEndOfPath(
     NI = NI->succ_empty() ? 0 : *(NI->succ_begin());
   }
 
-  return PathDiagnosticLocation(N->getLocationContext(), SM);
-}
-
-static SourceLocation getValidSourceLocation(const Stmt* S,
-                                             const LocationContext *LC) {
-  assert(LC);
-  SourceLocation L = S->getLocStart();
-
-  // S might be a temporary statement that does not have a location in the
-  // source code, so find an enclosing statement and use it's location.
-  if (!L.isValid()) {
-    ParentMap & PM = LC->getParentMap();
-
-    while (!L.isValid()) {
-      S = PM.getParent(S);
-      L = S->getLocStart();
-    }
-  }
-
-  return L;
+  return createDeclEnd(N->getLocationContext(), SM);
 }
 
 FullSourceLoc PathDiagnosticLocation::asLocation() const {

diff --git a/test/Analysis/plist-output-alternate.m b/test/Analysis/plist-output-alternate.m
index 9972011500127b146939eb74a2dd16b50acc02c8..b3a0564289f9f6795cc4b87f3b379e4f8344be10 100644 (file)
--- a/test/Analysis/plist-output-alternate.m
+++ b/test/Analysis/plist-output-alternate.m
@@ -620,7 +620,7 @@ void rdar8331641(int x) {
 // CHECK:           </dict>
 // CHECK:           <dict>
 // CHECK:            <key>line</key><integer>35</integer>
-// CHECK:            <key>col</key><integer>8</integer>
+// CHECK:            <key>col</key><integer>3</integer>
 // CHECK:            <key>file</key><integer>0</integer>
 // CHECK:           </dict>
 // CHECK:          </array>