r244998 - unique_ptrify ConsumedBlockInfo analysis to make it move assignable
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 13 18:26:20 PDT 2015
Author: dblaikie
Date: Thu Aug 13 20:26:19 2015
New Revision: 244998
URL: http://llvm.org/viewvc/llvm-project?rev=244998&view=rev
Log:
unique_ptrify ConsumedBlockInfo analysis to make it move assignable
ConsumedBlockInfo objects were move assigned, but only in a state where
the dtor was a no-op anyway. Subtle and easily could've happened in ways
that wouldn't've been safe - so this change makes it safe no matter what
state the ConsumedBlockInfo object is in.
Modified:
cfe/trunk/include/clang/Analysis/Analyses/Consumed.h
cfe/trunk/lib/Analysis/Consumed.cpp
Modified: cfe/trunk/include/clang/Analysis/Analyses/Consumed.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/Consumed.h?rev=244998&r1=244997&r2=244998&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/Consumed.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/Consumed.h Thu Aug 13 20:26:19 2015
@@ -162,8 +162,8 @@ namespace consumed {
ConsumedState getState(const CXXBindTemporaryExpr *Tmp) const;
/// \brief Merge this state map with another map.
- void intersect(const ConsumedStateMap *Other);
-
+ void intersect(const ConsumedStateMap &Other);
+
void intersectAtLoopHead(const CFGBlock *LoopHead, const CFGBlock *LoopBack,
const ConsumedStateMap *LoopBackStates,
ConsumedWarningsHandlerBase &WarningsHandler);
@@ -196,15 +196,19 @@ namespace consumed {
};
class ConsumedBlockInfo {
- std::vector<ConsumedStateMap*> StateMapsArray;
+ std::vector<std::unique_ptr<ConsumedStateMap>> StateMapsArray;
std::vector<unsigned int> VisitOrder;
public:
- ConsumedBlockInfo() { }
- ~ConsumedBlockInfo() { llvm::DeleteContainerPointers(StateMapsArray); }
+ ConsumedBlockInfo() = default;
+ ConsumedBlockInfo &operator=(ConsumedBlockInfo &&Other) {
+ StateMapsArray = std::move(Other.StateMapsArray);
+ VisitOrder = std::move(Other.VisitOrder);
+ return *this;
+ }
ConsumedBlockInfo(unsigned int NumBlocks, PostOrderCFGView *SortedGraph)
- : StateMapsArray(NumBlocks, nullptr), VisitOrder(NumBlocks, 0) {
+ : StateMapsArray(NumBlocks), VisitOrder(NumBlocks, 0) {
unsigned int VisitOrderCounter = 0;
for (PostOrderCFGView::iterator BI = SortedGraph->begin(),
BE = SortedGraph->end(); BI != BE; ++BI) {
@@ -214,17 +218,18 @@ namespace consumed {
bool allBackEdgesVisited(const CFGBlock *CurrBlock,
const CFGBlock *TargetBlock);
-
+
void addInfo(const CFGBlock *Block, ConsumedStateMap *StateMap,
- bool &AlreadyOwned);
- void addInfo(const CFGBlock *Block, ConsumedStateMap *StateMap);
-
+ std::unique_ptr<ConsumedStateMap> &OwnedStateMap);
+ void addInfo(const CFGBlock *Block,
+ std::unique_ptr<ConsumedStateMap> StateMap);
+
ConsumedStateMap* borrowInfo(const CFGBlock *Block);
void discardInfo(const CFGBlock *Block);
-
- ConsumedStateMap* getInfo(const CFGBlock *Block);
-
+
+ std::unique_ptr<ConsumedStateMap> getInfo(const CFGBlock *Block);
+
bool isBackEdge(const CFGBlock *From, const CFGBlock *To);
bool isBackEdgeTarget(const CFGBlock *Block);
};
@@ -233,8 +238,8 @@ namespace consumed {
class ConsumedAnalyzer {
ConsumedBlockInfo BlockInfo;
- ConsumedStateMap *CurrStates;
-
+ std::unique_ptr<ConsumedStateMap> CurrStates;
+
ConsumedState ExpectedReturnState;
void determineExpectedReturnState(AnalysisDeclContext &AC,
Modified: cfe/trunk/lib/Analysis/Consumed.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/Consumed.cpp?rev=244998&r1=244997&r2=244998&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/Consumed.cpp (original)
+++ cfe/trunk/lib/Analysis/Consumed.cpp Thu Aug 13 20:26:19 2015
@@ -1038,65 +1038,54 @@ bool ConsumedBlockInfo::allBackEdgesVisi
return true;
}
-void ConsumedBlockInfo::addInfo(const CFGBlock *Block,
- ConsumedStateMap *StateMap,
- bool &AlreadyOwned) {
-
+void ConsumedBlockInfo::addInfo(
+ const CFGBlock *Block, ConsumedStateMap *StateMap,
+ std::unique_ptr<ConsumedStateMap> &OwnedStateMap) {
+
assert(Block && "Block pointer must not be NULL");
-
- ConsumedStateMap *Entry = StateMapsArray[Block->getBlockID()];
-
+
+ auto &Entry = StateMapsArray[Block->getBlockID()];
+
if (Entry) {
- Entry->intersect(StateMap);
-
- } else if (AlreadyOwned) {
- StateMapsArray[Block->getBlockID()] = new ConsumedStateMap(*StateMap);
-
- } else {
- StateMapsArray[Block->getBlockID()] = StateMap;
- AlreadyOwned = true;
- }
+ Entry->intersect(*StateMap);
+ } else if (OwnedStateMap)
+ Entry = std::move(OwnedStateMap);
+ else
+ Entry = llvm::make_unique<ConsumedStateMap>(*StateMap);
}
void ConsumedBlockInfo::addInfo(const CFGBlock *Block,
- ConsumedStateMap *StateMap) {
+ std::unique_ptr<ConsumedStateMap> StateMap) {
assert(Block && "Block pointer must not be NULL");
- ConsumedStateMap *Entry = StateMapsArray[Block->getBlockID()];
-
+ auto &Entry = StateMapsArray[Block->getBlockID()];
+
if (Entry) {
- Entry->intersect(StateMap);
- delete StateMap;
-
+ Entry->intersect(*StateMap);
} else {
- StateMapsArray[Block->getBlockID()] = StateMap;
+ Entry = std::move(StateMap);
}
}
ConsumedStateMap* ConsumedBlockInfo::borrowInfo(const CFGBlock *Block) {
assert(Block && "Block pointer must not be NULL");
assert(StateMapsArray[Block->getBlockID()] && "Block has no block info");
-
- return StateMapsArray[Block->getBlockID()];
+
+ return StateMapsArray[Block->getBlockID()].get();
}
void ConsumedBlockInfo::discardInfo(const CFGBlock *Block) {
- unsigned int BlockID = Block->getBlockID();
- delete StateMapsArray[BlockID];
- StateMapsArray[BlockID] = nullptr;
+ StateMapsArray[Block->getBlockID()] = nullptr;
}
-ConsumedStateMap* ConsumedBlockInfo::getInfo(const CFGBlock *Block) {
+std::unique_ptr<ConsumedStateMap>
+ConsumedBlockInfo::getInfo(const CFGBlock *Block) {
assert(Block && "Block pointer must not be NULL");
-
- ConsumedStateMap *StateMap = StateMapsArray[Block->getBlockID()];
- if (isBackEdgeTarget(Block)) {
- return new ConsumedStateMap(*StateMap);
- } else {
- StateMapsArray[Block->getBlockID()] = nullptr;
- return StateMap;
- }
+
+ auto &Entry = StateMapsArray[Block->getBlockID()];
+ return isBackEdgeTarget(Block) ? llvm::make_unique<ConsumedStateMap>(*Entry)
+ : std::move(Entry);
}
bool ConsumedBlockInfo::isBackEdge(const CFGBlock *From, const CFGBlock *To) {
@@ -1166,15 +1155,15 @@ ConsumedStateMap::getState(const CXXBind
return CS_None;
}
-void ConsumedStateMap::intersect(const ConsumedStateMap *Other) {
+void ConsumedStateMap::intersect(const ConsumedStateMap &Other) {
ConsumedState LocalState;
-
- if (this->From && this->From == Other->From && !Other->Reachable) {
+
+ if (this->From && this->From == Other.From && !Other.Reachable) {
this->markUnreachable();
return;
}
-
- for (const auto &DM : Other->VarMap) {
+
+ for (const auto &DM : Other.VarMap) {
LocalState = this->getState(DM.first);
if (LocalState == CS_None)
@@ -1282,14 +1271,14 @@ bool ConsumedAnalyzer::splitState(const
if (PInfo.isVarTest()) {
CurrStates->setSource(Cond);
FalseStates->setSource(Cond);
- splitVarStateForIf(IfNode, PInfo.getVarTest(), CurrStates,
+ splitVarStateForIf(IfNode, PInfo.getVarTest(), CurrStates.get(),
FalseStates.get());
-
+
} else if (PInfo.isBinTest()) {
CurrStates->setSource(PInfo.testSourceNode());
FalseStates->setSource(PInfo.testSourceNode());
- splitVarStateForIfBinOp(PInfo, CurrStates, FalseStates.get());
-
+ splitVarStateForIfBinOp(PInfo, CurrStates.get(), FalseStates.get());
+
} else {
return false;
}
@@ -1337,14 +1326,13 @@ bool ConsumedAnalyzer::splitState(const
CFGBlock::const_succ_iterator SI = CurrBlock->succ_begin();
if (*SI)
- BlockInfo.addInfo(*SI, CurrStates);
+ BlockInfo.addInfo(*SI, std::move(CurrStates));
else
- delete CurrStates;
-
+ CurrStates = nullptr;
+
if (*++SI)
- BlockInfo.addInfo(*SI, FalseStates.release());
+ BlockInfo.addInfo(*SI, std::move(FalseStates));
- CurrStates = nullptr;
return true;
}
@@ -1363,10 +1351,10 @@ void ConsumedAnalyzer::run(AnalysisDeclC
// AC.getCFG()->viewCFG(LangOptions());
BlockInfo = ConsumedBlockInfo(CFGraph->getNumBlockIDs(), SortedGraph);
-
- CurrStates = new ConsumedStateMap();
- ConsumedStmtVisitor Visitor(AC, *this, CurrStates);
-
+
+ CurrStates = llvm::make_unique<ConsumedStateMap>();
+ ConsumedStmtVisitor Visitor(AC, *this, CurrStates.get());
+
// Add all trackable parameters to the state map.
for (const auto *PI : D->params())
Visitor.VisitParmVarDecl(PI);
@@ -1380,13 +1368,12 @@ void ConsumedAnalyzer::run(AnalysisDeclC
continue;
} else if (!CurrStates->isReachable()) {
- delete CurrStates;
CurrStates = nullptr;
continue;
}
-
- Visitor.reset(CurrStates);
-
+
+ Visitor.reset(CurrStates.get());
+
// Visit all of the basic block's statements.
for (const auto &B : *CurrBlock) {
switch (B.getKind()) {
@@ -1429,28 +1416,24 @@ void ConsumedAnalyzer::run(AnalysisDeclC
if (CurrBlock->succ_size() > 1 ||
(CurrBlock->succ_size() == 1 &&
(*CurrBlock->succ_begin())->pred_size() > 1)) {
-
- bool OwnershipTaken = false;
-
+
+ auto *RawState = CurrStates.get();
+
for (CFGBlock::const_succ_iterator SI = CurrBlock->succ_begin(),
SE = CurrBlock->succ_end(); SI != SE; ++SI) {
if (*SI == nullptr) continue;
if (BlockInfo.isBackEdge(CurrBlock, *SI)) {
- BlockInfo.borrowInfo(*SI)->intersectAtLoopHead(*SI, CurrBlock,
- CurrStates,
- WarningsHandler);
-
+ BlockInfo.borrowInfo(*SI)->intersectAtLoopHead(
+ *SI, CurrBlock, RawState, WarningsHandler);
+
if (BlockInfo.allBackEdgesVisited(CurrBlock, *SI))
BlockInfo.discardInfo(*SI);
} else {
- BlockInfo.addInfo(*SI, CurrStates, OwnershipTaken);
+ BlockInfo.addInfo(*SI, RawState, CurrStates);
}
}
-
- if (!OwnershipTaken)
- delete CurrStates;
CurrStates = nullptr;
}
@@ -1463,8 +1446,8 @@ void ConsumedAnalyzer::run(AnalysisDeclC
} // End of block iterator.
// Delete the last existing state map.
- delete CurrStates;
-
+ CurrStates = nullptr;
+
WarningsHandler.emitDiagnostics();
}
}} // end namespace clang::consumed
More information about the cfe-commits
mailing list