Skip to content

Commit

Permalink
[AST] Allow limiting the scope of common AST traversals (getParents, …
Browse files Browse the repository at this point in the history
…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

llvm-svn: 346847
  • Loading branch information
sam-mccall committed Nov 14, 2018
1 parent 49037d2 commit 814e797
Show file tree
Hide file tree
Showing 7 changed files with 289 additions and 191 deletions.
38 changes: 14 additions & 24 deletions clang/include/clang/AST/ASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -569,26 +569,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
IntrusiveRefCntPtr<ExternalASTSource> ExternalSource;
ASTMutationListener *Listener = nullptr;

/// Contains parents of a node.
using ParentVector = llvm::SmallVector<ast_type_traits::DynTypedNode, 2>;

/// Maps from a node to its parents. This is used for nodes that have
/// pointer identity only, which are more common and we can save space by
/// only storing a unique pointer to them.
using ParentMapPointers =
llvm::DenseMap<const void *,
llvm::PointerUnion4<const Decl *, const Stmt *,
ast_type_traits::DynTypedNode *,
ParentVector *>>;

/// Parent map for nodes without pointer identity. We store a full
/// DynTypedNode for all keys.
using ParentMapOtherNodes =
llvm::DenseMap<ast_type_traits::DynTypedNode,
llvm::PointerUnion4<const Decl *, const Stmt *,
ast_type_traits::DynTypedNode *,
ParentVector *>>;

/// Container for either a single DynTypedNode or for an ArrayRef to
/// DynTypedNode. For use with ParentMap.
class DynTypedNodeList {
Expand Down Expand Up @@ -630,7 +610,17 @@ class ASTContext : public RefCountedBase<ASTContext> {
}
};

/// Returns the parents of the given node.
// A traversal scope limits the parts of the AST visible to certain analyses.
// RecursiveASTVisitor::TraverseAST will only visit reachable nodes, and
// getParents() will only observe reachable parent edges.
//
// The scope is defined by a set of "top-level" declarations.
// Initially, it is the entire TU: {getTranslationUnitDecl()}.
// Changing the scope clears the parent cache, which is expensive to rebuild.
std::vector<Decl *> getTraversalScope() const { return TraversalScope; }
void setTraversalScope(const std::vector<Decl *> &);

/// Returns the parents of the given node (within the traversal scope).
///
/// Note that this will lazily compute the parents of all nodes
/// and store them for later retrieval. Thus, the first call is O(n)
Expand Down Expand Up @@ -2924,13 +2914,13 @@ class ASTContext : public RefCountedBase<ASTContext> {
// but we include it here so that ASTContext can quickly deallocate them.
llvm::PointerIntPair<StoredDeclsMap *, 1> LastSDM;

std::unique_ptr<ParentMapPointers> PointerParents;
std::unique_ptr<ParentMapOtherNodes> OtherParents;
std::vector<Decl *> TraversalScope;
class ParentMap;
std::unique_ptr<ParentMap> Parents;

std::unique_ptr<VTableContextBase> VTContext;

void ReleaseDeclContextMaps();
void ReleaseParentMapEntries();

public:
enum PragmaSectionFlag : unsigned {
Expand Down
10 changes: 10 additions & 0 deletions clang/include/clang/AST/RecursiveASTVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,16 @@ template <typename Derived> class RecursiveASTVisitor {
/// Return whether this visitor should traverse post-order.
bool shouldTraversePostOrder() const { return false; }

/// Recursively visits an entire AST, starting from the top-level Decls
/// in the AST traversal scope (by default, the TranslationUnitDecl).
/// \returns false if visitation was terminated early.
bool TraverseAST(ASTContext &AST) {
for (Decl *D : AST.getTraversalScope())
if (!getDerived().TraverseDecl(D))
return false;
return true;
}

/// Recursively visit a statement or expression, by
/// dispatching to Traverse*() based on the argument's dynamic type.
///
Expand Down
Loading

0 comments on commit 814e797

Please sign in to comment.