[cfe-commits] r168177 - in /cfe/trunk: lib/ASTMatchers/ASTMatchersInternal.cpp unittests/ASTMatchers/ASTMatchersTest.cpp
Daniel Jasper
djasper at google.com
Fri Nov 16 10:39:23 PST 2012
Author: djasper
Date: Fri Nov 16 12:39:22 2012
New Revision: 168177
URL: http://llvm.org/viewvc/llvm-project?rev=168177&view=rev
Log:
Fix partial-match-bind-behavior with forEachDescendant() matchers.
The problem is that a partial match of an (explicit or implicit) allOf matcher
binds results, i.e.
recordDecl(decl().bind("x"), hasName("A"))
can very well bind a record that is not named "A". With this fix, the common
cases of stumbling over this bug are fixed by the BoundNodesMap overwriting the
results of a partial match. An error can still be created with a weird
combination of anyOf and allOf (see inactive test). We need to decide whether
this is worth fixing, as the fix will have performance impact.
Review: http://llvm-reviews.chandlerc.com/D124
Modified:
cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
Modified: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp?rev=168177&r1=168176&r2=168177&view=diff
==============================================================================
--- cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp Fri Nov 16 12:39:22 2012
@@ -27,8 +27,11 @@
}
void BoundNodesMap::copyTo(BoundNodesMap *Other) const {
- copy(NodeMap.begin(), NodeMap.end(),
- inserter(Other->NodeMap, Other->NodeMap.begin()));
+ for (IDToNodeMap::const_iterator I = NodeMap.begin(),
+ E = NodeMap.end();
+ I != E; ++I) {
+ Other->NodeMap[I->first] = I->second;
+ }
}
BoundNodesTree::BoundNodesTree() {}
Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=168177&r1=168176&r2=168177&view=diff
==============================================================================
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Fri Nov 16 12:39:22 2012
@@ -2777,6 +2777,22 @@
new VerifyIdIsBoundTo<FieldDecl>("x", 1)));
}
+TEST(ForEachDescendant, NestedForEachDescendant) {
+ DeclarationMatcher m = recordDecl(
+ isDefinition(), decl().bind("x"), hasName("C"));
+ EXPECT_TRUE(matchAndVerifyResultTrue(
+ "class A { class B { class C {}; }; };",
+ recordDecl(hasName("A"), anyOf(m, forEachDescendant(m))),
+ new VerifyIdIsBoundTo<Decl>("x", "C")));
+
+ // FIXME: This is not really a useful matcher, but the result is still
+ // surprising (currently binds "A").
+ //EXPECT_TRUE(matchAndVerifyResultTrue(
+ // "class A { class B { class C {}; }; };",
+ // recordDecl(hasName("A"), allOf(hasDescendant(m), anyOf(m, anything()))),
+ // new VerifyIdIsBoundTo<Decl>("x", "C")));
+}
+
TEST(ForEachDescendant, BindsMultipleNodes) {
EXPECT_TRUE(matchAndVerifyResultTrue(
"class C { class D { int x; int y; }; "
More information about the cfe-commits
mailing list