[analyzer; new edges] Simplify edges in a C++11 for-range loop.

Previously our edges were completely broken here; now, the final result
is a very simple set of edges in most cases: one up to the "for" keyword
for context, and one into the body of the loop. This matches the behavior
for ObjC for-in loops.

In the AST, however, CXXForRangeStmts are handled very differently from
ObjCForCollectionStmts. Since they are specified in terms of equivalent
statements in the C++ standard, we actually have implicit AST nodes for
all of the semantic statements. This makes evaluation very easy, but
diagnostic locations a bit trickier. Fortunately, the problem can be
generally defined away by marking all of the implicit statements as
part of the top-level for-range statement.

One of the implicit statements in a for-range statement is the declaration
of implicit iterators __begin and __end. The CFG synthesizes two
separate DeclStmts to match each of these decls, but until now these
synthetic DeclStmts weren't in the function's ParentMap. Now, the CFG
keeps track of its synthetic statements, and the AnalysisDeclContext will
make sure to add them to the ParentMap.

<rdar://problem/14038483>

llvm-svn: 183449

clang/include/clang/AST/ParentMap.h

@@ -29,6 +29,11 @@ public:
   /// visited and updated or inserted but not the parents of S.
   void addStmt(Stmt* S);
 
+  /// Manually sets the parent of \p S to \p Parent.
+  ///
+  /// If \p S is already in the map, this method will update the mapping.
+  void setParent(const Stmt *S, const Stmt *Parent);
+
   Stmt *getParent(Stmt*) const;
   Stmt *getParentIgnoreParens(Stmt *) const;
   Stmt *getParentIgnoreParenCasts(Stmt *) const;

clang/include/clang/Analysis/CFG.h

@@ -745,6 +745,35 @@ public:
     TryDispatchBlocks.push_back(block);
   }
 
+  /// Records a synthetic DeclStmt and the DeclStmt it was constructed from.
+  ///
+  /// The CFG uses synthetic DeclStmts when a single AST DeclStmt contains
+  /// multiple decls.
+  void addSyntheticDeclStmt(const DeclStmt *Synthetic,
+                            const DeclStmt *Source) {
+    assert(Synthetic->isSingleDecl() && "Can handle single declarations only");
+    assert(Synthetic != Source && "Don't include original DeclStmts in map");
+    assert(!SyntheticDeclStmts.count(Synthetic) && "Already in map");
+    SyntheticDeclStmts[Synthetic] = Source;
+  }
+
+  typedef llvm::DenseMap<const DeclStmt *, const DeclStmt *>::const_iterator
+    synthetic_stmt_iterator;
+
+  /// Iterates over synthetic DeclStmts in the CFG.
+  ///
+  /// Each element is a (synthetic statement, source statement) pair.
+  ///
+  /// \sa addSyntheticDeclStmt
+  synthetic_stmt_iterator synthetic_stmt_begin() const {
+    return SyntheticDeclStmts.begin();
+  }
+
+  /// \sa synthetic_stmt_begin
+  synthetic_stmt_iterator synthetic_stmt_end() const {
+    return SyntheticDeclStmts.end();
+  }
+
   //===--------------------------------------------------------------------===//
   // Member templates useful for various batch operations over CFGs.
   //===--------------------------------------------------------------------===//
@@ -810,6 +839,9 @@ private:
   /// This is the collection of such blocks present in the CFG.
   std::vector<const CFGBlock *> TryDispatchBlocks;
 
+  /// Collects DeclStmts synthesized for this CFG and maps each one back to its
+  /// source DeclStmt.
+  llvm::DenseMap<const DeclStmt *, const DeclStmt *> SyntheticDeclStmts;
 };
 } // end namespace clang
 

clang/lib/AST/ParentMap.cpp

@@ -110,6 +110,13 @@ void ParentMap::addStmt(Stmt* S) {
   }
 }
 
+void ParentMap::setParent(const Stmt *S, const Stmt *Parent) {
+  assert(S);
+  assert(Parent);
+  MapTy *M = reinterpret_cast<MapTy *>(Impl);
+  M->insert(std::make_pair(const_cast<Stmt *>(S), const_cast<Stmt *>(Parent)));
+}
+
 Stmt* ParentMap::getParent(Stmt* S) const {
   MapTy* M = (MapTy*) Impl;
   MapTy::iterator I = M->find(S);

clang/lib/Analysis/AnalysisDeclContext.cpp

@@ -157,6 +157,19 @@ AnalysisDeclContext::getBlockForRegisteredExpression(const Stmt *stmt) {
   return itr->second;
 }
 
+/// Add each synthetic statement in the CFG to the parent map, using the
+/// source statement's parent.
+static void addParentsForSyntheticStmts(const CFG *TheCFG, ParentMap &PM) {
+  if (!TheCFG)
+    return;
+
+  for (CFG::synthetic_stmt_iterator I = TheCFG->synthetic_stmt_begin(),
+                                    E = TheCFG->synthetic_stmt_end();
+       I != E; ++I) {
+    PM.setParent(I->first, PM.getParent(I->second));
+  }
+}
+
 CFG *AnalysisDeclContext::getCFG() {
   if (!cfgBuildOptions.PruneTriviallyFalseEdges)
     return getUnoptimizedCFG();
@@ -167,6 +180,9 @@ CFG *AnalysisDeclContext::getCFG() {
     // Even when the cfg is not successfully built, we don't
     // want to try building it again.
     builtCFG = true;
+
+    if (PM)
+      addParentsForSyntheticStmts(cfg.get(), *PM);
   }
   return cfg.get();
 }
@@ -180,6 +196,9 @@ CFG *AnalysisDeclContext::getUnoptimizedCFG() {
     // Even when the cfg is not successfully built, we don't
     // want to try building it again.
     builtCompleteCFG = true;
+
+    if (PM)
+      addParentsForSyntheticStmts(completeCFG.get(), *PM);
   }
   return completeCFG.get();
 }
