Skip to content

Commit

Permalink
[AST] Ignore implicit nodes in IgnoreUnlessSpelledInSource mode
Browse files Browse the repository at this point in the history
Update the ASTNodeTraverser to dump only nodes spelled in source.  There
are only a few which need to be handled, but Decl nodes for which
isImplicit() is true are handled together.

Update the RAV instances used in ASTMatchFinder to ignore the nodes too.
As with handling of template instantiations, it is necessary to allow
the RAV to process the implicit nodes because they need to be visitable
before the first traverse() matcher is encountered.  An exception to
this is in the MatchChildASTVisitor, because we sometimes wish to make a
node matchable but make its children not-matchable.  This is the case
for defaulted CXXMethodDecls for example.

Extend TransformerTests to illustrate the kinds of problems that can
arise when performing source code rewriting due to matching implicit
nodes.

This change accounts for handling nodes not spelled in source when using
direct matching of nodes, and when using the has() and hasDescendant()
matchers.  Other matchers such as
cxxRecordDecl(hasMethod(cxxMethodDecl())) still succeed for
compiler-generated methods for example after this change.  Updating the
implementations of hasMethod() and other matchers is for a follow-up
patch.

Differential Revision: https://reviews.llvm.org/D90982
  • Loading branch information
steveire committed Nov 17, 2020
1 parent 34c0f3c commit 246b428
Show file tree
Hide file tree
Showing 6 changed files with 985 additions and 16 deletions.
4 changes: 3 additions & 1 deletion clang/docs/ReleaseNotes.rst
Expand Up @@ -230,7 +230,9 @@ release of Clang. Users of the build system should adjust accordingly.
AST Matchers
------------

- ...
- The behavior of TK_IgnoreUnlessSpelledInSource with the traverse() matcher
has been changed to no longer match on template instantiations or on
implicit nodes which are not spelled in the source.

clang-format
------------
Expand Down
34 changes: 33 additions & 1 deletion clang/include/clang/AST/ASTNodeTraverser.h
Expand Up @@ -85,6 +85,9 @@ class ASTNodeTraverser
TraversalKind GetTraversalKind() const { return Traversal; }

