Revert "[analyzer; alternate edges] improve support for edges with PseudoObjectExprs."

Ted and I spent a long time discussing this today and found out that neither
the existing code nor the new code was doing what either of us thought it
was, which is never good. The good news is we found a much simpler way to
fix the motivating test case (an ObjCSubscriptExpr).

This reverts r182083, but pieces of it will come back in subsequent commits.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@182185 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/ParentMap.h b/include/clang/AST/ParentMap.h
index 9ec77530128aaa2bac4921221171577c80de9c5d..62eae02c15256bc64a38704091fe0a913f491a3d 100644 (file)
--- a/include/clang/AST/ParentMap.h
+++ b/include/clang/AST/ParentMap.h
@@ -20,9 +20,8 @@ class Expr;
 
 class ParentMap {
   void* Impl;
-  bool IsSemantic;
 public:
-  ParentMap(Stmt* ASTRoot, bool isSemantic = false);
+  ParentMap(Stmt* ASTRoot);
   ~ParentMap();
 
   /// \brief Adds and/or updates the parent/child-relations of the complete

diff --git a/include/clang/Analysis/AnalysisContext.h b/include/clang/Analysis/AnalysisContext.h
index 1e6d70591cb4d141a281ed053692473318a1b492..46d7d07e090720c3637467976a42dc3c75f56841 100644 (file)
--- a/include/clang/Analysis/AnalysisContext.h
+++ b/include/clang/Analysis/AnalysisContext.h
@@ -82,7 +82,6 @@ class AnalysisDeclContext {
 
   bool builtCFG, builtCompleteCFG;
   OwningPtr<ParentMap> PM;
-  OwningPtr<ParentMap> SemanticPM;
   OwningPtr<PseudoConstantAnalysis> PCA;
   OwningPtr<CFGReverseBlockReachabilityAnalysis> CFA;
 
@@ -166,9 +165,6 @@ public:
   bool isCFGBuilt() const { return builtCFG; }
 
   ParentMap &getParentMap();
-
-  ParentMap &getSemanticParentMap();
-
   PseudoConstantAnalysis *getPseudoConstantAnalysis();
 
   typedef const VarDecl * const * referenced_decls_iterator;
@@ -249,10 +245,6 @@ public:
     return getAnalysisDeclContext()->getParentMap();
   }
 
-  ParentMap &getSemanticParentMap() const {
-    return getAnalysisDeclContext()->getSemanticParentMap();
-  }
-
   const ImplicitParamDecl *getSelfDecl() const {
     return Ctx->getSelfDecl();
   }

diff --git a/lib/AST/ParentMap.cpp b/lib/AST/ParentMap.cpp
index 735926d81df0a3b09d0b53fd36db8b69a8c991b8..113592860b77eee8a6e66a4a44ce3c65228b6d9f 100644 (file)
--- a/lib/AST/ParentMap.cpp
+++ b/lib/AST/ParentMap.cpp
@@ -25,7 +25,7 @@ enum OpaqueValueMode {
   OV_Opaque
 };
 
-static void BuildParentMap(MapTy& M, Stmt* S, bool isSemantic,
+static void BuildParentMap(MapTy& M, Stmt* S,
                            OpaqueValueMode OVMode = OV_Transparent) {
 
   switch (S->getStmtClass()) {
@@ -33,17 +33,14 @@ static void BuildParentMap(MapTy& M, Stmt* S, bool isSemantic,
     assert(OVMode == OV_Transparent && "Should not appear alongside OVEs");
     PseudoObjectExpr *POE = cast<PseudoObjectExpr>(S);
 
-    if (!isSemantic) {
-      M[POE->getSyntacticForm()] = S;
-      BuildParentMap(M, POE->getSyntacticForm(), OV_Transparent);
-    }
+    M[POE->getSyntacticForm()] = S;
+    BuildParentMap(M, POE->getSyntacticForm(), OV_Transparent);
 
     for (PseudoObjectExpr::semantics_iterator I = POE->semantics_begin(),
                                               E = POE->semantics_end();
          I != E; ++I) {
       M[*I] = S;
-      BuildParentMap(M, *I, isSemantic,
-                     isSemantic ? OV_Transparent : OV_Opaque);
+      BuildParentMap(M, *I, OV_Opaque);
     }
     break;
   }
@@ -52,16 +49,16 @@ static void BuildParentMap(MapTy& M, Stmt* S, bool isSemantic,
     BinaryConditionalOperator *BCO = cast<BinaryConditionalOperator>(S);
 
     M[BCO->getCommon()] = S;
-    BuildParentMap(M, BCO->getCommon(), isSemantic, OV_Transparent);
+    BuildParentMap(M, BCO->getCommon(), OV_Transparent);
 
     M[BCO->getCond()] = S;
-    BuildParentMap(M, BCO->getCond(), isSemantic, OV_Opaque);
+    BuildParentMap(M, BCO->getCond(), OV_Opaque);
 
     M[BCO->getTrueExpr()] = S;
-    BuildParentMap(M, BCO->getTrueExpr(), isSemantic, OV_Opaque);
+    BuildParentMap(M, BCO->getTrueExpr(), OV_Opaque);
 
     M[BCO->getFalseExpr()] = S;
-    BuildParentMap(M, BCO->getFalseExpr(), isSemantic, OV_Transparent);
+    BuildParentMap(M, BCO->getFalseExpr(), OV_Transparent);
 
     break;
   }
@@ -69,25 +66,24 @@ static void BuildParentMap(MapTy& M, Stmt* S, bool isSemantic,
     if (OVMode == OV_Transparent) {
       OpaqueValueExpr *OVE = cast<OpaqueValueExpr>(S);
       M[OVE->getSourceExpr()] = S;
-      BuildParentMap(M, OVE->getSourceExpr(), isSemantic, OV_Transparent);
+      BuildParentMap(M, OVE->getSourceExpr(), OV_Transparent);
     }
     break;
   default:
     for (Stmt::child_range I = S->children(); I; ++I) {
       if (*I) {
         M[*I] = S;
-        BuildParentMap(M, *I, isSemantic, OVMode);
+        BuildParentMap(M, *I, OVMode);
       }
     }
     break;
   }
 }
 
-ParentMap::ParentMap(Stmt* S, bool isSemantic) : Impl(0),
-                                                 IsSemantic(isSemantic) {
+ParentMap::ParentMap(Stmt* S) : Impl(0) {
   if (S) {
     MapTy *M = new MapTy();
-    BuildParentMap(*M, S, isSemantic);
+    BuildParentMap(*M, S);
     Impl = M;
   }
 }
@@ -98,7 +94,7 @@ ParentMap::~ParentMap() {
 
 void ParentMap::addStmt(Stmt* S) {
   if (S) {
-    BuildParentMap(*(MapTy*) Impl, S, IsSemantic);
+    BuildParentMap(*(MapTy*) Impl, S);
   }
 }
 

diff --git a/lib/Analysis/AnalysisDeclContext.cpp b/lib/Analysis/AnalysisDeclContext.cpp
index 316892ea91528bcce72a1492963894c8b64ff6c4..5ff7842407a9a63a312b3c93af0e0eac0fef2a4c 100644 (file)
--- a/lib/Analysis/AnalysisDeclContext.cpp
+++ b/lib/Analysis/AnalysisDeclContext.cpp
@@ -212,34 +212,20 @@ void AnalysisDeclContext::dumpCFG(bool ShowColors) {
     getCFG()->dump(getASTContext().getLangOpts(), ShowColors);
 }
 
-static ParentMap *constructParentMap(bool isSemantic,
-                                     Stmt *Body,
-                                     const Decl *D) {
-  ParentMap *PM = new ParentMap(Body, isSemantic);
-  if (const CXXConstructorDecl *C = dyn_cast<CXXConstructorDecl>(D)) {
-    for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
-         E = C->init_end();
-         I != E; ++I) {
-      PM->addStmt((*I)->getInit());
-    }
-  }
-  return PM;
-}
-
 ParentMap &AnalysisDeclContext::getParentMap() {
   if (!PM) {
-    PM.reset(constructParentMap(false, getBody(), getDecl()));
+    PM.reset(new ParentMap(getBody()));
+    if (const CXXConstructorDecl *C = dyn_cast<CXXConstructorDecl>(getDecl())) {
+      for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
+                                                   E = C->init_end();
+           I != E; ++I) {
+        PM->addStmt((*I)->getInit());
+      }
+    }
   }
   return *PM;
 }
 
-ParentMap &AnalysisDeclContext::getSemanticParentMap() {
-  if (!SemanticPM) {
-    SemanticPM.reset(constructParentMap(true, getBody(), getDecl()));
-  }
-  return *SemanticPM;
-}
-
 PseudoConstantAnalysis *AnalysisDeclContext::getPseudoConstantAnalysis() {
   if (!PCA)
     PCA.reset(new PseudoConstantAnalysis(getBody()));

diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp
index ab3464507de6ea85821415fe2f34b237fe407191..519555396dd475e3e61b35e54c6acac6f753ae1a 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1873,78 +1873,9 @@ static bool isIncrementOrInitInForLoop(const Stmt *S, const Stmt *FL) {
 typedef llvm::DenseSet<const PathDiagnosticCallPiece *>
         OptimizedCallsSet;
 
-typedef llvm::DenseMap<const Stmt *,
-                       Optional<const PseudoObjectExpr *> >
-        PseudoObjectExprMap;
-
-/// Return the PseudoObjectExpr that contains this statement (if any).
-static const PseudoObjectExpr *
-getContainingPseudoObjectExpr(PseudoObjectExprMap &PEM,
-                              ParentMap &PM,
-                              const Stmt *S) {
-  if (!S)
-    return 0;
-
-  Optional<const PseudoObjectExpr *> &Entry = PEM[S];
-  if (!Entry.hasValue()) {
-    const Stmt *Parent = PM.getParentIgnoreParens(S);
-    if (const PseudoObjectExpr *PE = dyn_cast_or_null<PseudoObjectExpr>(Parent))
-      Entry = PE;
-    else
-      Entry = getContainingPseudoObjectExpr(PEM, PM, Parent);
-  }
-  return Entry.getValue();
-}
-
-#if 0
-static void printPath(PathPieces &path, ParentMap &PM) {
-  unsigned index = 0;
-  for (PathPieces::iterator I = path.begin(), E = path.end(); I != E; ++I ) {
-    llvm::errs() << "[" << index++ << "]\n";
-    if (isa<PathDiagnosticCallPiece>(*I)) {
-      llvm::errs() << "  CALL\n";
-      continue;
-    }
-    if (isa<PathDiagnosticEventPiece>(*I)) {
-      llvm::errs() << "  EVENT\n";
-      continue;
-    }
-    if (const PathDiagnosticControlFlowPiece *CP = dyn_cast<PathDiagnosticControlFlowPiece>(*I)) {
-      llvm::errs() << "  CONTROL\n";
-      const Stmt *s1Start = getLocStmt(CP->getStartLocation());
-      const Stmt *s1End   = getLocStmt(CP->getEndLocation());
-      if (s1Start) {
-        s1Start->dump();
-        llvm::errs() << "PARENT: \n";
-        const Stmt *Parent = getStmtParent(s1Start, PM);
-        if (Parent) {
-          Parent->dump();
-        }
-      }
-      else {
-        llvm::errs() << "NULL\n";
-      }
-      llvm::errs() << " --------- ===== ----- \n";
-      if (s1End) {
-        s1End->dump();
-        llvm::errs() << "PARENT: \n";
-        const Stmt *Parent = getStmtParent(s1End, PM);
-        if (Parent) {
-          Parent->dump();
-        }
-      }
-      else {
-        llvm::errs() << "NULL\n";
-      }
-    }
-  }
-}
-#endif
-
 static bool optimizeEdges(PathPieces &path, SourceManager &SM,
                           OptimizedCallsSet &OCS,
-                          LocationContextMap &LCM,
-                          PseudoObjectExprMap &PEM) {
+                          LocationContextMap &LCM) {
   bool hasChanges = false;
   const LocationContext *LC = LCM[&path];
   assert(LC);
@@ -1959,7 +1890,7 @@ static bool optimizeEdges(PathPieces &path, SourceManager &SM,
       // Record the fact that a call has been optimized so we only do the
       // effort once.
       if (!OCS.count(CallI)) {
-        while (optimizeEdges(CallI->path, SM, OCS, LCM, PEM)) {}
+        while (optimizeEdges(CallI->path, SM, OCS, LCM)) {}
         OCS.insert(CallI);
       }
       ++I;
@@ -1975,7 +1906,7 @@ static bool optimizeEdges(PathPieces &path, SourceManager &SM,
       continue;
     }
 
-    ParentMap &PM = LC->getSemanticParentMap();
+    ParentMap &PM = LC->getParentMap();
     const Stmt *s1Start = getLocStmt(PieceI->getStartLocation());
     const Stmt *s1End   = getLocStmt(PieceI->getEndLocation());
     const Stmt *level1 = getStmtParent(s1Start, PM);
@@ -2002,39 +1933,6 @@ static bool optimizeEdges(PathPieces &path, SourceManager &SM,
       }
     }
 
-    // Prune out edges for pseudo object expressions.
-    //
-    // Case 1: incoming into a pseudo expr.
-    //
-    // An edge into a subexpression of a pseudo object expression
-    // should be replaced with an edge to the pseudo object expression
-    // itself.
-    const PseudoObjectExpr *PE = getContainingPseudoObjectExpr(PEM, PM, s1End);
-    if (PE) {
-      PathDiagnosticLocation L(PE, SM, LC);
-      PieceI->setEndLocation(L);
-      // Do not increment the iterator.  It is possible we will match again.
-      hasChanges = true;
-      continue;
-    }
-
-    // Prune out edges for pseudo object expressions.
-    //
-    // Case 2: outgoing from a pseudo expr.
-    //
-    // An edge into a subexpression of a pseudo object expression
-    // should be replaced with an edge to the pseudo object expression
-    // itself.
-    PE = getContainingPseudoObjectExpr(PEM, PM, s1Start);
-    if (PE) {
-      PathDiagnosticLocation L(PE, SM, LC);
-      PieceI->setStartLocation(L);
-      // Do not increment the iterator.  It is possible we will match again.
-      hasChanges = true;
-      continue;
-    }
-
-    // Pattern match on two edges after this point.
     PathPieces::iterator NextI = I; ++NextI;
     if (NextI == E)
       break;
@@ -2884,8 +2782,7 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD,
         // to an aesthetically pleasing subset that conveys the
         // necessary information.
         OptimizedCallsSet OCS;
-        PseudoObjectExprMap PEM;
-        while (optimizeEdges(PD.getMutablePieces(), SM, OCS, LCM, PEM)) {}
+        while (optimizeEdges(PD.getMutablePieces(), SM, OCS, LCM)) {}
 
         // Adjust edges into loop conditions to make them more uniform
         // and aesthetically pleasing.