@@ -222,6 +241,10 @@ ParentMap &AnalysisDeclContext::getParentMap() {
         PM->addStmt((*I)->getInit());
       }
     }
+    if (builtCFG)
+      addParentsForSyntheticStmts(getCFG(), *PM);
+    if (builtCompleteCFG)
+      addParentsForSyntheticStmts(getUnoptimizedCFG(), *PM);
   }
   return *PM;
 }

clang/lib/Analysis/CFG.cpp

@@ -1627,6 +1627,7 @@ CFGBlock *CFGBuilder::VisitDeclStmt(DeclStmt *DS) {
     Decl *D = *I;
     void *Mem = cfg->getAllocator().Allocate(sizeof(DeclStmt), A);
     DeclStmt *DSNew = new (Mem) DeclStmt(DG, D->getLocation(), GetEndLoc(D));
+    cfg->addSyntheticDeclStmt(DSNew, DS);
 
     // Append the fake DeclStmt to block.
     B = VisitDeclSubExpr(DSNew);
@@ -3953,6 +3954,10 @@ Stmt *CFGBlock::getTerminatorCondition() {
     default:
       break;
 
+    case Stmt::CXXForRangeStmtClass:
+      E = cast<CXXForRangeStmt>(Terminator)->getCond();
+      break;
+
     case Stmt::ForStmtClass:
       E = cast<ForStmt>(Terminator)->getCond();
       break;

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

@@ -363,6 +363,7 @@ static const Stmt *getEnclosingParent(const Stmt *S, const ParentMap &PM) {
   case Stmt::DoStmtClass:
   case Stmt::WhileStmtClass:
   case Stmt::ObjCForCollectionStmtClass:
+  case Stmt::CXXForRangeStmtClass:
     return Parent;
   default:
     break;
@@ -404,6 +405,10 @@ getEnclosingStmtLocation(const Stmt *S, SourceManager &SMgr, const ParentMap &P,
           return PathDiagnosticLocation(Parent, SMgr, LC);
         else
           return PathDiagnosticLocation(S, SMgr, LC);
+      case Stmt::CXXForRangeStmtClass:
+        if (cast<CXXForRangeStmt>(Parent)->getBody() == S)
+          return PathDiagnosticLocation(S, SMgr, LC);
+        break;
       case Stmt::DoStmtClass:
           return PathDiagnosticLocation(S, SMgr, LC);
       case Stmt::ForStmtClass:
@@ -1253,6 +1258,7 @@ static bool isLoop(const Stmt *Term) {
     case Stmt::ForStmtClass:
     case Stmt::WhileStmtClass:
     case Stmt::ObjCForCollectionStmtClass:
+    case Stmt::CXXForRangeStmtClass:
       return true;
     default:
       // Note that we intentionally do not include do..while here.
@@ -1302,6 +1308,15 @@ static const Stmt *getStmtBeforeCond(ParentMap &PM, const Stmt *Term,
 static bool isInLoopBody(ParentMap &PM, const Stmt *S, const Stmt *Term) {
   const Stmt *LoopBody = 0;
   switch (Term->getStmtClass()) {
+    case Stmt::CXXForRangeStmtClass: {
+      const CXXForRangeStmt *FR = cast<CXXForRangeStmt>(Term);
+      if (isContainedByStmt(PM, FR->getInc(), S))
+        return true;
+      if (isContainedByStmt(PM, FR->getLoopVarStmt(), S))
+        return true;
+      LoopBody = FR->getBody();
+      break;
+    }
     case Stmt::ForStmtClass: {
       const ForStmt *FS = cast<ForStmt>(Term);
       if (isContainedByStmt(PM, FS->getInc(), S))
@@ -1718,16 +1733,20 @@ GenerateAlternateExtensivePathDiagnostic(PathDiagnostic& PD,
         // Are we jumping to the head of a loop?  Add a special diagnostic.
         if (const Stmt *Loop = BE->getSrc()->getLoopTarget()) {
           PathDiagnosticLocation L(Loop, SM, PDB.LC);
-          const CompoundStmt *CS = NULL;
+          const Stmt *Body = NULL;
 
           if (const ForStmt *FS = dyn_cast<ForStmt>(Loop))
-            CS = dyn_cast<CompoundStmt>(FS->getBody());
+            Body = FS->getBody();
           else if (const WhileStmt *WS = dyn_cast<WhileStmt>(Loop))
-            CS = dyn_cast<CompoundStmt>(WS->getBody());
+            Body = WS->getBody();
           else if (const ObjCForCollectionStmt *OFS =
-                   dyn_cast<ObjCForCollectionStmt>(Loop)) {
-            CS = dyn_cast<CompoundStmt>(OFS->getBody());
+                     dyn_cast<ObjCForCollectionStmt>(Loop)) {
+            Body = OFS->getBody();
+          } else if (const CXXForRangeStmt *FRS =
+                       dyn_cast<CXXForRangeStmt>(Loop)) {
+            Body = FRS->getBody();
           }
+          // do-while statements are explicitly excluded here
 
           PathDiagnosticEventPiece *p =
             new PathDiagnosticEventPiece(L, "Looping back to the head "
@@ -1737,7 +1756,7 @@ GenerateAlternateExtensivePathDiagnostic(PathDiagnostic& PD,
           addEdgeToPath(PD.getActivePath(), PrevLoc, p->getLocation(), PDB.LC);
           PD.getActivePath().push_front(p);
 
-          if (CS) {
+          if (const CompoundStmt *CS = dyn_cast_or_null<CompoundStmt>(Body)) {
             addEdgeToPath(PD.getActivePath(), PrevLoc,
                           PathDiagnosticLocation::createEndBrace(CS, SM),
                           PDB.LC);
@@ -1871,16 +1890,22 @@ static bool isConditionForTerminator(const Stmt *S, const Stmt *Cond) {
     }
     case Stmt::ObjCForCollectionStmtClass:
       return cast<ObjCForCollectionStmt>(S)->getElement() == Cond;
+    case Stmt::CXXForRangeStmtClass: {
+      const CXXForRangeStmt *FRS = cast<CXXForRangeStmt>(S);
+      return FRS->getCond() == Cond || FRS->getRangeInit() == Cond;
+    }
     default:
       return false;
   }
 }
 
 static bool isIncrementOrInitInForLoop(const Stmt *S, const Stmt *FL) {
-  const ForStmt *FS = dyn_cast<ForStmt>(FL);
-  if (!FS)
-    return false;
-  return FS->getInc() == S || FS->getInit() == S;
+  if (const ForStmt *FS = dyn_cast<ForStmt>(FL))
+    return FS->getInc() == S || FS->getInit() == S;
+  if (const CXXForRangeStmt *FRS = dyn_cast<CXXForRangeStmt>(FL))
+    return FRS->getInc() == S || FRS->getRangeStmt() == S ||
+           FRS->getLoopVarStmt() || FRS->getRangeInit() == S;
+  return false;
 }
 
 typedef llvm::DenseSet<const PathDiagnosticCallPiece *>
@@ -1903,16 +1928,15 @@ static void addContextEdges(PathPieces &pieces, SourceManager &SM,
       continue;
 
     PathDiagnosticLocation SrcLoc = Piece->getStartLocation();
-    const Stmt *Src = getLocStmt(SrcLoc);
     SmallVector<PathDiagnosticLocation, 4> SrcContexts;
 
-    PathDiagnosticLocation NextSrcContext =
-      getEnclosingStmtLocation(Src, SM, PM, LCtx, /*allowNested=*/true);
-    const Stmt *InnerStmt = Src;
+    PathDiagnosticLocation NextSrcContext = SrcLoc;
+    const Stmt *InnerStmt = 0;
     while (NextSrcContext.isValid() && NextSrcContext.asStmt() != InnerStmt) {
       SrcContexts.push_back(NextSrcContext);
       InnerStmt = NextSrcContext.asStmt();
-      NextSrcContext = getEnclosingStmtLocation(InnerStmt, SM, PM, LCtx, true);
+      NextSrcContext = getEnclosingStmtLocation(InnerStmt, SM, PM, LCtx,
+                                                /*allowNested=*/true);
     }
 
     // Repeatedly split the edge as necessary.
@@ -2024,7 +2048,8 @@ static void simplifySimpleBranches(PathPieces &pieces) {
     // We only perform this transformation for specific branch kinds.
     // We don't want to do this for do..while, for example.
     if (!(isa<ForStmt>(s1Start) || isa<WhileStmt>(s1Start) ||
-          isa<IfStmt>(s1Start) || isa<ObjCForCollectionStmt>(s1Start)))
+          isa<IfStmt>(s1Start) || isa<ObjCForCollectionStmt>(s1Start) ||
+          isa<CXXForRangeStmt>(s1Start)))
       continue;
 
     // Is s1End the branch condition?