r175938 - Remove the CFGElement "Invalid" state.

Ted Kremenek kremenek at apple.com
Sat Feb 23 10:39:14 PST 2013


Fantastic!  Thank you!

On Feb 22, 2013, at 4:29 PM, David Blaikie <dblaikie at gmail.com> wrote:

> Author: dblaikie
> Date: Fri Feb 22 18:29:34 2013
> New Revision: 175938
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=175938&view=rev
> Log:
> Remove the CFGElement "Invalid" state.
> 
> Use Optional<CFG*> where invalid states were needed previously. In the one case
> where that's not possible (beginAutomaticObjDtorsInsert) just use a dummy
> CFGAutomaticObjDtor.
> 
> Thanks for the help from Jordan Rose & discussion/feedback from Ted Kremenek
> and Doug Gregor.
> 
> Post commit code review feedback on r175796 by Ted Kremenek.
> 
> Modified:
>    cfe/trunk/include/clang/Analysis/CFG.h
>    cfe/trunk/include/clang/Analysis/ProgramPoint.h
>    cfe/trunk/lib/Analysis/CFG.cpp
>    cfe/trunk/lib/Analysis/CFGStmtMap.cpp
>    cfe/trunk/lib/Analysis/LiveVariables.cpp
>    cfe/trunk/lib/Analysis/ReachableCode.cpp
>    cfe/trunk/lib/Analysis/ThreadSafety.cpp
>    cfe/trunk/lib/Analysis/UninitializedValues.cpp
>    cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
> 
> Modified: cfe/trunk/include/clang/Analysis/CFG.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CFG.h?rev=175938&r1=175937&r2=175938&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Analysis/CFG.h (original)
> +++ cfe/trunk/include/clang/Analysis/CFG.h Fri Feb 22 18:29:34 2013
> @@ -49,7 +49,6 @@ class CFGElement {
> public:
>   enum Kind {
>     // main kind
> -    Invalid,
>     Statement,
>     Initializer,
>     // dtor kind
> @@ -70,8 +69,8 @@ protected:
>     : Data1(const_cast<void*>(Ptr1), ((unsigned) kind) & 0x3),
>       Data2(const_cast<void*>(Ptr2), (((unsigned) kind) >> 2) & 0x3) {}
> 
> -public:
>   CFGElement() {}
> +public:
> 
>   /// \brief Convert to the specified CFGElement type, asserting that this
>   /// CFGElement is of the desired type.
> @@ -84,12 +83,12 @@ public:
>     return t;
>   }
> 
> -  /// \brief Convert to the specified CFGElement type, returning an invalid
> -  /// CFGElement if this CFGElement is not of the desired type.
> +  /// \brief Convert to the specified CFGElement type, returning None if this
> +  /// CFGElement is not of the desired type.
>   template<typename T>
> -  T getAs() const {
> +  Optional<T> getAs() const {
>     if (!T::isKind(*this))
> -      return T();
> +      return None;
>     T t;
>     CFGElement& e = t;
>     e = *this;
> @@ -102,10 +101,6 @@ public:
>     x |= Data1.getInt();
>     return (Kind) x;
>   }
> -
> -  bool isValid() const { return getKind() != Invalid; }
> -
> -  operator bool() const { return isValid(); }
> };
> 
> class CFGStmt : public CFGElement {
> @@ -574,7 +569,7 @@ public:
>   // the elements beginning at the last position in prepared space.
>   iterator beginAutomaticObjDtorsInsert(iterator I, size_t Cnt,
>       BumpVectorContext &C) {
> -    return iterator(Elements.insert(I.base(), Cnt, CFGElement(), C));
> +    return iterator(Elements.insert(I.base(), Cnt, CFGAutomaticObjDtor(0, 0), C));
>   }
>   iterator insertAutomaticObjDtor(iterator I, VarDecl *VD, Stmt *S) {
>     *I = CFGAutomaticObjDtor(VD, S);
> @@ -757,8 +752,8 @@ public:
>     for (const_iterator I=begin(), E=end(); I != E; ++I)
>       for (CFGBlock::const_iterator BI=(*I)->begin(), BE=(*I)->end();
>            BI != BE; ++BI) {
> -        if (CFGStmt stmt = BI->getAs<CFGStmt>())
> -          O(const_cast<Stmt*>(stmt.getStmt()));
> +        if (Optional<CFGStmt> stmt = BI->getAs<CFGStmt>())
> +          O(const_cast<Stmt*>(stmt->getStmt()));
>       }
>   }
> 
> 
> Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=175938&r1=175937&r2=175938&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
> +++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Fri Feb 22 18:29:34 2013
> @@ -202,9 +202,9 @@ public:
>     return reinterpret_cast<const CFGBlock*>(getData1());
>   }
> 
> -  const CFGElement getFirstElement() const {
> +  Optional<CFGElement> getFirstElement() const {
>     const CFGBlock *B = getBlock();
> -    return B->empty() ? CFGElement() : B->front();
> +    return B->empty() ? Optional<CFGElement>() : B->front();
>   }
> 
> private:
> 
> Modified: cfe/trunk/lib/Analysis/CFG.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=175938&r1=175937&r2=175938&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/CFG.cpp (original)
> +++ cfe/trunk/lib/Analysis/CFG.cpp Fri Feb 22 18:29:34 2013
> @@ -3332,7 +3332,6 @@ CFG* CFG::buildCFG(const Decl *D, Stmt *
> const CXXDestructorDecl *
> CFGImplicitDtor::getDestructorDecl(ASTContext &astContext) const {
>   switch (getKind()) {
> -    case CFGElement::Invalid:
>     case CFGElement::Statement:
>     case CFGElement::Initializer:
>       llvm_unreachable("getDestructorDecl should only be used with "
> @@ -3406,8 +3405,8 @@ static BlkExprMapTy* PopulateBlkExprMap(
> 
>   for (CFG::iterator I=cfg.begin(), E=cfg.end(); I != E; ++I)
>     for (CFGBlock::iterator BI=(*I)->begin(), EI=(*I)->end(); BI != EI; ++BI)
> -      if (CFGStmt S = BI->getAs<CFGStmt>())
> -        FindSubExprAssignments(S.getStmt(), SubExprAssignments);
> +      if (Optional<CFGStmt> S = BI->getAs<CFGStmt>())
> +        FindSubExprAssignments(S->getStmt(), SubExprAssignments);
> 
>   for (CFG::iterator I=cfg.begin(), E=cfg.end(); I != E; ++I) {
> 
> @@ -3415,10 +3414,10 @@ static BlkExprMapTy* PopulateBlkExprMap(
>     // block-level that are block-level expressions.
> 
>     for (CFGBlock::iterator BI=(*I)->begin(), EI=(*I)->end(); BI != EI; ++BI) {
> -      CFGStmt CS = BI->getAs<CFGStmt>();
> +      Optional<CFGStmt> CS = BI->getAs<CFGStmt>();
>       if (!CS)
>         continue;
> -      if (const Expr *Exp = dyn_cast<Expr>(CS.getStmt())) {
> +      if (const Expr *Exp = dyn_cast<Expr>(CS->getStmt())) {
>         assert((Exp->IgnoreParens() == Exp) && "No parens on block-level exps");
> 
>         if (const BinaryOperator* B = dyn_cast<BinaryOperator>(Exp)) {
> @@ -3531,8 +3530,8 @@ public:
>       unsigned j = 1;
>       for (CFGBlock::const_iterator BI = (*I)->begin(), BEnd = (*I)->end() ;
>            BI != BEnd; ++BI, ++j ) {        
> -        if (CFGStmt SE = BI->getAs<CFGStmt>()) {
> -          const Stmt *stmt= SE.getStmt();
> +        if (Optional<CFGStmt> SE = BI->getAs<CFGStmt>()) {
> +          const Stmt *stmt= SE->getStmt();
>           std::pair<unsigned, unsigned> P((*I)->getBlockID(), j);
>           StmtMap[stmt] = P;
> 
> @@ -3721,8 +3720,8 @@ public:
> 
> static void print_elem(raw_ostream &OS, StmtPrinterHelper* Helper,
>                        const CFGElement &E) {
> -  if (CFGStmt CS = E.getAs<CFGStmt>()) {
> -    const Stmt *S = CS.getStmt();
> +  if (Optional<CFGStmt> CS = E.getAs<CFGStmt>()) {
> +    const Stmt *S = CS->getStmt();
> 
>     if (Helper) {
> 
> @@ -3769,8 +3768,8 @@ static void print_elem(raw_ostream &OS,
>     if (isa<Expr>(S))
>       OS << '\n';
> 
> -  } else if (CFGInitializer IE = E.getAs<CFGInitializer>()) {
> -    const CXXCtorInitializer *I = IE.getInitializer();
> +  } else if (Optional<CFGInitializer> IE = E.getAs<CFGInitializer>()) {
> +    const CXXCtorInitializer *I = IE->getInitializer();
>     if (I->isBaseInitializer())
>       OS << I->getBaseClass()->getAsCXXRecordDecl()->getName();
>     else OS << I->getAnyMember()->getName();
> @@ -3784,8 +3783,9 @@ static void print_elem(raw_ostream &OS,
>       OS << " (Base initializer)\n";
>     else OS << " (Member initializer)\n";
> 
> -  } else if (CFGAutomaticObjDtor DE = E.getAs<CFGAutomaticObjDtor>()){
> -    const VarDecl *VD = DE.getVarDecl();
> +  } else if (Optional<CFGAutomaticObjDtor> DE =
> +                 E.getAs<CFGAutomaticObjDtor>()) {
> +    const VarDecl *VD = DE->getVarDecl();
>     Helper->handleDecl(VD, OS);
> 
>     const Type* T = VD->getType().getTypePtr();
> @@ -3796,20 +3796,20 @@ static void print_elem(raw_ostream &OS,
>     OS << ".~" << T->getAsCXXRecordDecl()->getName().str() << "()";
>     OS << " (Implicit destructor)\n";
> 
> -  } else if (CFGBaseDtor BE = E.getAs<CFGBaseDtor>()) {
> -    const CXXBaseSpecifier *BS = BE.getBaseSpecifier();
> +  } else if (Optional<CFGBaseDtor> BE = E.getAs<CFGBaseDtor>()) {
> +    const CXXBaseSpecifier *BS = BE->getBaseSpecifier();
>     OS << "~" << BS->getType()->getAsCXXRecordDecl()->getName() << "()";
>     OS << " (Base object destructor)\n";
> 
> -  } else if (CFGMemberDtor ME = E.getAs<CFGMemberDtor>()) {
> -    const FieldDecl *FD = ME.getFieldDecl();
> +  } else if (Optional<CFGMemberDtor> ME = E.getAs<CFGMemberDtor>()) {
> +    const FieldDecl *FD = ME->getFieldDecl();
>     const Type *T = FD->getType()->getBaseElementTypeUnsafe();
>     OS << "this->" << FD->getName();
>     OS << ".~" << T->getAsCXXRecordDecl()->getName() << "()";
>     OS << " (Member object destructor)\n";
> 
> -  } else if (CFGTemporaryDtor TE = E.getAs<CFGTemporaryDtor>()) {
> -    const CXXBindTemporaryExpr *BT = TE.getBindTemporaryExpr();
> +  } else if (Optional<CFGTemporaryDtor> TE = E.getAs<CFGTemporaryDtor>()) {
> +    const CXXBindTemporaryExpr *BT = TE->getBindTemporaryExpr();
>     OS << "~" << BT->getType()->getAsCXXRecordDecl()->getName() << "()";
>     OS << " (Temporary object destructor)\n";
>   }
> 
> Modified: cfe/trunk/lib/Analysis/CFGStmtMap.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFGStmtMap.cpp?rev=175938&r1=175937&r2=175938&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/CFGStmtMap.cpp (original)
> +++ cfe/trunk/lib/Analysis/CFGStmtMap.cpp Fri Feb 22 18:29:34 2013
> @@ -50,11 +50,11 @@ static void Accumulate(SMap &SM, CFGBloc
>   // First walk the block-level expressions.
>   for (CFGBlock::iterator I = B->begin(), E = B->end(); I != E; ++I) {
>     const CFGElement &CE = *I;
> -    CFGStmt CS = CE.getAs<CFGStmt>();
> +    Optional<CFGStmt> CS = CE.getAs<CFGStmt>();
>     if (!CS)
>       continue;
> 
> -    CFGBlock *&Entry = SM[CS.getStmt()];
> +    CFGBlock *&Entry = SM[CS->getStmt()];
>     // If 'Entry' is already initialized (e.g., a terminator was already),
>     // skip.
>     if (Entry)
> 
> Modified: cfe/trunk/lib/Analysis/LiveVariables.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/LiveVariables.cpp?rev=175938&r1=175937&r2=175938&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/LiveVariables.cpp (original)
> +++ cfe/trunk/lib/Analysis/LiveVariables.cpp Fri Feb 22 18:29:34 2013
> @@ -474,8 +474,9 @@ LiveVariablesImpl::runOnBlock(const CFGB
>        ei = block->rend(); it != ei; ++it) {
>     const CFGElement &elem = *it;
> 
> -    if (CFGAutomaticObjDtor Dtor = elem.getAs<CFGAutomaticObjDtor>()){
> -      val.liveDecls = DSetFact.add(val.liveDecls, Dtor.getVarDecl());
> +    if (Optional<CFGAutomaticObjDtor> Dtor =
> +            elem.getAs<CFGAutomaticObjDtor>()) {
> +      val.liveDecls = DSetFact.add(val.liveDecls, Dtor->getVarDecl());
>       continue;
>     }
> 
> @@ -534,8 +535,9 @@ LiveVariables::computeLiveness(AnalysisD
>     if (killAtAssign)
>       for (CFGBlock::const_iterator bi = block->begin(), be = block->end();
>            bi != be; ++bi) {
> -        if (CFGStmt cs = bi->getAs<CFGStmt>()) {
> -          if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(cs.getStmt())) {
> +        if (Optional<CFGStmt> cs = bi->getAs<CFGStmt>()) {
> +          if (const BinaryOperator *BO =
> +                  dyn_cast<BinaryOperator>(cs->getStmt())) {
>             if (BO->getOpcode() == BO_Assign) {
>               if (const DeclRefExpr *DR =
>                     dyn_cast<DeclRefExpr>(BO->getLHS()->IgnoreParens())) {
> 
> Modified: cfe/trunk/lib/Analysis/ReachableCode.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ReachableCode.cpp?rev=175938&r1=175937&r2=175938&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/ReachableCode.cpp (original)
> +++ cfe/trunk/lib/Analysis/ReachableCode.cpp Fri Feb 22 18:29:34 2013
> @@ -95,8 +95,8 @@ static bool isValidDeadStmt(const Stmt *
> 
> const Stmt *DeadCodeScan::findDeadCode(const clang::CFGBlock *Block) {
>   for (CFGBlock::const_iterator I = Block->begin(), E = Block->end(); I!=E; ++I)
> -    if (CFGStmt CS = I->getAs<CFGStmt>()) {
> -      const Stmt *S = CS.getStmt();
> +    if (Optional<CFGStmt> CS = I->getAs<CFGStmt>()) {
> +      const Stmt *S = CS->getStmt();
>       if (isValidDeadStmt(S))
>         return S;
>     }
> 
> Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=175938&r1=175937&r2=175938&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
> +++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Feb 22 18:29:34 2013
> @@ -1397,8 +1397,8 @@ static void findBlockLocations(CFG *CFGr
>       for (CFGBlock::const_reverse_iterator BI = CurrBlock->rbegin(),
>            BE = CurrBlock->rend(); BI != BE; ++BI) {
>         // FIXME: Handle other CFGElement kinds.
> -        if (CFGStmt CS = BI->getAs<CFGStmt>()) {
> -          CurrBlockInfo->ExitLoc = CS.getStmt()->getLocStart();
> +        if (Optional<CFGStmt> CS = BI->getAs<CFGStmt>()) {
> +          CurrBlockInfo->ExitLoc = CS->getStmt()->getLocStart();
>           break;
>         }
>       }
> @@ -1410,8 +1410,8 @@ static void findBlockLocations(CFG *CFGr
>       for (CFGBlock::const_iterator BI = CurrBlock->begin(),
>            BE = CurrBlock->end(); BI != BE; ++BI) {
>         // FIXME: Handle other CFGElement kinds.
> -        if (CFGStmt CS = BI->getAs<CFGStmt>()) {
> -          CurrBlockInfo->EntryLoc = CS.getStmt()->getLocStart();
> +        if (Optional<CFGStmt> CS = BI->getAs<CFGStmt>()) {
> +          CurrBlockInfo->EntryLoc = CS->getStmt()->getLocStart();
>           break;
>         }
>       }
> @@ -2234,8 +2234,8 @@ inline bool neverReturns(const CFGBlock*
>     return false;
> 
>   CFGElement Last = B->back();
> -  if (CFGStmt S = Last.getAs<CFGStmt>()) {
> -    if (isa<CXXThrowExpr>(S.getStmt()))
> +  if (Optional<CFGStmt> S = Last.getAs<CFGStmt>()) {
> +    if (isa<CXXThrowExpr>(S->getStmt()))
>       return true;
>   }
>   return false;
> 
> Modified: cfe/trunk/lib/Analysis/UninitializedValues.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/UninitializedValues.cpp?rev=175938&r1=175937&r2=175938&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/UninitializedValues.cpp (original)
> +++ cfe/trunk/lib/Analysis/UninitializedValues.cpp Fri Feb 22 18:29:34 2013
> @@ -746,9 +746,8 @@ static bool runOnBlock(const CFGBlock *b
>   TransferFunctions tf(vals, cfg, block, ac, classification, handler);
>   for (CFGBlock::const_iterator I = block->begin(), E = block->end(); 
>        I != E; ++I) {
> -    if (CFGStmt cs = I->getAs<CFGStmt>()) {
> -      tf.Visit(const_cast<Stmt*>(cs.getStmt()));
> -    }
> +    if (Optional<CFGStmt> cs = I->getAs<CFGStmt>())
> +      tf.Visit(const_cast<Stmt*>(cs->getStmt()));
>   }
>   return vals.updateValueVectorWithScratch(block);
> }
> 
> Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=175938&r1=175937&r2=175938&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
> +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Fri Feb 22 18:29:34 2013
> @@ -762,8 +762,8 @@ nam



More information about the cfe-commits mailing list