Index: cfe/trunk/include/clang/Analysis/CFG.h =================================================================== --- cfe/trunk/include/clang/Analysis/CFG.h +++ cfe/trunk/include/clang/Analysis/CFG.h @@ -494,33 +494,44 @@ /// Represents CFGBlock terminator statement. /// -/// TemporaryDtorsBranch bit is set to true if the terminator marks a branch -/// in control flow of destructors of temporaries. In this case terminator -/// statement is the same statement that branches control flow in evaluation -/// of matching full expression. class CFGTerminator { - llvm::PointerIntPair Data; +public: + enum Kind { + /// A branch that corresponds to a statement in the code, + /// such as an if-statement. + StmtBranch, + /// A branch in control flow of destructors of temporaries. In this case + /// terminator statement is the same statement that branches control flow + /// in evaluation of matching full expression. + TemporaryDtorsBranch, + + /// Number of different kinds, for sanity checks. We subtract 1 so that + /// to keep receiving compiler warnings when we don't cover all enum values + /// in a switch. + NumKindsMinusOne = TemporaryDtorsBranch + }; + +private: + static constexpr int KindBits = 1; + static_assert((1 << KindBits) > NumKindsMinusOne, + "Not enough room for kind!"); + llvm::PointerIntPair Data; public: - CFGTerminator() = default; - CFGTerminator(Stmt *S, bool TemporaryDtorsBranch = false) - : Data(S, TemporaryDtorsBranch) {} + CFGTerminator() { assert(!isValid()); } + CFGTerminator(Stmt *S, Kind K = StmtBranch) : Data(S, K) {} + bool isValid() const { return Data.getOpaqueValue() != nullptr; } Stmt *getStmt() { return Data.getPointer(); } const Stmt *getStmt() const { return Data.getPointer(); } + Kind getKind() const { return static_cast(Data.getInt()); } - bool isTemporaryDtorsBranch() const { return Data.getInt(); } - - operator Stmt *() { return getStmt(); } - operator const Stmt *() const { return getStmt(); } - - Stmt *operator->() { return getStmt(); } - const Stmt *operator->() const { return getStmt(); } - - Stmt &operator*() { return *getStmt(); } - const Stmt &operator*() const { return *getStmt(); } - - explicit operator bool() const { return getStmt(); } + bool isStmtBranch() const { + return getKind() == StmtBranch; + } + bool isTemporaryDtorsBranch() const { + return getKind() == TemporaryDtorsBranch; + } }; /// Represents a single basic block in a source-level CFG. @@ -836,8 +847,10 @@ void setLoopTarget(const Stmt *loopTarget) { LoopTarget = loopTarget; } void setHasNoReturnElement() { HasNoReturnElement = true; } - CFGTerminator getTerminator() { return Terminator; } - const CFGTerminator getTerminator() const { return Terminator; } + CFGTerminator getTerminator() const { return Terminator; } + + Stmt *getTerminatorStmt() { return Terminator.getStmt(); } + const Stmt *getTerminatorStmt() const { return Terminator.getStmt(); } Stmt *getTerminatorCondition(bool StripParens = true); Index: cfe/trunk/include/clang/Analysis/ProgramPoint.h =================================================================== --- cfe/trunk/include/clang/Analysis/ProgramPoint.h +++ cfe/trunk/include/clang/Analysis/ProgramPoint.h @@ -257,7 +257,7 @@ } const Stmt *getTerminator() const { - return getBlock()->getTerminator(); + return getBlock()->getTerminatorStmt(); } private: Index: cfe/trunk/lib/Analysis/CFG.cpp =================================================================== --- cfe/trunk/lib/Analysis/CFG.cpp +++ cfe/trunk/lib/Analysis/CFG.cpp @@ -1956,7 +1956,7 @@ = Blk->beginAutomaticObjDtorsInsert(Blk->end(), B.distance(E), C); for (LocalScope::const_iterator I = B; I != E; ++I) InsertPos = Blk->insertAutomaticObjDtor(InsertPos, *I, - Blk->getTerminator()); + Blk->getTerminatorStmt()); } /// prependAutomaticObjLifetimeWithTerminator - Prepend lifetime CFGElements for @@ -1971,8 +1971,10 @@ BumpVectorContext &C = cfg->getBumpVectorContext(); CFGBlock::iterator InsertPos = Blk->beginLifetimeEndsInsert(Blk->end(), B.distance(E), C); - for (LocalScope::const_iterator I = B; I != E; ++I) - InsertPos = Blk->insertLifetimeEnds(InsertPos, *I, Blk->getTerminator()); + for (LocalScope::const_iterator I = B; I != E; ++I) { + InsertPos = + Blk->insertLifetimeEnds(InsertPos, *I, Blk->getTerminatorStmt()); + } } /// prependAutomaticObjScopeEndWithTerminator - Prepend scope end CFGElements for @@ -1991,7 +1993,7 @@ LocalScope::const_iterator PlaceToInsert = B; for (LocalScope::const_iterator I = B; I != E; ++I) PlaceToInsert = I; - Blk->insertScopeEnd(InsertPos, *PlaceToInsert, Blk->getTerminator()); + Blk->insertScopeEnd(InsertPos, *PlaceToInsert, Blk->getTerminatorStmt()); return *PlaceToInsert; } @@ -4612,7 +4614,8 @@ } assert(Context.TerminatorExpr); CFGBlock *Decision = createBlock(false); - Decision->setTerminator(CFGTerminator(Context.TerminatorExpr, true)); + Decision->setTerminator(CFGTerminator(Context.TerminatorExpr, + CFGTerminator::TemporaryDtorsBranch)); addSuccessor(Decision, Block, !Context.KnownExecuted.isFalse()); addSuccessor(Decision, FalseSucc ? FalseSucc : Context.Succ, !Context.KnownExecuted.isTrue()); @@ -4820,7 +4823,7 @@ // If the 'To' has no label or is labeled but the label isn't a // CaseStmt then filter this edge. if (const SwitchStmt *S = - dyn_cast_or_null(From->getTerminator().getStmt())) { + dyn_cast_or_null(From->getTerminatorStmt())) { if (S->isAllEnumCasesCovered()) { const Stmt *L = To->getLabel(); if (!L || !isa(L)) @@ -5055,9 +5058,15 @@ public: void print(CFGTerminator T) { - if (T.isTemporaryDtorsBranch()) + switch (T.getKind()) { + case CFGTerminator::StmtBranch: + Visit(T.getStmt()); + break; + case CFGTerminator::TemporaryDtorsBranch: OS << "(Temp Dtor) "; - Visit(T.getStmt()); + Visit(T.getStmt()); + break; + } } }; @@ -5366,7 +5375,7 @@ } // Print the terminator of this block. - if (B.getTerminator()) { + if (B.getTerminator().isValid()) { if (ShowColors) OS.changeColor(raw_ostream::GREEN); @@ -5519,7 +5528,7 @@ } Stmt *CFGBlock::getTerminatorCondition(bool StripParens) { - Stmt *Terminator = this->Terminator; + Stmt *Terminator = getTerminatorStmt(); if (!Terminator) return nullptr; Index: cfe/trunk/lib/Analysis/CFGStmtMap.cpp =================================================================== --- cfe/trunk/lib/Analysis/CFGStmtMap.cpp +++ cfe/trunk/lib/Analysis/CFGStmtMap.cpp @@ -70,7 +70,7 @@ // Finally, look at the terminator. If the terminator was already added // because it is a block-level expression in another block, overwrite // that mapping. - if (Stmt *Term = B->getTerminator()) + if (Stmt *Term = B->getTerminatorStmt()) SM[Term] = B; } Index: cfe/trunk/lib/Analysis/Consumed.cpp =================================================================== --- cfe/trunk/lib/Analysis/Consumed.cpp +++ cfe/trunk/lib/Analysis/Consumed.cpp @@ -76,7 +76,7 @@ static SourceLocation getLastStmtLoc(const CFGBlock *Block) { // Find the source location of the last statement in the block, if the block // is not empty. - if (const Stmt *StmtNode = Block->getTerminator()) { + if (const Stmt *StmtNode = Block->getTerminatorStmt()) { return StmtNode->getBeginLoc(); } else { for (CFGBlock::const_reverse_iterator BI = Block->rbegin(), Index: cfe/trunk/lib/Analysis/LiveVariables.cpp =================================================================== --- cfe/trunk/lib/Analysis/LiveVariables.cpp +++ cfe/trunk/lib/Analysis/LiveVariables.cpp @@ -501,7 +501,7 @@ TransferFunctions TF(*this, val, obs, block); // Visit the terminator (if any). - if (const Stmt *term = block->getTerminator()) + if (const Stmt *term = block->getTerminatorStmt()) TF.Visit(const_cast(term)); // Apply the transfer function for all Stmts in the block. Index: cfe/trunk/lib/Analysis/ProgramPoint.cpp =================================================================== --- cfe/trunk/lib/Analysis/ProgramPoint.cpp +++ cfe/trunk/lib/Analysis/ProgramPoint.cpp @@ -144,7 +144,7 @@ Out << "Edge: (B" << E.getSrc()->getBlockID() << ", B" << E.getDst()->getBlockID() << ')'; - if (const Stmt *T = E.getSrc()->getTerminator()) { + if (const Stmt *T = E.getSrc()->getTerminatorStmt()) { SourceLocation SLoc = T->getBeginLoc(); Out << "\\|Terminator: "; Index: cfe/trunk/lib/Analysis/ReachableCode.cpp =================================================================== --- cfe/trunk/lib/Analysis/ReachableCode.cpp +++ cfe/trunk/lib/Analysis/ReachableCode.cpp @@ -48,7 +48,7 @@ static bool isTrivialDoWhile(const CFGBlock *B, const Stmt *S) { // Check if the block ends with a do...while() and see if 'S' is the // condition. - if (const Stmt *Term = B->getTerminator()) { + if (const Stmt *Term = B->getTerminatorStmt()) { if (const DoStmt *DS = dyn_cast(Term)) { const Expr *Cond = DS->getCond()->IgnoreParenCasts(); return Cond == S && isTrivialExpression(Cond); @@ -116,7 +116,7 @@ // the call to the destructor. assert(Current->succ_size() == 2); Current = *(Current->succ_begin() + 1); - } else if (!Current->getTerminator() && Current->succ_size() == 1) { + } else if (!Current->getTerminatorStmt() && Current->succ_size() == 1) { // If there is only one successor, we're not dealing with outgoing control // flow. Thus, look into the next block. Current = *Current->succ_begin(); @@ -292,7 +292,7 @@ /// Returns true if we should always explore all successors of a block. static bool shouldTreatSuccessorsAsReachable(const CFGBlock *B, Preprocessor &PP) { - if (const Stmt *Term = B->getTerminator()) { + if (const Stmt *Term = B->getTerminatorStmt()) { if (isa(Term)) return true; // Specially handle '||' and '&&'. @@ -461,12 +461,11 @@ return S; } - if (CFGTerminator T = Block->getTerminator()) { - if (!T.isTemporaryDtorsBranch()) { - const Stmt *S = T.getStmt(); - if (isValidDeadStmt(S)) - return S; - } + CFGTerminator T = Block->getTerminator(); + if (T.isStmtBranch()) { + const Stmt *S = T.getStmt(); + if (S && isValidDeadStmt(S)) + return S; } return nullptr; Index: cfe/trunk/lib/Analysis/ThreadSafety.cpp =================================================================== --- cfe/trunk/lib/Analysis/ThreadSafety.cpp +++ cfe/trunk/lib/Analysis/ThreadSafety.cpp @@ -815,7 +815,7 @@ // Find the source location of the last statement in the block, if the // block is not empty. - if (const Stmt *S = CurrBlock->getTerminator()) { + if (const Stmt *S = CurrBlock->getTerminatorStmt()) { CurrBlockInfo->EntryLoc = CurrBlockInfo->ExitLoc = S->getBeginLoc(); } else { for (CFGBlock::const_reverse_iterator BI = CurrBlock->rbegin(), @@ -1499,7 +1499,7 @@ const Stmt *Cond = PredBlock->getTerminatorCondition(); // We don't acquire try-locks on ?: branches, only when its result is used. - if (!Cond || isa(PredBlock->getTerminator())) + if (!Cond || isa(PredBlock->getTerminatorStmt())) return; bool Negate = false; @@ -2402,7 +2402,7 @@ // a difference in locksets is probably due to a bug in that block, rather // than in some other predecessor. In that case, keep the other // predecessor's lockset. - if (const Stmt *Terminator = (*PI)->getTerminator()) { + if (const Stmt *Terminator = (*PI)->getTerminatorStmt()) { if (isa(Terminator) || isa(Terminator)) { SpecialBlocks.push_back(*PI); continue; @@ -2441,7 +2441,7 @@ // it might also be part of a switch. Also, a subsequent destructor // might add to the lockset, in which case the real issue might be a // double lock on the other path. - const Stmt *Terminator = PrevBlock->getTerminator(); + const Stmt *Terminator = PrevBlock->getTerminatorStmt(); bool IsLoop = Terminator && isa(Terminator); FactSet PrevLockset; Index: cfe/trunk/lib/Analysis/UninitializedValues.cpp =================================================================== --- cfe/trunk/lib/Analysis/UninitializedValues.cpp +++ cfe/trunk/lib/Analysis/UninitializedValues.cpp @@ -651,7 +651,7 @@ // uninitialized. for (const auto *Block : cfg) { unsigned BlockID = Block->getBlockID(); - const Stmt *Term = Block->getTerminator(); + const Stmt *Term = Block->getTerminatorStmt(); if (SuccsVisited[BlockID] && SuccsVisited[BlockID] < Block->succ_size() && Term) { // This block inevitably leads to the use. If we have an edge from here Index: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp =================================================================== --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp @@ -398,7 +398,8 @@ for (const auto *B : *cfg) { if (!live[B->getBlockID()]) { if (B->pred_begin() == B->pred_end()) { - if (B->getTerminator() && isa(B->getTerminator())) + const Stmt *Term = B->getTerminatorStmt(); + if (Term && isa(Term)) // When not adding EH edges from calls, catch clauses // can otherwise seem dead. Avoid noting them as dead. count += reachable_code::ScanReachableFromBlock(B, live); @@ -446,7 +447,8 @@ // No more CFGElements in the block? if (ri == re) { - if (B.getTerminator() && isa(B.getTerminator())) { + const Stmt *Term = B.getTerminatorStmt(); + if (Term && isa(Term)) { HasAbnormalEdge = true; continue; } @@ -1077,7 +1079,7 @@ BlockQueue.pop_front(); if (!P) continue; - const Stmt *Term = P->getTerminator(); + const Stmt *Term = P->getTerminatorStmt(); if (Term && isa(Term)) continue; // Switch statement, good. @@ -1175,7 +1177,7 @@ } static const Stmt *getLastStmt(const CFGBlock &B) { - if (const Stmt *Term = B.getTerminator()) + if (const Stmt *Term = B.getTerminatorStmt()) return Term; for (CFGBlock::const_reverse_iterator ElemIt = B.rbegin(), ElemEnd = B.rend(); @@ -1281,11 +1283,11 @@ if (L.isMacroID()) continue; if (S.getLangOpts().CPlusPlus11) { - const Stmt *Term = B->getTerminator(); + const Stmt *Term = B->getTerminatorStmt(); // Skip empty cases. while (B->empty() && !Term && B->succ_size() == 1) { B = *B->succ_begin(); - Term = B->getTerminator(); + Term = B->getTerminatorStmt(); } if (!(B->empty() && Term && isa(Term))) { Preprocessor &PP = S.getPreprocessor(); Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp @@ -204,7 +204,7 @@ return S->getStmt(); } } - if (const Stmt *S = CB->getTerminator()) + if (const Stmt *S = CB->getTerminatorStmt()) return S; else return nullptr; @@ -250,7 +250,7 @@ bool UnreachableCodeChecker::isEmptyCFGBlock(const CFGBlock *CB) { return CB->getLabel() == nullptr // No labels && CB->size() == 0 // No statements - && !CB->getTerminator(); // No terminator + && !CB->getTerminatorStmt(); // No terminator } void ento::registerUnreachableCodeChecker(CheckerManager &mgr) { Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -678,7 +678,7 @@ const LocationContext *LC = N->getLocationContext(); const CFGBlock *Src = BE.getSrc(); const CFGBlock *Dst = BE.getDst(); - const Stmt *T = Src->getTerminator(); + const Stmt *T = Src->getTerminatorStmt(); if (!T) return; @@ -1203,7 +1203,7 @@ const CFGBlock *BSrc = BE->getSrc(); ParentMap &PM = PDB.getParentMap(); - if (const Stmt *Term = BSrc->getTerminator()) { + if (const Stmt *Term = BSrc->getTerminatorStmt()) { // Are we jumping past the loop body without ever executing the // loop (because the condition was false)? if (isLoop(Term)) { Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -1494,7 +1494,7 @@ return nullptr; CFGStmtMap *Map = CurLC->getAnalysisDeclContext()->getCFGStmtMap(); - CurTerminatorStmt = Map->getBlock(CurStmt)->getTerminator(); + CurTerminatorStmt = Map->getBlock(CurStmt)->getTerminatorStmt(); } else { return nullptr; } @@ -1566,7 +1566,7 @@ ProgramPoint ProgPoint = NI->getLocation(); if (Optional BE = ProgPoint.getAs()) { const CFGBlock *srcBlk = BE->getSrc(); - if (const Stmt *term = srcBlk->getTerminator()) { + if (const Stmt *term = srcBlk->getTerminatorStmt()) { if (term == CO) { bool TookTrueBranch = (*(srcBlk->succ_begin()) == BE->getDst()); if (TookTrueBranch) @@ -1852,7 +1852,7 @@ // here by looking at the state transition. if (Optional BE = progPoint.getAs()) { const CFGBlock *srcBlk = BE->getSrc(); - if (const Stmt *term = srcBlk->getTerminator()) + if (const Stmt *term = srcBlk->getTerminatorStmt()) return VisitTerminator(term, N, srcBlk, BE->getDst(), BR, BRC); return nullptr; } Index: cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp @@ -275,14 +275,14 @@ } void CoreEngine::HandleBlockExit(const CFGBlock * B, ExplodedNode *Pred) { - if (const Stmt *Term = B->getTerminator()) { + if (const Stmt *Term = B->getTerminatorStmt()) { switch (Term->getStmtClass()) { default: llvm_unreachable("Analysis for this terminator not implemented."); case Stmt::CXXBindTemporaryExprClass: HandleCleanupTemporaryBranch( - cast(B->getTerminator().getStmt()), B, Pred); + cast(Term), B, Pred); return; // Model static initializers. Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1862,7 +1862,7 @@ // other constraints) then consider completely unrolling it. if(AMgr.options.ShouldUnrollLoops) { unsigned maxBlockVisitOnPath = AMgr.options.maxBlockVisitOnPath; - const Stmt *Term = nodeBuilder.getContext().getBlock()->getTerminator(); + const Stmt *Term = nodeBuilder.getContext().getBlock()->getTerminatorStmt(); if (Term) { ProgramStateRef NewState = updateLoopStack(Term, AMgr.getASTContext(), Pred, maxBlockVisitOnPath); @@ -1883,7 +1883,7 @@ unsigned int BlockCount = nodeBuilder.getContext().blockCount(); if (BlockCount == AMgr.options.maxBlockVisitOnPath - 1 && AMgr.options.ShouldWidenLoops) { - const Stmt *Term = nodeBuilder.getContext().getBlock()->getTerminator(); + const Stmt *Term = nodeBuilder.getContext().getBlock()->getTerminatorStmt(); if (!(Term && (isa(Term) || isa(Term) || isa(Term)))) return; @@ -2008,8 +2008,8 @@ if (!BO || !BO->isLogicalOp()) return Condition; - assert(!B->getTerminator().isTemporaryDtorsBranch() && - "Temporary destructor branches handled by processBindTemporary."); + assert(B->getTerminator().isStmtBranch() && + "Other kinds of branches are handled separately!"); // For logical operations, we still have the case where some branches // use the traditional "merge" approach and others sink the branch Index: cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp @@ -234,7 +234,7 @@ ProgramPoint P = N->getLocation(); if (Optional BE = P.getAs()) - S = BE->getBlock()->getTerminator(); + S = BE->getBlock()->getTerminatorStmt(); if (S == LoopStmt) return false; Index: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -794,7 +794,7 @@ if (auto SP = P.getAs()) return SP->getStmt(); if (auto BE = P.getAs()) - return BE->getSrc()->getTerminator(); + return BE->getSrc()->getTerminatorStmt(); if (auto CE = P.getAs()) return CE->getCallExpr(); if (auto CEE = P.getAs())