[cfe-commits] r167695 - in /cfe/trunk: include/clang/ASTMatchers/ASTMatchersInternal.h lib/ASTMatchers/ASTMatchersInternal.cpp unittests/ASTMatchers/ASTMatchersTest.cpp unittests/ASTMatchers/ASTMatchersTest.h

Daniel Jasper djasper at google.com
Sun Nov 11 14:14:55 PST 2012


Author: djasper
Date: Sun Nov 11 16:14:55 2012
New Revision: 167695

URL: http://llvm.org/viewvc/llvm-project?rev=167695&view=rev
Log:
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

Modified:
    cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
    cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
    cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
    cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.h

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h?rev=167695&r1=167694&r2=167695&view=diff
==============================================================================
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h Sun Nov 11 16:14:55 2012
@@ -141,7 +141,7 @@
 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.

Modified: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp?rev=167695&r1=167694&r2=167695&view=diff
==============================================================================
--- cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp Sun Nov 11 16:14:55 2012
@@ -51,19 +51,20 @@
 
 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);
     }
   }
 }

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=167695&r1=167694&r2=167695&view=diff
==============================================================================
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Sun Nov 11 16:14:55 2012
@@ -2775,6 +2775,17 @@
       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

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.h?rev=167695&r1=167694&r2=167695&view=diff
==============================================================================
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.h (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.h Sun Nov 11 16:14:55 2012
@@ -38,7 +38,7 @@
 
   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;
     }





More information about the cfe-commits mailing list