From: Jordan Rose <jordan_rose@apple.com>
Date: Sat, 18 May 2013 02:27:09 +0000 (+0000)
Subject: [analyzer] "Fix" ParentMap to handle non-syntactic OpaqueValueExprs.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b9fdfb50983012b3460e3c27737ec8cdfdb8627d;p=clang

[analyzer] "Fix" ParentMap to handle non-syntactic OpaqueValueExprs.

Constructs like PseudoObjectExpr, where an expression can appear more than
once in the AST, use OpaqueValueExprs to guard against inadvertent
re-processing of the shared expression during AST traversal. The most
common form of this is to share expressions between the syntactic
"as-written" form of, say, an Objective-C property access 'obj.prop', and
the underlying "semantic" form '[obj prop]'.

However, some constructs can produce OpaqueValueExprs that don't appear in
the syntactic form at all; in these cases the ParentMap wasn't ever traversing
the children of these expressions. This patch fixes that by checking to see
if an OpaqueValueExpr's child has ever been traversed before. There's also a
bit of reset logic when visiting a PseudoObjectExpr to handle the case of
updating the ParentMap, which some external clients depend on.

This still isn't exactly the right fix because we probably want the parent
of the OpaqueValueExpr itself to be its location in the syntactic form if
it's syntactic and the PseudoObjectExpr or BinaryConditionalOperator itself
if it's semantic. Whe I originally wrote the code to do this, I didn't realize
that OpaqueValueExprs themselves are shared in the AST, not just their source
expressions. This patch doesn't change the existing behavior so as not to
break anything inadvertently relying on it; we'll come back to this later.

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

diff --git a/lib/AST/ParentMap.cpp b/lib/AST/ParentMap.cpp
index 113592860b..ed5ac77db0 100644
--- a/lib/AST/ParentMap.cpp
+++ b/lib/AST/ParentMap.cpp
@@ -33,6 +33,11 @@ static void BuildParentMap(MapTy& M, Stmt* S,
     assert(OVMode == OV_Transparent && "Should not appear alongside OVEs");
     PseudoObjectExpr *POE = cast<PseudoObjectExpr>(S);
 
+    // If we are rebuilding the map, clear out any existing state.
+    if (M[POE->getSyntacticForm()])
+      for (Stmt::child_range I = S->children(); I; ++I)
+        M[*I] = 0;
+
     M[POE->getSyntacticForm()] = S;
     BuildParentMap(M, POE->getSyntacticForm(), OV_Transparent);
 
@@ -62,13 +67,19 @@ static void BuildParentMap(MapTy& M, Stmt* S,
 
     break;
   }
-  case Stmt::OpaqueValueExprClass:
-    if (OVMode == OV_Transparent) {
-      OpaqueValueExpr *OVE = cast<OpaqueValueExpr>(S);
+  case Stmt::OpaqueValueExprClass: {
+    // FIXME: This isn't correct; it assumes that multiple OpaqueValueExprs
+    // share a single source expression, but in the AST a single
+    // OpaqueValueExpr is shared among multiple parent expressions.
+    // The right thing to do is to give the OpaqueValueExpr its syntactic
+    // parent, then not reassign that when traversing the semantic expressions.
+    OpaqueValueExpr *OVE = cast<OpaqueValueExpr>(S);
+    if (OVMode == OV_Transparent || !M[OVE->getSourceExpr()]) {
       M[OVE->getSourceExpr()] = S;
       BuildParentMap(M, OVE->getSourceExpr(), OV_Transparent);
     }
     break;
+  }
   default:
     for (Stmt::child_range I = S->children(); I; ++I) {
       if (*I) {