void Visit(const Decl *D) {
if (Traversal == TK_IgnoreUnlessSpelledInSource && D->isImplicit())
return;

getNodeDelegate().AddChild([=] {
getNodeDelegate().Visit(D);
if (!D)
Expand Down Expand Up @@ -144,7 +147,8 @@ class ASTNodeTraverser
if (isa<DeclStmt>(S) || isa<GenericSelectionExpr>(S))
return;

if (isa<LambdaExpr>(S) && Traversal == TK_IgnoreUnlessSpelledInSource)
if (Traversal == TK_IgnoreUnlessSpelledInSource &&
isa<LambdaExpr, CXXForRangeStmt, CallExpr>(S))
return;

for (const Stmt *SubStmt : S->children())
Expand Down Expand Up @@ -185,6 +189,8 @@ class ASTNodeTraverser
}

void Visit(const CXXCtorInitializer *Init) {
if (Traversal == TK_IgnoreUnlessSpelledInSource && !Init->isWritten())
return;
getNodeDelegate().AddChild([=] {
getNodeDelegate().Visit(Init);
Visit(Init->getInit());
Expand Down Expand Up @@ -401,6 +407,9 @@ class ASTNodeTraverser
if (const Expr *TRC = D->getTrailingRequiresClause())
Visit(TRC);

if (Traversal == TK_IgnoreUnlessSpelledInSource && D->isDefaulted())
return;

if (const auto *C = dyn_cast<CXXConstructorDecl>(D))
for (const auto *I : C->inits())
Visit(I);
Expand All @@ -417,6 +426,9 @@ class ASTNodeTraverser
}

void VisitVarDecl(const VarDecl *D) {
if (Traversal == TK_IgnoreUnlessSpelledInSource && D->isCXXForRangeDecl())
return;

if (D->hasInit())
Visit(D->getInit());
}
Expand Down Expand Up @@ -717,6 +729,26 @@ class ASTNodeTraverser
Visit(CatchParam);
}

void VisitCXXForRangeStmt(const CXXForRangeStmt *Node) {
if (Traversal == TK_IgnoreUnlessSpelledInSource) {
Visit(Node->getInit());
Visit(Node->getLoopVariable());
Visit(Node->getRangeInit());
Visit(Node->getBody());
}
}

void VisitCallExpr(const CallExpr *Node) {
for (const auto *Child :
make_filter_range(Node->children(), [this](const Stmt *Child) {
if (Traversal != TK_IgnoreUnlessSpelledInSource)
return false;
return !isa<CXXDefaultArgExpr>(Child);
})) {
Visit(Child);
}
}

void VisitExpressionTemplateArgument(const TemplateArgument &TA) {
Visit(TA.getAsExpr());
}
Expand Down
80 changes: 70 additions & 10 deletions clang/lib/ASTMatchers/ASTMatchFinder.cpp
Expand Up @@ -95,9 +95,12 @@ class MatchChildASTVisitor
// matching the descendants.
MatchChildASTVisitor(const DynTypedMatcher *Matcher, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder, int MaxDepth,
TraversalKind Traversal, ASTMatchFinder::BindKind Bind)
TraversalKind Traversal, bool IgnoreImplicitChildren,
ASTMatchFinder::BindKind Bind)
: Matcher(Matcher), Finder(Finder), Builder(Builder), CurrentDepth(0),
MaxDepth(MaxDepth), Traversal(Traversal), Bind(Bind), Matches(false) {}
MaxDepth(MaxDepth), Traversal(Traversal),
IgnoreImplicitChildren(IgnoreImplicitChildren), Bind(Bind),
Matches(false) {}

// Returns true if a match is found in the subtree rooted at the
// given AST node. This is done via a set of mutually recursive
Expand Down Expand Up @@ -145,6 +148,11 @@ class MatchChildASTVisitor
// They are public only to allow CRTP to work. They are *not *part
// of the public API of this class.
bool TraverseDecl(Decl *DeclNode) {

if (DeclNode && DeclNode->isImplicit() &&
Finder->isTraversalIgnoringImplicitNodes())
return baseTraverse(*DeclNode);

ScopedIncrement ScopedDepth(&CurrentDepth);
return (DeclNode == nullptr) || traverse(*DeclNode);
}
Expand Down Expand Up @@ -176,6 +184,10 @@ class MatchChildASTVisitor
Stmt *StmtToTraverse = getStmtToTraverse(StmtNode);
if (!StmtToTraverse)
return true;

if (IgnoreImplicitChildren && isa<CXXDefaultArgExpr>(StmtNode))
return true;

if (!match(*StmtToTraverse))
return false;
return VisitorBase::TraverseStmt(StmtToTraverse, Queue);
Expand Down Expand Up @@ -265,7 +277,7 @@ class MatchChildASTVisitor
}

bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return true; }
bool shouldVisitImplicitCode() const { return !IgnoreImplicitChildren; }

