r205809 - Thread Safety Analysis. Misc fixes to SExpr code, responding to code review
DeLesley Hutchins
delesley at google.com
Tue Apr 8 15:21:23 PDT 2014
Author: delesley
Date: Tue Apr 8 17:21:22 2014
New Revision: 205809
URL: http://llvm.org/viewvc/llvm-project?rev=205809&view=rev
Log:
Thread Safety Analysis. Misc fixes to SExpr code, responding to code review
by Aaron Ballman.
Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h?rev=205809&r1=205808&r2=205809&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h Tue Apr 8 17:21:22 2014
@@ -26,20 +26,14 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/StmtCXX.h"
-#include "clang/AST/StmtVisitor.h"
#include "clang/Analysis/Analyses/PostOrderCFGView.h"
#include "clang/Analysis/Analyses/ThreadSafetyTIL.h"
#include "clang/Analysis/AnalysisContext.h"
#include "clang/Analysis/CFG.h"
-#include "clang/Analysis/CFGStmtMap.h"
#include "clang/Basic/OperatorKinds.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
-#include "llvm/ADT/BitVector.h"
#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/FoldingSet.h"
-#include "llvm/ADT/ImmutableMap.h"
-#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include <vector>
@@ -81,9 +75,8 @@ public:
// Walks the clang CFG, and invokes methods on a given CFGVisitor.
class CFGWalker {
public:
- CFGWalker() : CFGraph(0), FDecl(0), ACtx(0), SortedGraph(0) {}
-
- ~CFGWalker() { }
+ CFGWalker() :
+ CFGraph(nullptr), FDecl(nullptr), ACtx(nullptr), SortedGraph(nullptr) {}
// Initialize the CFGWalker. This setup only needs to be done once, even
// if there are multiple passes over the CFG.
@@ -117,16 +110,14 @@ public:
V.enterCFGBlock(CurrBlock);
// Process statements
- for (CFGBlock::const_iterator BI = CurrBlock->begin(),
- BE = CurrBlock->end();
- BI != BE; ++BI) {
- switch (BI->getKind()) {
+ for (auto BI : *CurrBlock) {
+ switch (BI.getKind()) {
case CFGElement::Statement: {
- V.handleStatement(BI->castAs<CFGStmt>().getStmt());
+ V.handleStatement(BI.castAs<CFGStmt>().getStmt());
break;
}
case CFGElement::AutomaticObjectDtor: {
- CFGAutomaticObjDtor AD = BI->castAs<CFGAutomaticObjDtor>();
+ CFGAutomaticObjDtor AD = BI.castAs<CFGAutomaticObjDtor>();
CXXDestructorDecl *DD = const_cast<CXXDestructorDecl*>(
AD.getDestructorDecl(ACtx->getASTContext()));
VarDecl *VD = const_cast<VarDecl*>(AD.getVarDecl());
@@ -142,7 +133,7 @@ public:
for (CFGBlock::const_succ_iterator SI = CurrBlock->succ_begin(),
SE = CurrBlock->succ_end();
SI != SE; ++SI) {
- if (*SI == 0)
+ if (*SI == nullptr)
continue;
if (VisitedBlocks.alreadySet(*SI)) {
@@ -157,7 +148,7 @@ public:
V.exitCFG(&CFGraph->getExit());
}
-public:
+public: // TODO: make these private.
CFG *CFGraph;
const NamedDecl *FDecl;
AnalysisDeclContext *ACtx;
@@ -187,8 +178,9 @@ public:
CallingContext *Prev; // The previous context; or 0 if none.
bool SelfArrow; // is Self referred to with -> or .?
- CallingContext(const NamedDecl *D = 0, const Expr *S = 0, unsigned N = 0,
- const Expr *const *A = 0, CallingContext *P = 0)
+ CallingContext(const NamedDecl *D = nullptr, const Expr *S = nullptr,
+ unsigned N = 0, const Expr *const *A = nullptr,
+ CallingContext *P = nullptr)
: AttrDecl(D), SelfArg(S), NumArgs(N), FunArgs(A), Prev(P),
SelfArrow(false)
{}
@@ -202,7 +194,7 @@ public:
// Dispatches on the type of S.
til::SExpr *translate(const Stmt *S, CallingContext *Ctx);
-
+protected:
til::SExpr *translateDeclRefExpr(const DeclRefExpr *DRE,
CallingContext *Ctx) ;
til::SExpr *translateCXXThisExpr(const CXXThisExpr *TE, CallingContext *Ctx);
@@ -225,8 +217,9 @@ public:
const BinaryConditionalOperator *C, CallingContext *Ctx);
- SExprBuilder(til::MemRegionRef A, StatementMap *SM = 0)
- : Arena(A), SMap(SM), SelfVar(0) {
+public:
+ SExprBuilder(til::MemRegionRef A, StatementMap *SM = nullptr)
+ : Arena(A), SMap(SM), SelfVar(nullptr) {
// FIXME: we don't always have a self-variable.
SelfVar = new (Arena) til::Variable(til::Variable::VK_SFun);
}
Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h?rev=205809&r1=205808&r2=205809&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h Tue Apr 8 17:21:22 2014
@@ -53,6 +53,7 @@
#include <cassert>
#include <cstddef>
+#include <utility>
namespace clang {
namespace threadSafety {
@@ -115,7 +116,7 @@ public:
: Data(Dat), Size(0), Capacity(Cp) {}
SimpleArray(MemRegionRef A, size_t Cp)
: Data(A.allocateT<T>(Cp)), Size(0), Capacity(Cp) {}
- SimpleArray(SimpleArray<T> &A, bool Steal)
+ SimpleArray(SimpleArray<T> &&A)
: Data(A.Data), Size(A.Size), Capacity(A.Capacity) {
A.Data = nullptr;
A.Size = 0;
@@ -159,6 +160,8 @@ public:
}
private:
+ SimpleArray(const SimpleArray<T> &A) { }
+
T *Data;
size_t Size;
size_t Capacity;
@@ -178,7 +181,7 @@ typedef clang::UnaryOperatorKind TIL_Una
typedef clang::CastKind TIL_CastOpcode;
-enum TIL_TraversalKind {
+enum TraversalKind {
TRV_Normal,
TRV_Lazy, // subexpression may need to be traversed lazily
TRV_Tail // subexpression occurs in a tail position
@@ -315,7 +318,7 @@ public:
SExpr *definition() { return Definition.get(); }
void attachVar() const { ++NumUses; }
- void detachVar() const { --NumUses; }
+ void detachVar() const { assert(NumUses > 0); --NumUses; }
unsigned getID() { return Id; }
unsigned getBlockID() { return BlockID; }
@@ -347,7 +350,7 @@ private:
unsigned short BlockID;
unsigned short Id;
- mutable int NumUses;
+ mutable unsigned NumUses;
};
@@ -1075,9 +1078,9 @@ public:
SCFG(MemRegionRef A, unsigned Nblocks)
: SExpr(COP_SCFG), Blocks(A, Nblocks), Entry(nullptr), Exit(nullptr) {}
- SCFG(const SCFG &Cfg, BlockArray &Ba) // steals memory from ba
+ SCFG(const SCFG &Cfg, BlockArray &&Ba) // steals memory from Ba
: SExpr(COP_SCFG),
- Blocks(Ba, true),
+ Blocks(std::move(Ba)),
Entry(nullptr),
Exit(nullptr) {
// TODO: set entry and exit!
@@ -1126,9 +1129,9 @@ public:
SExpr *Term = nullptr)
: BlockID(0), Parent(nullptr), Args(A, Nargs), Instrs(A, Nins),
Terminator(Term) {}
- BasicBlock(const BasicBlock &B, VarArray &As, VarArray &Is, SExpr *T)
- : BlockID(0), Parent(nullptr), Args(As, true), Instrs(Is, true),
- Terminator(T)
+ BasicBlock(const BasicBlock &B, VarArray &&As, VarArray &&Is, SExpr *T)
+ : BlockID(0), Parent(nullptr),
+ Args(std::move(As)), Instrs(std::move(Is)), Terminator(T)
{}
unsigned blockID() const { return BlockID; }
@@ -1153,18 +1156,19 @@ public:
typename V::template Container<Variable*> Nas(Visitor, Args.size());
typename V::template Container<Variable*> Nis(Visitor, Instrs.size());
- for (unsigned I = 0; I < Args.size(); ++I) {
- typename V::R_SExpr Ne = Visitor.traverse(Args[I]->Definition);
- Variable *Nvd = Visitor.enterScope(*Args[I], Ne);
+ for (auto A : Args) {
+ typename V::R_SExpr Ne = Visitor.traverse(A->Definition);
+ Variable *Nvd = Visitor.enterScope(*A, Ne);
Nas.push_back(Nvd);
}
- for (unsigned J = 0; J < Instrs.size(); ++J) {
- typename V::R_SExpr Ne = Visitor.traverse(Instrs[J]->Definition);
- Variable *Nvd = Visitor.enterScope(*Instrs[J], Ne);
+ for (auto I : Instrs) {
+ typename V::R_SExpr Ne = Visitor.traverse(I->Definition);
+ Variable *Nvd = Visitor.enterScope(*I, Ne);
Nis.push_back(Nvd);
}
typename V::R_SExpr Nt = Visitor.traverse(Terminator);
+ // TODO: use reverse iterator
for (unsigned J = 0, JN = Instrs.size(); J < JN; ++J)
Visitor.exitScope(*Instrs[JN-J]);
for (unsigned I = 0, IN = Instrs.size(); I < IN; ++I)
@@ -1174,7 +1178,7 @@ public:
}
template <class C> typename C::CType compare(BasicBlock* E, C& Cmp) {
- // TODO -- implement CFG comparisons
+ // TODO: implement CFG comparisons
return Cmp.comparePointers(this, E);
}
@@ -1194,8 +1198,8 @@ template <class V>
typename V::R_SExpr SCFG::traverse(V &Visitor) {
Visitor.enterCFG(*this);
typename V::template Container<BasicBlock *> Bbs(Visitor, Blocks.size());
- for (unsigned I = 0; I < Blocks.size(); ++I) {
- BasicBlock *Nbb = Blocks[I]->traverse(Visitor);
+ for (auto B : Blocks) {
+ BasicBlock *Nbb = B->traverse(Visitor);
Bbs.push_back(Nbb);
}
Visitor.exitCFG(*this);
@@ -1211,9 +1215,12 @@ public:
static bool classof(const SExpr *E) { return E->opcode() == COP_Phi; }
- Phi(MemRegionRef A, unsigned Nvals) : SExpr(COP_Phi), Values(A, Nvals) {}
- Phi(const Phi &P, ValArray &Vs) // steals memory of vs
- : SExpr(COP_Phi), Values(Vs, true) {}
+ Phi(MemRegionRef A, unsigned Nvals)
+ : SExpr(COP_Phi), Values(A, Nvals)
+ {}
+ Phi(const Phi &P, ValArray &&Vs) // steals memory of Vs
+ : SExpr(COP_Phi), Values(std::move(Vs))
+ {}
const ValArray &values() const { return Values; }
ValArray &values() { return Values; }
@@ -1221,9 +1228,8 @@ public:
template <class V> typename V::R_SExpr traverse(V &Visitor) {
typename V::template Container<typename V::R_SExpr> Nvs(Visitor,
Values.size());
- for (ValArray::iterator I = Values.begin(), E = Values.end();
- I != E; ++I) {
- typename V::R_SExpr Nv = Visitor.traverse(*I);
+ for (auto Val : Values) {
+ typename V::R_SExpr Nv = Visitor.traverse(Val);
Nvs.push_back(Nv);
}
return Visitor.reducePhi(*this, Nvs);
@@ -1244,9 +1250,9 @@ public:
static bool classof(const SExpr *E) { return E->opcode() == COP_Goto; }
Goto(BasicBlock *B, unsigned Index)
- : SExpr(COP_Goto), TargetBlock(B) {}
- Goto(const Goto &G, BasicBlock *B, unsigned Index)
- : SExpr(COP_Goto), TargetBlock(B) {}
+ : SExpr(COP_Goto), TargetBlock(B), Index(0) {}
+ Goto(const Goto &G, BasicBlock *B, unsigned I)
+ : SExpr(COP_Goto), TargetBlock(B), Index(I) {}
BasicBlock *targetBlock() const { return TargetBlock; }
unsigned index() const { return Index; }
@@ -1325,22 +1331,22 @@ private:
// The reduceX methods control the kind of traversal (visitor, copy, etc.).
// These are separated into a separate class R for the purpose of code reuse.
// The full reducer interface also has methods to handle scopes
-template <class Self, class R> class TILTraversal : public R {
+template <class Self, class R> class Traversal : public R {
public:
Self *self() { return reinterpret_cast<Self *>(this); }
// Traverse an expression -- returning a result of type R_SExpr.
// Override this method to do something for every expression, regardless
- // of which kind it is. TIL_TraversalKind indicates the context in which
+ // of which kind it is. TraversalKind indicates the context in which
// the expression occurs, and can be:
// TRV_Normal
// TRV_Lazy -- e may need to be traversed lazily, using a Future.
// TRV_Tail -- e occurs in a tail position
- typename R::R_SExpr traverse(SExprRef &E, TIL_TraversalKind K = TRV_Normal) {
+ typename R::R_SExpr traverse(SExprRef &E, TraversalKind K = TRV_Normal) {
return traverse(E.get(), K);
}
- typename R::R_SExpr traverse(SExpr *E, TIL_TraversalKind K = TRV_Normal) {
+ typename R::R_SExpr traverse(SExpr *E, TraversalKind K = TRV_Normal) {
return traverseByCase(E);
}
@@ -1370,9 +1376,9 @@ public:
// The default behavior of reduce##X(...) is to create a copy of the original.
// Subclasses can override reduce##X to implement non-destructive rewriting
// passes.
-class TILCopyReducer {
+class CopyReducer {
public:
- TILCopyReducer() {}
+ CopyReducer() {}
void setArena(MemRegionRef A) { Arena = A; }
@@ -1385,13 +1391,13 @@ public:
template <class T> class Container {
public:
// Allocate a new container with a capacity for n elements.
- Container(TILCopyReducer &R, unsigned N) : Elems(R.Arena, N) {}
+ Container(CopyReducer &R, unsigned N) : Elems(R.Arena, N) {}
// Push a new element onto the container.
void push_back(T E) { Elems.push_back(E); }
private:
- friend class TILCopyReducer;
+ friend class CopyReducer;
SimpleArray<T> Elems;
};
@@ -1457,11 +1463,11 @@ public:
return new (Arena) Cast(Orig, E0);
}
- R_SExpr reduceSCFG(SCFG &Orig, Container<BasicBlock *> Bbs) {
- return new (Arena) SCFG(Orig, Bbs.Elems);
+ R_SExpr reduceSCFG(SCFG &Orig, Container<BasicBlock *> &Bbs) {
+ return new (Arena) SCFG(Orig, std::move(Bbs.Elems));
}
- R_SExpr reducePhi(Phi &Orig, Container<R_SExpr> As) {
- return new (Arena) Phi(Orig, As.Elems);
+ R_SExpr reducePhi(Phi &Orig, Container<R_SExpr> &As) {
+ return new (Arena) Phi(Orig, std::move(As.Elems));
}
R_SExpr reduceGoto(Goto &Orig, BasicBlock *B, unsigned Index) {
return new (Arena) Goto(Orig, B, Index);
@@ -1472,7 +1478,8 @@ public:
BasicBlock *reduceBasicBlock(BasicBlock &Orig, Container<Variable *> &As,
Container<Variable *> &Is, R_SExpr T) {
- return new (Arena) BasicBlock(Orig, As.Elems, Is.Elems, T);
+ return new (Arena) BasicBlock(Orig, std::move(As.Elems),
+ std::move(Is.Elems), T);
}
// Create a new variable from orig, and push it onto the lexical scope.
@@ -1496,7 +1503,7 @@ private:
};
-class SExprCopier : public TILTraversal<SExprCopier, TILCopyReducer> {
+class SExprCopier : public Traversal<SExprCopier, CopyReducer> {
public:
SExprCopier(MemRegionRef A) { setArena(A); }
@@ -1510,9 +1517,9 @@ public:
// Implements a Reducer that visits each subexpression, and returns either
// true or false.
-class TILVisitReducer {
+class VisitReducer {
public:
- TILVisitReducer() {}
+ VisitReducer() {}
// A visitor returns a bool, representing success or failure.
typedef bool R_SExpr;
@@ -1520,11 +1527,11 @@ public:
// A visitor "container" is a single bool, which accumulates success.
template <class T> class Container {
public:
- Container(TILVisitReducer &R, unsigned N) : Success(true) {}
+ Container(VisitReducer &R, unsigned N) : Success(true) {}
void push_back(bool E) { Success = Success && E; }
private:
- friend class TILVisitReducer;
+ friend class VisitReducer;
bool Success;
};
@@ -1565,11 +1572,11 @@ public:
R_SExpr reduceSCFG(SCFG &Orig, Container<BasicBlock *> Bbs) {
return Bbs.Success;
}
- R_SExpr reducePhi(Phi &Orig, Container<R_SExpr> As) {
+ R_SExpr reducePhi(Phi &Orig, Container<R_SExpr> &As) {
return As.Success;
}
- R_SExpr reduceGoto(Goto &Orig, BasicBlock *B, Container<R_SExpr> As) {
- return As.Success;
+ R_SExpr reduceGoto(Goto &Orig, BasicBlock *B, unsigned Index) {
+ return true;
}
R_SExpr reduceBranch(Branch &O, R_SExpr C, BasicBlock *B0, BasicBlock *B1) {
return C;
@@ -1596,11 +1603,11 @@ public:
// A visitor will visit each node, and halt if any reducer returns false.
template <class Self>
-class SExprVisitor : public TILTraversal<Self, TILVisitReducer> {
+class SExprVisitor : public Traversal<Self, VisitReducer> {
public:
SExprVisitor() : Success(true) {}
- bool traverse(SExpr *E, TIL_TraversalKind K = TRV_Normal) {
+ bool traverse(SExpr *E, TraversalKind K = TRV_Normal) {
Success = Success && this->traverseByCase(E);
return Success;
}
@@ -1617,10 +1624,11 @@ private:
// Basic class for comparison operations over expressions.
template <typename Self>
-class TILComparator {
-public:
+class Comparator {
+protected:
Self *self() { return reinterpret_cast<Self *>(this); }
+public:
bool compareByCase(SExpr *E1, SExpr* E2) {
switch (E1->opcode()) {
#define TIL_OPCODE_DEF(X) \
@@ -1635,7 +1643,7 @@ public:
};
-class TILEqualsComparator : public TILComparator<TILEqualsComparator> {
+class EqualsComparator : public Comparator<EqualsComparator> {
public:
// Result type for the comparison, e.g. bool for simple equality,
// or int for lexigraphic comparison (-1, 0, 1). Must have one value which
@@ -1663,7 +1671,7 @@ public:
}
static bool compareExprs(SExpr *E1, SExpr* E2) {
- TILEqualsComparator Eq;
+ EqualsComparator Eq;
return Eq.compare(E1, E2);
}
};
@@ -1671,7 +1679,7 @@ public:
// Pretty printer for TIL expressions
template <typename Self, typename StreamType>
-class TILPrettyPrinter {
+class PrettyPrinter {
public:
static void print(SExpr *E, StreamType &SS) {
Self printer;
@@ -1895,11 +1903,11 @@ protected:
for (auto BBI : *E) {
SS << "BB_" << BBI->blockID() << ":";
newline(SS);
- for (auto I : BBI->arguments()) {
+ for (auto A : BBI->arguments()) {
SS << "let ";
- self()->printVariable(I, SS);
+ self()->printVariable(A, SS);
SS << " = ";
- self()->printSExpr(I->definition(), SS, Prec_MAX);
+ self()->printSExpr(A->definition(), SS, Prec_MAX);
SS << ";";
newline(SS);
}
Modified: cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp?rev=205809&r1=205808&r2=205809&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp Tue Apr 8 17:21:22 2014
@@ -15,21 +15,15 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/StmtCXX.h"
-#include "clang/AST/StmtVisitor.h"
#include "clang/Analysis/Analyses/PostOrderCFGView.h"
-#include "clang/Analysis/Analyses/ThreadSafetyTIL.h"
#include "clang/Analysis/Analyses/ThreadSafetyCommon.h"
+#include "clang/Analysis/Analyses/ThreadSafetyTIL.h"
#include "clang/Analysis/AnalysisContext.h"
#include "clang/Analysis/CFG.h"
-#include "clang/Analysis/CFGStmtMap.h"
#include "clang/Basic/OperatorKinds.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
-#include "llvm/ADT/BitVector.h"
#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/FoldingSet.h"
-#include "llvm/ADT/ImmutableMap.h"
-#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include <vector>
@@ -401,7 +395,7 @@ private:
class LLVMPrinter :
- public til::TILPrettyPrinter<LLVMPrinter, llvm::raw_ostream> {
+ public til::PrettyPrinter<LLVMPrinter, llvm::raw_ostream> {
};
More information about the cfe-commits
mailing list