sbenza created this revision.
sbenza added a reviewer: alexfh.
sbenza added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
Prevent hasAncestor from comparing nodes that are not supported.
hasDescendant was fixed some time ago to avoid this problem.
I'm applying the same fix to hasAncestor: if any object in the Builder map is
not comparable, skip the cache.
http://reviews.llvm.org/D19231
Files:
lib/ASTMatchers/ASTMatchFinder.cpp
unittests/ASTMatchers/ASTMatchersTest.cpp
Index: unittests/ASTMatchers/ASTMatchersTest.cpp
===================================================================
--- unittests/ASTMatchers/ASTMatchersTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -708,6 +708,19 @@
decl(anyOf(hasDescendant(RD), hasDescendant(VD)))));
}
+TEST(DeclarationMatcher, HasAncestorMemoization) {
+ // This triggers an hasAncestor with a TemplateArgument in the bound nodes.
+ // That node can't be memoized so we have to check for it before trying to put
+ // it on the cache.
+ DeclarationMatcher CannotMemoize = classTemplateSpecializationDecl(
+ hasAnyTemplateArgument(templateArgument().bind("targ")),
+ forEach(fieldDecl(hasAncestor(forStmt()))));
+
+ EXPECT_TRUE(notMatches("template <typename T> struct S;"
+ "template <> struct S<int>{ int i; int j; };",
+ CannotMemoize));
+}
+
TEST(DeclarationMatcher, HasAttr) {
EXPECT_TRUE(matches("struct __attribute__((warn_unused)) X {};",
decl(hasAttr(clang::attr::WarnUnused))));
Index: lib/ASTMatchers/ASTMatchFinder.cpp
===================================================================
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -616,6 +616,10 @@
ActiveASTContext->getTranslationUnitDecl())
return false;
+ // For AST-nodes that don't have an identity, we can't memoize.
+ if (!Builder->isComparable())
+ return matchesAncestorOfRecursively(Node, Matcher, Builder, MatchMode);
+
MatchKey Key;
Key.MatcherID = Matcher.getID();
Key.Node = Node;
@@ -630,22 +634,34 @@
}
MemoizedMatchResult Result;
- Result.ResultOfMatch = false;
Result.Nodes = *Builder;
+ Result.ResultOfMatch =
+ matchesAncestorOfRecursively(Node, Matcher, &Result.Nodes, MatchMode);
+
+ MemoizedMatchResult &CachedResult = ResultCache[Key];
+ CachedResult = std::move(Result);
+
+ *Builder = CachedResult.Nodes;
+ return CachedResult.ResultOfMatch;
+ }
+ bool matchesAncestorOfRecursively(const ast_type_traits::DynTypedNode &Node,
+ const DynTypedMatcher &Matcher,
+ BoundNodesTreeBuilder *Builder,
+ AncestorMatchMode MatchMode) {
const auto &Parents = ActiveASTContext->getParents(Node);
assert(!Parents.empty() && "Found node that is not in the parent map.");
if (Parents.size() == 1) {
// Only one parent - do recursive memoization.
const ast_type_traits::DynTypedNode Parent = Parents[0];
- if (Matcher.matches(Parent, this, &Result.Nodes)) {
- Result.ResultOfMatch = true;
- } else if (MatchMode != ASTMatchFinder::AMM_ParentOnly) {
- // Reset the results to not include the bound nodes from the failed
- // match above.
- Result.Nodes = *Builder;
- Result.ResultOfMatch = memoizedMatchesAncestorOfRecursively(
- Parent, Matcher, &Result.Nodes, MatchMode);
+ BoundNodesTreeBuilder BuilderCopy = *Builder;
+ if (Matcher.matches(Parent, this, &BuilderCopy)) {
+ *Builder = std::move(BuilderCopy);
+ return true;
+ }
+ if (MatchMode != ASTMatchFinder::AMM_ParentOnly) {
+ return memoizedMatchesAncestorOfRecursively(Parent, Matcher, Builder,
+ MatchMode);
// Once we get back from the recursive call, the result will be the
// same as the parent's result.
}
@@ -655,10 +671,10 @@
std::deque<ast_type_traits::DynTypedNode> Queue(Parents.begin(),
Parents.end());
while (!Queue.empty()) {
- Result.Nodes = *Builder;
- if (Matcher.matches(Queue.front(), this, &Result.Nodes)) {
- Result.ResultOfMatch = true;
- break;
+ BoundNodesTreeBuilder BuilderCopy = *Builder;
+ if (Matcher.matches(Queue.front(), this, &BuilderCopy)) {
+ *Builder = std::move(BuilderCopy);
+ return true;
}
if (MatchMode != ASTMatchFinder::AMM_ParentOnly) {
for (const auto &Parent :
@@ -673,12 +689,7 @@
Queue.pop_front();
}
}
-
- MemoizedMatchResult &CachedResult = ResultCache[Key];
- CachedResult = std::move(Result);
-
- *Builder = CachedResult.Nodes;
- return CachedResult.ResultOfMatch;
+ return false;
}
// Implements a BoundNodesTree::Visitor that calls a MatchCallback with
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
