From 86ada78ef4993dbfec778ed962f19756184453bc Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Wed, 14 Nov 2018 10:33:30 +0000 Subject: [AST] Allow limiting the scope of common AST traversals (getParents, RAV). Summary: The goal is to allow analyses such as clang-tidy checks to run on a subset of the AST, e.g. "only on main-file decls" for interactive tools. Today, these become "problematically global" by running RecursiveASTVisitors rooted at the TUDecl, or by navigating up via ASTContext::getParent(). The scope is restricted using a set of top-level-decls that RecursiveASTVisitors should be rooted at. This also applies to the visitor that populates the parent map, and so the top-level-decls are considered to have no parents. This patch makes the traversal scope a mutable property of ASTContext. The more obvious way to do this is to pass the top-level decls to relevant functions directly, but this has some problems: - it's error-prone: accidentally mixing restricted and unrestricted scopes is a performance trap. Interleaving multiple analyses is common (many clang-tidy checks run matchers or RAVs from matcher callbacks) - it doesn't map well to the actual use cases, where we really do want *all* traversals to be restricted. - it involves a lot of plumbing in parts of the code that don't care about traversals. This approach was tried out in D54259 and D54261, I wanted to like it but it feels pretty awful in practice. Caveats: to get scope-limiting behavior of RecursiveASTVisitors, callers have to call the new TraverseAST(Ctx) function instead of TraverseDecl(TU). I think this is an improvement to the API regardless. Reviewers: klimek, ioeric Subscribers: mgorny, cfe-commits Differential Revision: https://reviews.llvm.org/D54309 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@346847 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/ASTMatchers/ASTMatchFinder.cpp | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) (limited to 'lib/ASTMatchers') diff --git a/lib/ASTMatchers/ASTMatchFinder.cpp b/lib/ASTMatchers/ASTMatchFinder.cpp index 63f8395b82..8d65714317 100644 --- a/lib/ASTMatchers/ASTMatchFinder.cpp +++ b/lib/ASTMatchers/ASTMatchFinder.cpp @@ -635,10 +635,6 @@ private: bool memoizedMatchesAncestorOfRecursively( const ast_type_traits::DynTypedNode &Node, const DynTypedMatcher &Matcher, BoundNodesTreeBuilder *Builder, AncestorMatchMode MatchMode) { - if (Node.get() == - 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); @@ -673,7 +669,22 @@ private: BoundNodesTreeBuilder *Builder, AncestorMatchMode MatchMode) { const auto &Parents = ActiveASTContext->getParents(Node); - assert(!Parents.empty() && "Found node that is not in the parent map."); + if (Parents.empty()) { + // Nodes may have no parents if: + // a) the node is the TranslationUnitDecl + // b) we have a limited traversal scope that excludes the parent edges + // c) there is a bug in the AST, and the node is not reachable + // Usually the traversal scope is the whole AST, which precludes b. + // Bugs are common enough that it's worthwhile asserting when we can. + assert(Node.get() || + /* Traversal scope is limited if none of the bounds are the TU */ + llvm::none_of(ActiveASTContext->getTraversalScope(), + [](Decl *D) { + return D->getKind() == Decl::TranslationUnit; + }) && + "Found node that is not in the complete parent map!"); + return false; + } if (Parents.size() == 1) { // Only one parent - do recursive memoization. const ast_type_traits::DynTypedNode Parent = Parents[0]; @@ -1019,7 +1030,7 @@ void MatchFinder::matchAST(ASTContext &Context) { internal::MatchASTVisitor Visitor(&Matchers, Options); Visitor.set_active_ast_context(&Context); Visitor.onStartOfTranslationUnit(); - Visitor.TraverseDecl(Context.getTranslationUnitDecl()); + Visitor.TraverseAST(Context); Visitor.onEndOfTranslationUnit(); } -- cgit v1.2.3