diff options
author | Sam McCall <sam.mccall@gmail.com> | 2018-11-14 10:33:30 +0000 |
---|---|---|
committer | Sam McCall <sam.mccall@gmail.com> | 2018-11-14 10:33:30 +0000 |
commit | 86ada78ef4993dbfec778ed962f19756184453bc (patch) | |
tree | be8f6f2ef2ee3c57334c31fbbcb55ca7099f0e74 /lib/ASTMatchers | |
parent | 40b6333e608f4f64a3314ee4fe4a35d40a2974d9 (diff) |
[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
Diffstat (limited to 'lib/ASTMatchers')
-rw-r--r-- | lib/ASTMatchers/ASTMatchFinder.cpp | 23 |
1 files changed, 17 insertions, 6 deletions
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<TranslationUnitDecl>() == - 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<TranslationUnitDecl>() || + /* 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(); } |