ParentMap: Restore the ability to update an existing map.

The Clang ASTs are a DAG, not a pure tree. However, ParentMap has to
choose a single parent for each object. In the main (only?) cases in
which the AST forms a DAG, it protects from multiple traversal by using
OpaqueValueExprs. Previously, ParentMap would just unconditionally look
through all OpaqueValueExprs when building its map.

In order to make this behavior better for the analyzer's diagnostics,
ParentMap was changed to not set a statement's parent if there already
was one in the map. However, ParentMap is supposed to allow updating
existing mappings by calling addStmt once again. This change makes the
"transparency" of OpaqueValueExprs explicit, and disables it when it
is not desired, rather than checking the current contents of the map.

This new code seems like a big change, but it should actually have
essentially the same performance as before. Only OpaqueValueExprs and
their users (PseudoObjectExpr and BinaryConditionalOperator) will
have any different behavior.

There should be no user-visible functionality change, though a test
has been added for the current behavior of BinaryConditionalOperator
source locations and accompanying Xcode arrows (which are not so great...).

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

1 files changed, 55 insertions, 14 deletions

diff --git a/lib/AST/ParentMap.cpp b/lib/AST/ParentMap.cpp
index fa87afd0fa..113592860b 100644
--- a/lib/AST/ParentMap.cpp
+++ b/lib/AST/ParentMap.cpp
@@ -20,22 +20,63 @@ using namespace clang;
 
 typedef llvm::DenseMap<Stmt*, Stmt*> MapTy;
 
-static void BuildParentMap(MapTy& M, Stmt* S) {
-  for (Stmt::child_range I = S->children(); I; ++I)
-    if (*I) {
-      // Prefer the first time we see this statement in the traversal.
-      // This is important for PseudoObjectExprs.
-      Stmt *&Parent = M[*I];
-      if (!Parent) {
-        Parent = S;
-        BuildParentMap(M, *I);
+enum OpaqueValueMode {
+  OV_Transparent,
+  OV_Opaque
+};
+
+static void BuildParentMap(MapTy& M, Stmt* S,
+                           OpaqueValueMode OVMode = OV_Transparent) {
+
+  switch (S->getStmtClass()) {
+  case Stmt::PseudoObjectExprClass: {
+    assert(OVMode == OV_Transparent && "Should not appear alongside OVEs");
+    PseudoObjectExpr *POE = cast<PseudoObjectExpr>(S);
+
+    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, OV_Opaque);
+    }
+    break;
+  }
+  case Stmt::BinaryConditionalOperatorClass: {
+    assert(OVMode == OV_Transparent && "Should not appear alongside OVEs");
+    BinaryConditionalOperator *BCO = cast<BinaryConditionalOperator>(S);
+
+    M[BCO->getCommon()] = S;
+    BuildParentMap(M, BCO->getCommon(), OV_Transparent);
+
+    M[BCO->getCond()] = S;
+    BuildParentMap(M, BCO->getCond(), OV_Opaque);
+
+    M[BCO->getTrueExpr()] = S;
+    BuildParentMap(M, BCO->getTrueExpr(), OV_Opaque);
+
+    M[BCO->getFalseExpr()] = S;
+    BuildParentMap(M, BCO->getFalseExpr(), OV_Transparent);
+
+    break;
+  }
+  case Stmt::OpaqueValueExprClass:
+    if (OVMode == OV_Transparent) {
+      OpaqueValueExpr *OVE = cast<OpaqueValueExpr>(S);
+      M[OVE->getSourceExpr()] = S;
+      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, OVMode);
       }
     }
-  
-  // Also include the source expr tree of an OpaqueValueExpr in the map.
-  if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(S)) {
-    M[OVE->getSourceExpr()] = S;
-    BuildParentMap(M, OVE->getSourceExpr());
+    break;
   }
 }