Author: sbenza Date: Tue Apr 19 10:52:56 2016 New Revision: 266748 URL: http://llvm.org/viewvc/llvm-project?rev=266748&view=rev Log: [ASTMatchers] Do not try to memoize nodes we can't compare.
Summary: 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. Reviewers: alexfh Subscribers: klimek, cfe-commits Differential Revision: http://reviews.llvm.org/D19231 Modified: cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Modified: cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp?rev=266748&r1=266747&r2=266748&view=diff ============================================================================== --- cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp (original) +++ cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp Tue Apr 19 10:52:56 2016 @@ -616,6 +616,10 @@ private: 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 @@ private: } 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 @@ private: 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 @@ private: 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 Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=266748&r1=266747&r2=266748&view=diff ============================================================================== --- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original) +++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Tue Apr 19 10:52:56 2016 @@ -708,6 +708,19 @@ TEST(DeclarationMatcher, HasDescendantMe 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)))); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
