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; } _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
