Hi klimek,
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.
http://llvm-reviews.chandlerc.com/D112
Files:
include/clang/ASTMatchers/ASTMatchersInternal.h
lib/ASTMatchers/ASTMatchersInternal.cpp
unittests/ASTMatchers/ASTMatchersTest.cpp
unittests/ASTMatchers/ASTMatchersTest.h
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===================================================================
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -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.
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===================================================================
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -51,19 +51,21 @@
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.copyTo(&CombinedBindings);
+ 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);
}
}
}
Index: unittests/ASTMatchers/ASTMatchersTest.cpp
===================================================================
--- unittests/ASTMatchers/ASTMatchersTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -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
Index: unittests/ASTMatchers/ASTMatchersTest.h
===================================================================
--- unittests/ASTMatchers/ASTMatchersTest.h
+++ unittests/ASTMatchers/ASTMatchersTest.h
@@ -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;
}
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===================================================================
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -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.
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===================================================================
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -51,19 +51,21 @@
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.copyTo(&CombinedBindings);
+ 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);
}
}
}
Index: unittests/ASTMatchers/ASTMatchersTest.cpp
===================================================================
--- unittests/ASTMatchers/ASTMatchersTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -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
Index: unittests/ASTMatchers/ASTMatchersTest.h
===================================================================
--- unittests/ASTMatchers/ASTMatchersTest.h
+++ unittests/ASTMatchers/ASTMatchersTest.h
@@ -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