private:
// Used for updating the depth during traversal.
Expand Down Expand Up @@ -360,6 +372,7 @@ class MatchChildASTVisitor
int CurrentDepth;
const int MaxDepth;
const TraversalKind Traversal;
const bool IgnoreImplicitChildren;
const ASTMatchFinder::BindKind Bind;
bool Matches;
};
Expand Down Expand Up @@ -497,19 +510,21 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
const DynTypedMatcher &Matcher,
BoundNodesTreeBuilder *Builder, int MaxDepth,
TraversalKind Traversal, BindKind Bind) {
bool ScopedTraversal = TraversingASTNodeNotSpelledInSource;
bool ScopedTraversal = TraversingASTNodeNotSpelledInSource ||
TraversingASTChildrenNotSpelledInSource;

if (const auto *CTSD = Node.get<ClassTemplateSpecializationDecl>()) {
int SK = CTSD->getSpecializationKind();
if (SK == TSK_ExplicitInstantiationDeclaration ||
SK == TSK_ExplicitInstantiationDefinition)
bool IgnoreImplicitChildren = false;

if (isTraversalIgnoringImplicitNodes()) {
IgnoreImplicitChildren = true;
if (Node.get<CXXForRangeStmt>())
ScopedTraversal = true;
}

ASTNodeNotSpelledInSourceScope RAII(this, ScopedTraversal);

MatchChildASTVisitor Visitor(
&Matcher, this, Builder, MaxDepth, Traversal, Bind);
MatchChildASTVisitor Visitor(&Matcher, this, Builder, MaxDepth, Traversal,
IgnoreImplicitChildren, Bind);
return Visitor.findMatch(Node);
}

Expand Down Expand Up @@ -616,6 +631,7 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,

private:
bool TraversingASTNodeNotSpelledInSource = false;
bool TraversingASTChildrenNotSpelledInSource = false;

struct ASTNodeNotSpelledInSourceScope {
ASTNodeNotSpelledInSourceScope(MatchASTVisitor *V, bool B)
Expand All @@ -631,6 +647,20 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
bool MB;
};

struct ASTChildrenNotSpelledInSource {
ASTChildrenNotSpelledInSource(MatchASTVisitor *V, bool B)
: MV(V), MB(V->TraversingASTChildrenNotSpelledInSource) {
V->TraversingASTChildrenNotSpelledInSource = B;
}
~ASTChildrenNotSpelledInSource() {
MV->TraversingASTChildrenNotSpelledInSource = MB;
}

private:
MatchASTVisitor *MV;
bool MB;
};

class TimeBucketRegion {
public:
TimeBucketRegion() : Bucket(nullptr) {}
Expand Down Expand Up @@ -1050,6 +1080,24 @@ bool MatchASTVisitor::TraverseDecl(Decl *DeclNode) {
if (!DeclNode) {
return true;
}

bool ScopedTraversal =
TraversingASTNodeNotSpelledInSource || DeclNode->isImplicit();
bool ScopedChildren = TraversingASTChildrenNotSpelledInSource;

if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(DeclNode)) {
auto SK = CTSD->getSpecializationKind();
if (SK == TSK_ExplicitInstantiationDeclaration ||
SK == TSK_ExplicitInstantiationDefinition)
ScopedChildren = true;
} else if (const auto *FD = dyn_cast<FunctionDecl>(DeclNode)) {
if (FD->isDefaulted())
ScopedChildren = true;
}

ASTNodeNotSpelledInSourceScope RAII1(this, ScopedTraversal);
ASTChildrenNotSpelledInSource RAII2(this, ScopedChildren);

match(*DeclNode);
return RecursiveASTVisitor<MatchASTVisitor>::TraverseDecl(DeclNode);
}
Expand All @@ -1058,6 +1106,10 @@ bool MatchASTVisitor::TraverseStmt(Stmt *StmtNode, DataRecursionQueue *Queue) {
if (!StmtNode) {
return true;
}
bool ScopedTraversal = TraversingASTNodeNotSpelledInSource ||
TraversingASTChildrenNotSpelledInSource;

ASTNodeNotSpelledInSourceScope RAII(this, ScopedTraversal);
match(*StmtNode);
return RecursiveASTVisitor<MatchASTVisitor>::TraverseStmt(StmtNode, Queue);
}
Expand Down Expand Up @@ -1103,6 +1155,14 @@ bool MatchASTVisitor::TraverseConstructorInitializer(
if (!CtorInit)
return true;

bool ScopedTraversal = TraversingASTNodeNotSpelledInSource ||
TraversingASTChildrenNotSpelledInSource;

if (!CtorInit->isWritten())
ScopedTraversal = true;

ASTNodeNotSpelledInSourceScope RAII1(this, ScopedTraversal);

match(*CtorInit);

return RecursiveASTVisitor<MatchASTVisitor>::TraverseConstructorInitializer(
Expand Down

0 comments on commit 246b428

Please sign in to comment.