Hi dblaikie, delesley, aaron.ballman,
The TestedVarsVisitor was folded into the ConsumedStmtVisitor. The
VarTestResult class was updated to allow these changes. As well, the
PropagationInfo class was updated for the same reasons.
http://llvm-reviews.chandlerc.com/D1480
Files:
include/clang/Analysis/Analyses/Consumed.h
lib/Analysis/Consumed.cpp
Index: include/clang/Analysis/Analyses/Consumed.h
===================================================================
--- include/clang/Analysis/Analyses/Consumed.h
+++ include/clang/Analysis/Analyses/Consumed.h
@@ -25,6 +25,8 @@
namespace clang {
namespace consumed {
+ class ConsumedStmtVisitor;
+
typedef SmallVector<PartialDiagnosticAt, 1> OptionalNotes;
typedef std::pair<PartialDiagnosticAt, OptionalNotes> DelayedDiag;
typedef std::list<DelayedDiag> DiagList;
@@ -90,11 +92,11 @@
enum ConsumedState {
// No state information for the given variable.
- CS_None,
+ CS_None = 0,
- CS_Unknown,
- CS_Unconsumed,
- CS_Consumed
+ CS_Unknown = 1,
+ CS_Unconsumed = 2,
+ CS_Consumed = 3
};
class ConsumedStateMap {
@@ -144,18 +146,6 @@
void markVisited(const CFGBlock *Block);
};
-
- struct VarTestResult {
- const VarDecl *Var;
- SourceLocation Loc;
- bool UnconsumedInTrueBranch;
-
- VarTestResult() : Var(NULL), Loc(), UnconsumedInTrueBranch(true) {}
-
- VarTestResult(const VarDecl *Var, SourceLocation Loc,
- bool UnconsumedInTrueBranch)
- : Var(Var), Loc(Loc), UnconsumedInTrueBranch(UnconsumedInTrueBranch) {}
- };
/// A class that handles the analysis of uniqueness violations.
class ConsumedAnalyzer {
@@ -169,7 +159,8 @@
CacheMapType ConsumableTypeCache;
bool hasConsumableAttributes(const CXXRecordDecl *RD);
- void splitState(const CFGBlock *CurrBlock, const IfStmt *Terminator);
+ void splitState(const ConsumedStmtVisitor &Visitor,
+ const CFGBlock *CurrBlock, const IfStmt *Terminator);
public:
Index: lib/Analysis/Consumed.cpp
===================================================================
--- lib/Analysis/Consumed.cpp
+++ lib/Analysis/Consumed.cpp
@@ -32,6 +32,8 @@
// TODO: Mark variables as Unknown going into while- or for-loops only if they
// are referenced inside that block. (Deferred)
+// TODO: Handle variable definitions, e.g. bool valid = x.isValid();
+// if (valid) ...; (Deferred)
// TODO: Add a method(s) to identify which method calls perform what state
// transitions. (Deferred)
// TODO: Take notes on state transitions to provide better warning messages.
@@ -41,6 +43,8 @@
// TODO: Test IsFalseVisitor with values in the unknown state. (Deferred)
// TODO: Look into combining IsFalseVisitor and TestedVarsVisitor. (Deferred)
+#define INVERT_CONSUMED_UNCONSUMED(State) static_cast<ConsumedState>(State ^ 1)
+
using namespace clang;
using namespace consumed;
@@ -69,40 +73,79 @@
}
namespace {
+struct VarTestResult {
+ const VarDecl *Var;
+ SourceLocation Loc;
+ ConsumedState TrueBranchState;
+
+ VarTestResult() : Var(NULL), Loc(), TrueBranchState(CS_None) {}
+
+ VarTestResult(const VarDecl *Var, SourceLocation Loc,
+ ConsumedState TrueBranchState)
+ : Var(Var), Loc(Loc), TrueBranchState(TrueBranchState) {}
+};
+} // end anonymous::VarTestResult
+
+namespace clang {
+namespace consumed {
class ConsumedStmtVisitor : public ConstStmtVisitor<ConsumedStmtVisitor> {
- union PropagationUnion {
- ConsumedState State;
- const VarDecl *Var;
+ enum ConstantInfo {
+ CV_None,
+ CV_True,
+ CV_False
};
- class PropagationInfo {
- PropagationUnion StateOrVar;
+ struct RangeInfo {
+ unsigned Begin, End;
+ ConstantInfo ConstantValue;
+ };
- public:
- bool IsVar;
+ class PropagationInfo {
+ union PropagationUnion {
+ ConsumedState State;
+ RangeInfo Range;
+ const VarDecl *Var;
+ } InfoUnion;
- PropagationInfo() : IsVar(false) {
- StateOrVar.State = consumed::CS_None;
+ public:
+ enum {
+ IT_None,
+ IT_State,
+ IT_Range,
+ IT_Var
+ } InfoType;
+
+ PropagationInfo() : InfoType(IT_None) {}
+
+ PropagationInfo(unsigned RBegin, unsigned REnd) : InfoType(IT_Range) {
+ InfoUnion.Range.Begin = RBegin;
+ InfoUnion.Range.End = REnd;
+ InfoUnion.Range.ConstantValue = CV_None;
}
- PropagationInfo(ConsumedState State) : IsVar(false) {
- StateOrVar.State = State;
+ PropagationInfo(ConsumedState State) : InfoType(IT_State) {
+ InfoUnion.State = State;
}
- PropagationInfo(const VarDecl *Var) : IsVar(true) {
- StateOrVar.Var = Var;
+ PropagationInfo(const VarDecl *Var) : InfoType(IT_Var) {
+ InfoUnion.Var = Var;
}
- ConsumedState getState() const { return StateOrVar.State; }
+ const ConsumedState & getState() const { return InfoUnion.State; }
+ const RangeInfo & getRange() const { return InfoUnion.Range; }
+ const VarDecl * getVar() const { return InfoUnion.Var; }
- const VarDecl * getVar() const { return IsVar ? StateOrVar.Var : NULL; }
+ bool isState() const { return InfoType == IT_State; }
+ bool isRange() const { return InfoType == IT_Range; }
+ bool isVar() const { return InfoType == IT_Var; }
};
typedef llvm::DenseMap<const Stmt *, PropagationInfo> MapType;
typedef std::pair<const Stmt *, PropagationInfo> PairType;
typedef MapType::iterator InfoEntry;
-
+ typedef MapType::const_iterator ConstInfoEntry;
+
AnalysisDeclContext &AC;
ConsumedAnalyzer &Analyzer;
ConsumedStateMap *StateMap;
@@ -115,6 +158,8 @@
bool isLikeMoveAssignment(const CXXMethodDecl *MethodDecl);
public:
+
+ llvm::SmallVector<VarTestResult, 5> TestedVars;
void Visit(const Stmt *StmtNode);
@@ -134,22 +179,34 @@
ConsumedStmtVisitor(AnalysisDeclContext &AC, ConsumedAnalyzer &Analyzer,
ConsumedStateMap *StateMap)
: AC(AC), Analyzer(Analyzer), StateMap(StateMap) {}
-
+
+ std::pair<unsigned, unsigned> getTestRange(const Stmt *StmtNode) const {
+ ConstInfoEntry Entry = PropagationMap.find(StmtNode);
+
+ if (Entry != PropagationMap.end() && Entry->second.isRange()) {
+ return std::make_pair(Entry->second.getRange().Begin,
+ Entry->second.getRange().End);
+ } else {
+ return std::make_pair(0, 0);
+ }
+ }
+
void reset() {
PropagationMap.clear();
+ TestedVars.clear();
}
};
// TODO: When we support CallableWhenConsumed this will have to check for
// the different attributes and change the behavior bellow. (Deferred)
-void ConsumedStmtVisitor::checkCallability(const PropagationInfo &PState,
+void ConsumedStmtVisitor::checkCallability(const PropagationInfo &PInfo,
const FunctionDecl *FunDecl,
const CallExpr *Call) {
if (!FunDecl->hasAttr<CallableWhenUnconsumedAttr>()) return;
- if (PState.IsVar) {
- const VarDecl *Var = PState.getVar();
+ if (PInfo.isVar()) {
+ const VarDecl *Var = PInfo.getVar();
switch (StateMap->getState(Var)) {
case CS_Consumed:
@@ -170,7 +227,7 @@
}
} else {
- switch (PState.getState()) {
+ switch (PInfo.getState()) {
case CS_Consumed:
Analyzer.WarningsHandler.warnUseOfTempWhileConsumed(
FunDecl->getNameAsString(), Call->getExprLoc());
@@ -250,24 +307,24 @@
InfoEntry Entry = PropagationMap.find(Call->getArg(Index));
- if (Entry == PropagationMap.end() || !Entry->second.IsVar) {
+ if (Entry == PropagationMap.end() || !Entry->second.isVar()) {
continue;
}
- PropagationInfo PState = Entry->second;
+ PropagationInfo PInfo = Entry->second;
if (ParamType->isRValueReferenceType() ||
(ParamType->isLValueReferenceType() &&
!cast<LValueReferenceType>(*ParamType).isSpelledAsLValue())) {
- StateMap->setState(PState.getVar(), consumed::CS_Consumed);
+ StateMap->setState(PInfo.getVar(), consumed::CS_Consumed);
} else if (!(ParamType.isConstQualified() ||
((ParamType->isReferenceType() ||
ParamType->isPointerType()) &&
ParamType->getPointeeType().isConstQualified()))) {
- StateMap->setState(PState.getVar(), consumed::CS_Unknown);
+ StateMap->setState(PInfo.getVar(), consumed::CS_Unknown);
}
}
}
@@ -295,11 +352,11 @@
} else if (Constructor->isMoveConstructor()) {
- PropagationInfo PState =
+ PropagationInfo PInfo =
PropagationMap.find(Call->getArg(0))->second;
- if (PState.IsVar) {
- const VarDecl* Var = PState.getVar();
+ if (PInfo.isVar()) {
+ const VarDecl* Var = PInfo.getVar();
PropagationMap.insert(PairType(Call,
PropagationInfo(StateMap->getState(Var))));
@@ -307,7 +364,7 @@
StateMap->setState(Var, consumed::CS_Consumed);
} else {
- PropagationMap.insert(PairType(Call, PState));
+ PropagationMap.insert(PairType(Call, PInfo));
}
} else if (Constructor->isCopyConstructor()) {
@@ -331,16 +388,27 @@
InfoEntry Entry = PropagationMap.find(Call->getCallee()->IgnoreParens());
if (Entry != PropagationMap.end()) {
- PropagationInfo PState = Entry->second;
+ PropagationInfo PInfo = Entry->second;
const CXXMethodDecl *MethodDecl = Call->getMethodDecl();
- checkCallability(PState, MethodDecl, Call);
+ checkCallability(PInfo, MethodDecl, Call);
- if (PState.IsVar) {
- if (MethodDecl->hasAttr<ConsumesAttr>())
- StateMap->setState(PState.getVar(), consumed::CS_Consumed);
- else if (!MethodDecl->isConst())
- StateMap->setState(PState.getVar(), consumed::CS_Unknown);
+ if (PInfo.isVar()) {
+ if (isTestingFunction(MethodDecl)) {
+ unsigned short CurrIndex = TestedVars.size();
+
+ PropagationMap.insert(PairType(Call,
+ PropagationInfo(CurrIndex, CurrIndex + 1)));
+
+ TestedVars.push_back(
+ VarTestResult(PInfo.getVar(), Call->getExprLoc(), CS_Unconsumed));
+
+ } else if (MethodDecl->hasAttr<ConsumesAttr>()) {
+ StateMap->setState(PInfo.getVar(), consumed::CS_Consumed);
+
+ } else if (!MethodDecl->isConst()) {
+ StateMap->setState(PInfo.getVar(), consumed::CS_Unknown);
+ }
}
}
}
@@ -359,46 +427,46 @@
InfoEntry LEntry = PropagationMap.find(Call->getArg(0));
InfoEntry REntry = PropagationMap.find(Call->getArg(1));
- PropagationInfo LPState, RPState;
+ PropagationInfo LPInfo, RPInfo;
if (LEntry != PropagationMap.end() &&
REntry != PropagationMap.end()) {
- LPState = LEntry->second;
- RPState = REntry->second;
+ LPInfo = LEntry->second;
+ RPInfo = REntry->second;
- if (LPState.IsVar && RPState.IsVar) {
- StateMap->setState(LPState.getVar(),
- StateMap->getState(RPState.getVar()));
+ if (LPInfo.isVar() && RPInfo.isVar()) {
+ StateMap->setState(LPInfo.getVar(),
+ StateMap->getState(RPInfo.getVar()));
- StateMap->setState(RPState.getVar(), consumed::CS_Consumed);
+ StateMap->setState(RPInfo.getVar(), consumed::CS_Consumed);
- PropagationMap.insert(PairType(Call, LPState));
+ PropagationMap.insert(PairType(Call, LPInfo));
- } else if (LPState.IsVar && !RPState.IsVar) {
- StateMap->setState(LPState.getVar(), RPState.getState());
+ } else if (LPInfo.isVar() && !RPInfo.isVar()) {
+ StateMap->setState(LPInfo.getVar(), RPInfo.getState());
- PropagationMap.insert(PairType(Call, LPState));
+ PropagationMap.insert(PairType(Call, LPInfo));
- } else if (!LPState.IsVar && RPState.IsVar) {
+ } else if (!LPInfo.isVar() && RPInfo.isVar()) {
PropagationMap.insert(PairType(Call,
- PropagationInfo(StateMap->getState(RPState.getVar()))));
+ PropagationInfo(StateMap->getState(RPInfo.getVar()))));
- StateMap->setState(RPState.getVar(), consumed::CS_Consumed);
+ StateMap->setState(RPInfo.getVar(), consumed::CS_Consumed);
} else {
- PropagationMap.insert(PairType(Call, RPState));
+ PropagationMap.insert(PairType(Call, RPInfo));
}
} else if (LEntry != PropagationMap.end() &&
REntry == PropagationMap.end()) {
- LPState = LEntry->second;
+ LPInfo = LEntry->second;
- if (LPState.IsVar) {
- StateMap->setState(LPState.getVar(), consumed::CS_Unknown);
+ if (LPInfo.isVar()) {
+ StateMap->setState(LPInfo.getVar(), consumed::CS_Unknown);
- PropagationMap.insert(PairType(Call, LPState));
+ PropagationMap.insert(PairType(Call, LPInfo));
} else {
PropagationMap.insert(PairType(Call,
@@ -408,10 +476,10 @@
} else if (LEntry == PropagationMap.end() &&
REntry != PropagationMap.end()) {
- RPState = REntry->second;
+ RPInfo = REntry->second;
- if (RPState.IsVar) {
- const VarDecl *Var = RPState.getVar();
+ if (RPInfo.isVar()) {
+ const VarDecl *Var = RPInfo.getVar();
PropagationMap.insert(PairType(Call,
PropagationInfo(StateMap->getState(Var))));
@@ -419,7 +487,7 @@
StateMap->setState(Var, consumed::CS_Consumed);
} else {
- PropagationMap.insert(PairType(Call, RPState));
+ PropagationMap.insert(PairType(Call, RPInfo));
}
}
@@ -430,20 +498,30 @@
InfoEntry Entry = PropagationMap.find(Call->getArg(0));
if (Entry != PropagationMap.end()) {
- PropagationInfo PState = Entry->second;
+ PropagationInfo PInfo = Entry->second;
- checkCallability(PState, FunDecl, Call);
+ checkCallability(PInfo, FunDecl, Call);
- if (PState.IsVar) {
- if (FunDecl->hasAttr<ConsumesAttr>()) {
+ if (PInfo.isVar()) {
+ if (isTestingFunction(FunDecl)) {
+ unsigned short CurrIndex = TestedVars.size();
+
+ PropagationMap.insert(PairType(Call,
+ PropagationInfo(CurrIndex, CurrIndex + 1)));
+
+ TestedVars.push_back(
+ VarTestResult(PInfo.getVar(), Call->getExprLoc(), CS_Unconsumed));
+
+ } else if (FunDecl->hasAttr<ConsumesAttr>()) {
// Handle consuming operators.
- StateMap->setState(PState.getVar(), consumed::CS_Consumed);
+ StateMap->setState(PInfo.getVar(), consumed::CS_Consumed);
+
} else if (const CXXMethodDecl *MethodDecl =
dyn_cast_or_null<CXXMethodDecl>(FunDecl)) {
// Handle non-constant member operators.
if (!MethodDecl->isConst())
- StateMap->setState(PState.getVar(), consumed::CS_Unknown);
+ StateMap->setState(PInfo.getVar(), consumed::CS_Unknown);
}
}
}
@@ -482,84 +560,42 @@
}
void ConsumedStmtVisitor::VisitUnaryOperator(const UnaryOperator *UOp) {
- if (UOp->getOpcode() == UO_AddrOf) {
- InfoEntry Entry = PropagationMap.find(UOp->getSubExpr());
-
- if (Entry != PropagationMap.end())
- PropagationMap.insert(PairType(UOp, Entry->second));
- }
-}
-
-void ConsumedStmtVisitor::VisitVarDecl(const VarDecl *Var) {
- if (Analyzer.isConsumableType(Var->getType())) {
- PropagationInfo PState =
- PropagationMap.find(Var->getInit())->second;
-
- StateMap->setState(Var, PState.IsVar ?
- StateMap->getState(PState.getVar()) : PState.getState());
- }
-}
-} // end anonymous::ConsumedStmtVisitor
-
-namespace {
-
-// TODO: Handle variable definitions, e.g. bool valid = x.isValid();
-// if (valid) ...; (Deferred)
-class TestedVarsVisitor : public RecursiveASTVisitor<TestedVarsVisitor> {
-
- bool Invert;
- SourceLocation CurrTestLoc;
+ InfoEntry Entry = PropagationMap.find(UOp->getSubExpr());
+ if (Entry == PropagationMap.end()) return;
- ConsumedStateMap *StateMap;
-
-public:
- bool IsUsefulConditional;
- VarTestResult Test;
-
- TestedVarsVisitor(ConsumedStateMap *StateMap) : Invert(false),
- StateMap(StateMap), IsUsefulConditional(false) {}
+ switch (UOp->getOpcode()) {
+ case UO_AddrOf:
+ PropagationMap.insert(PairType(UOp, Entry->second));
+ break;
- bool VisitCallExpr(CallExpr *Call);
- bool VisitDeclRefExpr(DeclRefExpr *DeclRef);
- bool VisitUnaryOperator(UnaryOperator *UnaryOp);
-};
-
-bool TestedVarsVisitor::VisitCallExpr(CallExpr *Call) {
- if (const FunctionDecl *FunDecl =
- dyn_cast_or_null<FunctionDecl>(Call->getDirectCallee())) {
-
- if (isTestingFunction(FunDecl)) {
- CurrTestLoc = Call->getExprLoc();
- IsUsefulConditional = true;
- return true;
+ case UO_LNot:
+ if (Entry->second.isRange()) {
+ const RangeInfo &Range = Entry->second.getRange();
+
+ for (unsigned index = Range.Begin; index < Range.End; ++index) {
+ ConsumedState &TestState = TestedVars[index].TrueBranchState;
+ TestState = INVERT_CONSUMED_UNCONSUMED(TestState);
+ }
+
+ PropagationMap.insert(PairType(UOp, PropagationInfo(Entry->second)));
}
-
- IsUsefulConditional = false;
- }
-
- return false;
-}
-
-bool TestedVarsVisitor::VisitDeclRefExpr(DeclRefExpr *DeclRef) {
- if (const VarDecl *Var = dyn_cast_or_null<VarDecl>(DeclRef->getDecl()))
- if (StateMap->getState(Var) != consumed::CS_None)
- Test = VarTestResult(Var, CurrTestLoc, !Invert);
+ break;
- return true;
+ default:
+ break;
+ }
}
-bool TestedVarsVisitor::VisitUnaryOperator(UnaryOperator *UnaryOp) {
- if (UnaryOp->getOpcode() == UO_LNot) {
- Invert = true;
- TraverseStmt(UnaryOp->getSubExpr());
+void ConsumedStmtVisitor::VisitVarDecl(const VarDecl *Var) {
+ if (Analyzer.isConsumableType(Var->getType())) {
+ PropagationInfo PInfo =
+ PropagationMap.find(Var->getInit())->second;
- } else {
- IsUsefulConditional = false;
+ StateMap->setState(Var, PInfo.isVar() ?
+ StateMap->getState(PInfo.getVar()) : PInfo.getState());
}
-
- return false;
}
-} // end anonymouse::TestedVarsVisitor
+}} // end clang::consumed::ConsumedStmtVisitor
namespace clang {
namespace consumed {
@@ -696,33 +732,37 @@
// TODO: Handle other forms of branching with precision, including while- and
// for-loops. (Deferred)
-void ConsumedAnalyzer::splitState(const CFGBlock *CurrBlock,
+void ConsumedAnalyzer::splitState(const ConsumedStmtVisitor &Visitor,
+ const CFGBlock *CurrBlock,
const IfStmt *Terminator) {
- TestedVarsVisitor Visitor(CurrStates);
- Visitor.TraverseStmt(const_cast<Expr*>(Terminator->getCond()));
+ bool HasElse = Terminator->getElse() != NULL;
+ ConsumedStateMap *ElseOrMergeStates = new ConsumedStateMap(*CurrStates);
- bool HasElse = Terminator->getElse() != NULL;
+ std::pair<unsigned, unsigned> TestRange =
+ Visitor.getTestRange(Terminator->getCond());
- ConsumedStateMap *ElseOrMergeStates = new ConsumedStateMap(*CurrStates);
+ Terminator->getCond()->dump();
- if (Visitor.IsUsefulConditional) {
- ConsumedState VarState = CurrStates->getState(Visitor.Test.Var);
+ // When there are no valid tests the start and end index will both be 0.
+ while (TestRange.first < TestRange.second) {
+ const VarTestResult &Test = Visitor.TestedVars[TestRange.first++];
+ ConsumedState VarState = CurrStates->getState(Test.Var);
if (VarState != CS_Unknown) {
// FIXME: Make this not warn if the test is from a macro expansion.
// (Deferred)
- WarningsHandler.warnUnnecessaryTest(Visitor.Test.Var->getNameAsString(),
- stateToString(VarState), Visitor.Test.Loc);
+ WarningsHandler.warnUnnecessaryTest(Test.Var->getNameAsString(),
+ stateToString(VarState), Test.Loc);
}
- if (Visitor.Test.UnconsumedInTrueBranch) {
- CurrStates->setState(Visitor.Test.Var, CS_Unconsumed);
- if (HasElse) ElseOrMergeStates->setState(Visitor.Test.Var, CS_Consumed);
+ if (Test.TrueBranchState == CS_Unconsumed) {
+ CurrStates->setState(Test.Var, CS_Unconsumed);
+ if (HasElse) ElseOrMergeStates->setState(Test.Var, CS_Consumed);
} else {
- CurrStates->setState(Visitor.Test.Var, CS_Consumed);
- if (HasElse) ElseOrMergeStates->setState(Visitor.Test.Var, CS_Unconsumed);
+ CurrStates->setState(Test.Var, CS_Consumed);
+ if (HasElse) ElseOrMergeStates->setState(Test.Var, CS_Unconsumed);
}
}
@@ -773,7 +813,7 @@
if (const IfStmt *Terminator =
dyn_cast_or_null<IfStmt>(CurrBlock->getTerminator().getStmt())) {
- splitState(CurrBlock, Terminator);
+ splitState(Visitor, CurrBlock, Terminator);
CurrStates = NULL;
} else if (CurrBlock->succ_size() > 1) {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
