[PATCH] D42721: [analyzer] NFC: Use construction contexts for finding the target region for the construction.
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL324800: [analyzer] NFC: Use CFG construction contexts
instead of homemade lookahead. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D42721?vs=133732=133735#toc
Repository:
rL LLVM
https://reviews.llvm.org/D42721
Files:
cfe/trunk/include/clang/Analysis/CFG.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -114,12 +114,14 @@
const LocationContext *LCtx = Pred->getLocationContext();
ProgramStateRef State = Pred->getState();
- // See if we're constructing an existing region by looking at the next
- // element in the CFG.
-
- if (auto Elem = findElementDirectlyInitializedByCurrentConstructor()) {
-if (Optional StmtElem = Elem->getAs()) {
- if (const CXXNewExpr *CNE = dyn_cast(StmtElem->getStmt())) {
+ // See if we're constructing an existing region by looking at the
+ // current construction context.
+ const NodeBuilderContext = getBuilderContext();
+ const CFGBlock *B = CurrBldrCtx.getBlock();
+ const CFGElement = (*B)[currStmtIdx];
+ if (auto CC = E.getAs()) {
+if (const Stmt *TriggerStmt = CC->getTriggerStmt()) {
+ if (const CXXNewExpr *CNE = dyn_cast(TriggerStmt)) {
if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
// TODO: Detect when the allocator returns a null pointer.
// Constructor shall not be called in this case.
@@ -135,7 +137,7 @@
return MR;
}
}
- } else if (auto *DS = dyn_cast(StmtElem->getStmt())) {
+ } else if (auto *DS = dyn_cast(TriggerStmt)) {
if (const auto *Var = dyn_cast(DS->getSingleDecl())) {
if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) {
SVal LValue = State->getLValue(Var, LCtx);
@@ -145,11 +147,9 @@
return LValue.getAsRegion();
}
}
- } else {
-llvm_unreachable("Unexpected directly initialized element!");
}
-} else if (Optional InitElem = Elem->getAs()) {
- const CXXCtorInitializer *Init = InitElem->getInitializer();
+ // TODO: Consider other directly initialized elements.
+} else if (const CXXCtorInitializer *Init = CC->getTriggerInit()) {
assert(Init->isAnyMemberInitializer());
const CXXMethodDecl *CurCtor = cast(LCtx->getDecl());
Loc ThisPtr =
@@ -183,53 +183,6 @@
return MRMgr.getCXXTempObjectRegion(CE, LCtx);
}
-/// Returns true if the initializer for \Elem can be a direct
-/// constructor.
-static bool canHaveDirectConstructor(CFGElement Elem){
- // DeclStmts and CXXCtorInitializers for fields can be directly constructed.
-
- if (Optional StmtElem = Elem.getAs()) {
-if (isa(StmtElem->getStmt())) {
- return true;
-}
-if (isa(StmtElem->getStmt())) {
- return true;
-}
- }
-
- if (Elem.getKind() == CFGElement::Initializer) {
-return true;
- }
-
- return false;
-}
-
-Optional
-ExprEngine::findElementDirectlyInitializedByCurrentConstructor() {
- const NodeBuilderContext = getBuilderContext();
- // See if we're constructing an existing region by looking at the next
- // element in the CFG.
- const CFGBlock *B = CurrBldrCtx.getBlock();
- assert(isa(((*B)[currStmtIdx]).castAs().getStmt()));
- unsigned int NextStmtIdx = currStmtIdx + 1;
- if (NextStmtIdx >= B->size())
-return None;
-
- CFGElement Next = (*B)[NextStmtIdx];
-
- // Is this a destructor? If so, we might be in the middle of an assignment
- // to a local or member: look ahead one more element to see what we find.
- while (Next.getAs() && NextStmtIdx + 1 < B->size()) {
-++NextStmtIdx;
-Next = (*B)[NextStmtIdx];
- }
-
- if (canHaveDirectConstructor(Next))
-return Next;
-
- return None;
-}
-
const CXXConstructExpr *
ExprEngine::findDirectConstructorForCurrentCFGElement() {
// Go backward in the CFG to see if the previous element (ignoring
@@ -241,7 +194,6 @@
return nullptr;
const CFGBlock *B = getBuilderContext().getBlock();
- assert(canHaveDirectConstructor((*B)[currStmtIdx]));
unsigned int PreviousStmtIdx = currStmtIdx - 1;
CFGElement Previous = (*B)[PreviousStmtIdx];
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -665,13 +665,6 @@
/// constructing
[PATCH] D42721: [analyzer] NFC: Use construction contexts for finding the target region for the construction.
NoQ updated this revision to Diff 133732.
NoQ added a comment.
Rebase.
https://reviews.llvm.org/D42721
Files:
include/clang/Analysis/CFG.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -114,12 +114,14 @@
const LocationContext *LCtx = Pred->getLocationContext();
ProgramStateRef State = Pred->getState();
- // See if we're constructing an existing region by looking at the next
- // element in the CFG.
-
- if (auto Elem = findElementDirectlyInitializedByCurrentConstructor()) {
-if (Optional StmtElem = Elem->getAs()) {
- if (const CXXNewExpr *CNE = dyn_cast(StmtElem->getStmt())) {
+ // See if we're constructing an existing region by looking at the
+ // current construction context.
+ const NodeBuilderContext = getBuilderContext();
+ const CFGBlock *B = CurrBldrCtx.getBlock();
+ const CFGElement = (*B)[currStmtIdx];
+ if (auto CC = E.getAs()) {
+if (const Stmt *TriggerStmt = CC->getTriggerStmt()) {
+ if (const CXXNewExpr *CNE = dyn_cast(TriggerStmt)) {
if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
// TODO: Detect when the allocator returns a null pointer.
// Constructor shall not be called in this case.
@@ -135,7 +137,7 @@
return MR;
}
}
- } else if (auto *DS = dyn_cast(StmtElem->getStmt())) {
+ } else if (auto *DS = dyn_cast(TriggerStmt)) {
if (const auto *Var = dyn_cast(DS->getSingleDecl())) {
if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) {
SVal LValue = State->getLValue(Var, LCtx);
@@ -145,11 +147,9 @@
return LValue.getAsRegion();
}
}
- } else {
-llvm_unreachable("Unexpected directly initialized element!");
}
-} else if (Optional InitElem = Elem->getAs()) {
- const CXXCtorInitializer *Init = InitElem->getInitializer();
+ // TODO: Consider other directly initialized elements.
+} else if (const CXXCtorInitializer *Init = CC->getTriggerInit()) {
assert(Init->isAnyMemberInitializer());
const CXXMethodDecl *CurCtor = cast(LCtx->getDecl());
Loc ThisPtr =
@@ -183,53 +183,6 @@
return MRMgr.getCXXTempObjectRegion(CE, LCtx);
}
-/// Returns true if the initializer for \Elem can be a direct
-/// constructor.
-static bool canHaveDirectConstructor(CFGElement Elem){
- // DeclStmts and CXXCtorInitializers for fields can be directly constructed.
-
- if (Optional StmtElem = Elem.getAs()) {
-if (isa(StmtElem->getStmt())) {
- return true;
-}
-if (isa(StmtElem->getStmt())) {
- return true;
-}
- }
-
- if (Elem.getKind() == CFGElement::Initializer) {
-return true;
- }
-
- return false;
-}
-
-Optional
-ExprEngine::findElementDirectlyInitializedByCurrentConstructor() {
- const NodeBuilderContext = getBuilderContext();
- // See if we're constructing an existing region by looking at the next
- // element in the CFG.
- const CFGBlock *B = CurrBldrCtx.getBlock();
- assert(isa(((*B)[currStmtIdx]).castAs().getStmt()));
- unsigned int NextStmtIdx = currStmtIdx + 1;
- if (NextStmtIdx >= B->size())
-return None;
-
- CFGElement Next = (*B)[NextStmtIdx];
-
- // Is this a destructor? If so, we might be in the middle of an assignment
- // to a local or member: look ahead one more element to see what we find.
- while (Next.getAs() && NextStmtIdx + 1 < B->size()) {
-++NextStmtIdx;
-Next = (*B)[NextStmtIdx];
- }
-
- if (canHaveDirectConstructor(Next))
-return Next;
-
- return None;
-}
-
const CXXConstructExpr *
ExprEngine::findDirectConstructorForCurrentCFGElement() {
// Go backward in the CFG to see if the previous element (ignoring
@@ -241,7 +194,6 @@
return nullptr;
const CFGBlock *B = getBuilderContext().getBlock();
- assert(canHaveDirectConstructor((*B)[currStmtIdx]));
unsigned int PreviousStmtIdx = currStmtIdx - 1;
CFGElement Previous = (*B)[PreviousStmtIdx];
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -665,13 +665,6 @@
/// constructing into an existing region.
const CXXConstructExpr *findDirectConstructorForCurrentCFGElement();
- /// For a CXXConstructExpr, walk forward in the current CFG block to find the
- /// CFGElement for the DeclStmt or CXXInitCtorInitializer or CXXNewExpr which
- /// is directly constructed by this constructor. Returns None if the current
- /// constructor expression did not directly construct into an existing
- /// region.
- Optional
[PATCH] D42721: [analyzer] NFC: Use construction contexts for finding the target region for the construction.
NoQ updated this revision to Diff 132707.
NoQ added a comment.
Rebase.
https://reviews.llvm.org/D42721
Files:
include/clang/Analysis/CFG.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -114,12 +114,14 @@
const LocationContext *LCtx = Pred->getLocationContext();
ProgramStateRef State = Pred->getState();
- // See if we're constructing an existing region by looking at the next
- // element in the CFG.
-
- if (auto Elem = findElementDirectlyInitializedByCurrentConstructor()) {
-if (Optional StmtElem = Elem->getAs()) {
- if (const CXXNewExpr *CNE = dyn_cast(StmtElem->getStmt())) {
+ // See if we're constructing an existing region by looking at the
+ // current construction context.
+ const NodeBuilderContext = getBuilderContext();
+ const CFGBlock *B = CurrBldrCtx.getBlock();
+ const CFGElement = (*B)[currStmtIdx];
+ if (auto CC = E.getAs()) {
+if (const Stmt *TriggerStmt = CC->getTriggerStmt()) {
+ if (const CXXNewExpr *CNE = dyn_cast(TriggerStmt)) {
if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
// TODO: Detect when the allocator returns a null pointer.
// Constructor shall not be called in this case.
@@ -135,7 +137,7 @@
return MR;
}
}
- } else if (auto *DS = dyn_cast(StmtElem->getStmt())) {
+ } else if (auto *DS = dyn_cast(TriggerStmt)) {
if (const auto *Var = dyn_cast(DS->getSingleDecl())) {
if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) {
SVal LValue = State->getLValue(Var, LCtx);
@@ -145,11 +147,9 @@
return LValue.getAsRegion();
}
}
- } else {
-llvm_unreachable("Unexpected directly initialized element!");
}
-} else if (Optional InitElem = Elem->getAs()) {
- const CXXCtorInitializer *Init = InitElem->getInitializer();
+ // TODO: Consider other directly initialized elements.
+} else if (const CXXCtorInitializer *Init = CC->getTriggerInit()) {
assert(Init->isAnyMemberInitializer());
const CXXMethodDecl *CurCtor = cast(LCtx->getDecl());
Loc ThisPtr =
@@ -183,56 +183,6 @@
return MRMgr.getCXXTempObjectRegion(CE, LCtx);
}
-/// Returns true if the initializer for \Elem can be a direct
-/// constructor.
-static bool canHaveDirectConstructor(CFGElement Elem){
- // DeclStmts and CXXCtorInitializers for fields can be directly constructed.
-
- if (Optional StmtElem = Elem.getAs()) {
-if (isa(StmtElem->getStmt())) {
- return true;
-}
-if (isa(StmtElem->getStmt())) {
- return true;
-}
- }
-
- if (Elem.getKind() == CFGElement::Initializer) {
-return true;
- }
-
- return false;
-}
-
-Optional
-ExprEngine::findElementDirectlyInitializedByCurrentConstructor() {
- const NodeBuilderContext = getBuilderContext();
- // See if we're constructing an existing region by looking at the next
- // element in the CFG.
- const CFGBlock *B = CurrBldrCtx.getBlock();
- // TODO: Retrieve construction target from CFGConstructor directly.
- assert(
- (*B)[currStmtIdx].getAs() ||
- isa(((*B)[currStmtIdx]).castAs().getStmt()));
- unsigned int NextStmtIdx = currStmtIdx + 1;
- if (NextStmtIdx >= B->size())
-return None;
-
- CFGElement Next = (*B)[NextStmtIdx];
-
- // Is this a destructor? If so, we might be in the middle of an assignment
- // to a local or member: look ahead one more element to see what we find.
- while (Next.getAs() && NextStmtIdx + 1 < B->size()) {
-++NextStmtIdx;
-Next = (*B)[NextStmtIdx];
- }
-
- if (canHaveDirectConstructor(Next))
-return Next;
-
- return None;
-}
-
const CXXConstructExpr *
ExprEngine::findDirectConstructorForCurrentCFGElement() {
// Go backward in the CFG to see if the previous element (ignoring
@@ -244,7 +194,6 @@
return nullptr;
const CFGBlock *B = getBuilderContext().getBlock();
- assert(canHaveDirectConstructor((*B)[currStmtIdx]));
unsigned int PreviousStmtIdx = currStmtIdx - 1;
CFGElement Previous = (*B)[PreviousStmtIdx];
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -665,13 +665,6 @@
/// constructing into an existing region.
const CXXConstructExpr *findDirectConstructorForCurrentCFGElement();
- /// For a CXXConstructExpr, walk forward in the current CFG block to find the
- /// CFGElement for the DeclStmt or CXXInitCtorInitializer or CXXNewExpr which
- /// is directly constructed by this constructor. Returns None if the current
- ///
[PATCH] D42721: [analyzer] NFC: Use construction contexts for finding the target region for the construction.
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
NoQ added a dependency: D42719: [CFG] [analyzer] Add construction context when
constructor is wrapped into ExprWithCleanups..
Previously, we were scanning the CFG forward in order to find out which CFG
element contains the expression which is being initialized by the current
constructor. Now we have all the context that we needed directly in the CFG
(woohoo). As shown in the previous patches in the stack of patches, the new
context thing is pretty easy to extend to more cases that we previously didn't
support at all. More refactoring would follow soon.
Repository:
rC Clang
https://reviews.llvm.org/D42721
Files:
include/clang/Analysis/CFG.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -115,12 +115,14 @@
const LocationContext *LCtx = Pred->getLocationContext();
ProgramStateRef State = Pred->getState();
- // See if we're constructing an existing region by looking at the next
- // element in the CFG.
-
- if (auto Elem = findElementDirectlyInitializedByCurrentConstructor()) {
-if (Optional StmtElem = Elem->getAs()) {
- if (const CXXNewExpr *CNE = dyn_cast(StmtElem->getStmt())) {
+ // See if we're constructing an existing region by looking at the
+ // current construction context.
+ const NodeBuilderContext = getBuilderContext();
+ const CFGBlock *B = CurrBldrCtx.getBlock();
+ const CFGElement = (*B)[currStmtIdx];
+ if (auto CC = E.getAs()) {
+if (const Stmt *TriggerStmt = CC->getTriggerStmt()) {
+ if (const CXXNewExpr *CNE = dyn_cast(TriggerStmt)) {
if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
// TODO: Detect when the allocator returns a null pointer.
// Constructor shall not be called in this case.
@@ -136,20 +138,18 @@
return MR;
}
}
- } else if (auto *DS = dyn_cast(StmtElem->getStmt())) {
+ } else if (auto *DS = dyn_cast(TriggerStmt)) {
if (const auto *Var = dyn_cast(DS->getSingleDecl())) {
if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) {
SVal LValue = State->getLValue(Var, LCtx);
QualType Ty = Var->getType();
LValue = makeZeroElementRegion(State, LValue, Ty, CallOpts);
return LValue.getAsRegion();
}
}
- } else {
-llvm_unreachable("Unexpected directly initialized element!");
}
-} else if (Optional InitElem = Elem->getAs()) {
- const CXXCtorInitializer *Init = InitElem->getInitializer();
+ // TODO: Consider other directly initialized elements.
+} else if (const CXXCtorInitializer *Init = CC->getTriggerInit()) {
assert(Init->isAnyMemberInitializer());
const CXXMethodDecl *CurCtor = cast(LCtx->getDecl());
Loc ThisPtr =
@@ -182,56 +182,6 @@
return MRMgr.getCXXTempObjectRegion(CE, LCtx);
}
-/// Returns true if the initializer for \Elem can be a direct
-/// constructor.
-static bool canHaveDirectConstructor(CFGElement Elem){
- // DeclStmts and CXXCtorInitializers for fields can be directly constructed.
-
- if (Optional StmtElem = Elem.getAs()) {
-if (isa(StmtElem->getStmt())) {
- return true;
-}
-if (isa(StmtElem->getStmt())) {
- return true;
-}
- }
-
- if (Elem.getKind() == CFGElement::Initializer) {
-return true;
- }
-
- return false;
-}
-
-Optional
-ExprEngine::findElementDirectlyInitializedByCurrentConstructor() {
- const NodeBuilderContext = getBuilderContext();
- // See if we're constructing an existing region by looking at the next
- // element in the CFG.
- const CFGBlock *B = CurrBldrCtx.getBlock();
- // TODO: Retrieve construction target from CFGConstructor directly.
- assert(
- (*B)[currStmtIdx].getAs() ||
- isa(((*B)[currStmtIdx]).castAs().getStmt()));
- unsigned int NextStmtIdx = currStmtIdx + 1;
- if (NextStmtIdx >= B->size())
-return None;
-
- CFGElement Next = (*B)[NextStmtIdx];
-
- // Is this a destructor? If so, we might be in the middle of an assignment
- // to a local or member: look ahead one more element to see what we find.
- while (Next.getAs() && NextStmtIdx + 1 < B->size()) {
-++NextStmtIdx;
-Next = (*B)[NextStmtIdx];
- }
-
- if (canHaveDirectConstructor(Next))
-return Next;
-
- return None;
-}
-
const CXXConstructExpr *
ExprEngine::findDirectConstructorForCurrentCFGElement() {
// Go backward in the CFG to see if the previous element (ignoring
@@ -243,7 +193,6 @@
return nullptr;
const CFGBlock *B = getBuilderContext().getBlock();
-
