[analyzer] Refactor: make PathDiagnosticLocation responsible for validation of SourceLocations (commit 5 of ?):
- 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 e24544a..0c21788 100644
--- 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 @@
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 @@
*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 166b6fd..9022a1e 100644
--- a/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -175,8 +175,8 @@
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 @@
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 @@
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 @@
}
if (firstCharOnly)
- L = PathDiagnosticLocation(L.asLocation());
+ L.setSingleLocKind();
return L;
}
@@ -908,17 +910,14 @@
~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 @@
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 @@
// 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 @@
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 baef8cc..3356ed4 100644
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -90,8 +90,8 @@
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 495451d..3c798e0 100644
--- a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -130,11 +130,71 @@
// 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 @@
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 @@
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 9972011..b3a0564 100644
--- a/test/Analysis/plist-output-alternate.m
+++ b/test/Analysis/plist-output-alternate.m
@@ -620,7 +620,7 @@
// 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>