From 33806cdefcffb091515bc329158d8c05714aa01a Mon Sep 17 00:00:00 2001
From: Daniel Jasper <djasper@google.com>
Date: Sun, 11 Nov 2012 22:14:55 +0000
Subject: [PATCH] Fix binding of nodes in case of forEach..() matchers.

When recursively visiting the generated matches, the aggregated bindings need
to be copied during the recursion. Otherwise, we they might not be properly
overwritten (which is shown by the test), or there might be bound nodes present
that were bound on a different matching branch.

Review: http://llvm-reviews.chandlerc.com/D112
llvm-svn: 167695
---
 clang/include/clang/ASTMatchers/ASTMatchersInternal.h |  2 +-
 clang/lib/ASTMatchers/ASTMatchersInternal.cpp         | 11 ++++++-----
 clang/unittests/ASTMatchers/ASTMatchersTest.cpp       | 11 +++++++++++
 clang/unittests/ASTMatchers/ASTMatchersTest.h         |  2 +-
 4 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
index 7bcf90f..e5365ff 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -141,7 +141,7 @@ public:
 private:
   void visitMatchesRecursively(
       Visitor* ResultVistior,
-      BoundNodesMap *AggregatedBindings);
+      const BoundNodesMap& AggregatedBindings);
 
   // FIXME: Find out whether we want to use different data structures here -
   // first benchmarks indicate that it doesn't matter though.
diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index e7ee8ee..408195d 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -51,19 +51,20 @@ void BoundNodesTree::copyTo(BoundNodesTreeBuilder* Builder) const {
 
 void BoundNodesTree::visitMatches(Visitor* ResultVisitor) {
   BoundNodesMap AggregatedBindings;
-  visitMatchesRecursively(ResultVisitor, &AggregatedBindings);
+  visitMatchesRecursively(ResultVisitor, AggregatedBindings);
 }
 
 void BoundNodesTree::
 visitMatchesRecursively(Visitor* ResultVisitor,
-                        BoundNodesMap* AggregatedBindings) {
-  Bindings.copyTo(AggregatedBindings);
+                        const BoundNodesMap& AggregatedBindings) {
+  BoundNodesMap CombinedBindings(AggregatedBindings);
+  Bindings.copyTo(&CombinedBindings);
   if (RecursiveBindings.empty()) {
-    ResultVisitor->visitMatch(BoundNodes(*AggregatedBindings));
+    ResultVisitor->visitMatch(BoundNodes(CombinedBindings));
   } else {
     for (unsigned I = 0; I < RecursiveBindings.size(); ++I) {
       RecursiveBindings[I].visitMatchesRecursively(ResultVisitor,
-                                                   AggregatedBindings);
+                                                   CombinedBindings);
     }
   }
 }
diff --git a/clang/unittests/ASTMatchers/ASTMatchersTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersTest.cpp
index ad54693..e15940a 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -2775,6 +2775,17 @@ TEST(ForEachDescendant, BindsRecursiveCombinations) {
       new VerifyIdIsBoundTo<FieldDecl>("f", 8)));
 }
 
+TEST(ForEachDescendant, BindsCorrectNodes) {
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+      "class C { void f(); int i; };",
+      recordDecl(hasName("C"), forEachDescendant(decl().bind("decl"))),
+      new VerifyIdIsBoundTo<FieldDecl>("decl", 1)));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+      "class C { void f() {} int i; };",
+      recordDecl(hasName("C"), forEachDescendant(decl().bind("decl"))),
+      new VerifyIdIsBoundTo<FunctionDecl>("decl", 1)));
+}
+
 
 TEST(IsTemplateInstantiation, MatchesImplicitClassTemplateInstantiation) {
   // Make sure that we can both match the class by name (::X) and by the type
diff --git a/clang/unittests/ASTMatchers/ASTMatchersTest.h b/clang/unittests/ASTMatchers/ASTMatchersTest.h
index 5e63b6b..3b23ada 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTest.h
+++ b/clang/unittests/ASTMatchers/ASTMatchersTest.h
@@ -38,7 +38,7 @@ public:
 
   virtual void run(const MatchFinder::MatchResult &Result) {
     if (FindResultReviewer != NULL) {
-      *Verified = FindResultReviewer->run(&Result.Nodes, Result.Context);
+      *Verified |= FindResultReviewer->run(&Result.Nodes, Result.Context);
     } else {
       *Verified = true;
     }
-- 
2.7.4