maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

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
llvm-svn: 168177
This commit is contained in:
Daniel Jasper 2012-11-16 18:39:22 +00:00
parent 7b8af0ea05
commit 94a56856d2
2 changed files with 21 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
clang/lib/ASTMatchers/ASTMatchersInternal.cpp
View File

@ -27,8 +27,11 @@ void BoundNodesMap::copyTo(BoundNodesTreeBuilder *Builder) const {
}
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() {}

16
clang/unittests/ASTMatchers/ASTMatchersTest.cpp
View File

@ -2777,6 +2777,22 @@ TEST(ForEachDescendant, BindsOneNode) {
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; }; "