[clang] 4cadb66 - [AST] Update matchers to be traverse-aware
Stephen Kelly via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 17 08:50:48 PST 2020
Author: Stephen Kelly
Date: 2020-11-17T16:31:08Z
New Revision: 4cadb66b490e3323f79a89120c3442456e4c24fa
URL: https://github.com/llvm/llvm-project/commit/4cadb66b490e3323f79a89120c3442456e4c24fa
DIFF: https://github.com/llvm/llvm-project/commit/4cadb66b490e3323f79a89120c3442456e4c24fa.diff
LOG: [AST] Update matchers to be traverse-aware
Don't match Stmt or Decl nodes not spelled in the source when using
TK_IgnoreUnlessSpelledInSource. This prevents accidental modification
of source code at incorrect locations.
Differential Revision: https://reviews.llvm.org/D90984
Added:
Modified:
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/include/clang/ASTMatchers/ASTMatchersInternal.h
clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
clang/unittests/Tooling/TransformerTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index f10baf5c3b5e..39274331a12e 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -3106,9 +3106,16 @@ AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
/// \c A but not \c B.
AST_MATCHER_P(CXXRecordDecl, hasMethod, internal::Matcher<CXXMethodDecl>,
InnerMatcher) {
- return matchesFirstInPointerRange(InnerMatcher, Node.method_begin(),
- Node.method_end(), Finder,
- Builder) != Node.method_end();
+ BoundNodesTreeBuilder Result(*Builder);
+ auto MatchIt = matchesFirstInPointerRange(InnerMatcher, Node.method_begin(),
+ Node.method_end(), Finder, &Result);
+ if (MatchIt == Node.method_end())
+ return false;
+
+ if (Finder->isTraversalIgnoringImplicitNodes() && (*MatchIt)->isImplicit())
+ return false;
+ *Builder = std::move(Result);
+ return true;
}
/// Matches the generated class of lambda expressions.
@@ -4044,7 +4051,15 @@ AST_POLYMORPHIC_MATCHER_P(argumentCountIs,
CallExpr, CXXConstructExpr,
CXXUnresolvedConstructExpr, ObjCMessageExpr),
unsigned, N) {
- return Node.getNumArgs() == N;
+ unsigned NumArgs = Node.getNumArgs();
+ if (!Finder->isTraversalIgnoringImplicitNodes())
+ return NumArgs == N;
+ while (NumArgs) {
+ if (!isa<CXXDefaultArgExpr>(Node.getArg(NumArgs - 1)))
+ break;
+ --NumArgs;
+ }
+ return NumArgs == N;
}
/// Matches the n'th argument of a call expression or a constructor
@@ -4060,9 +4075,12 @@ AST_POLYMORPHIC_MATCHER_P2(hasArgument,
CallExpr, CXXConstructExpr,
CXXUnresolvedConstructExpr, ObjCMessageExpr),
unsigned, N, internal::Matcher<Expr>, InnerMatcher) {
- return (N < Node.getNumArgs() &&
- InnerMatcher.matches(
- *Node.getArg(N)->IgnoreParenImpCasts(), Finder, Builder));
+ if (N >= Node.getNumArgs())
+ return false;
+ const Expr *Arg = Node.getArg(N);
+ if (Finder->isTraversalIgnoringImplicitNodes() && isa<CXXDefaultArgExpr>(Arg))
+ return false;
+ return InnerMatcher.matches(*Arg->IgnoreParenImpCasts(), Finder, Builder);
}
/// Matches the n'th item of an initializer list expression.
@@ -4154,9 +4172,11 @@ AST_MATCHER(CXXCatchStmt, isCatchAll) {
/// record matches Foo, hasAnyConstructorInitializer matches foo_(1)
AST_MATCHER_P(CXXConstructorDecl, hasAnyConstructorInitializer,
internal::Matcher<CXXCtorInitializer>, InnerMatcher) {
- return matchesFirstInPointerRange(InnerMatcher, Node.init_begin(),
- Node.init_end(), Finder,
- Builder) != Node.init_end();
+ auto MatchIt = matchesFirstInPointerRange(InnerMatcher, Node.init_begin(),
+ Node.init_end(), Finder, Builder);
+ if (MatchIt == Node.init_end())
+ return false;
+ return (*MatchIt)->isWritten() || !Finder->isTraversalIgnoringImplicitNodes();
}
/// Matches the field declaration of a constructor initializer.
@@ -4281,6 +4301,9 @@ AST_POLYMORPHIC_MATCHER_P(hasAnyArgument,
CXXUnresolvedConstructExpr, ObjCMessageExpr),
internal::Matcher<Expr>, InnerMatcher) {
for (const Expr *Arg : Node.arguments()) {
+ if (Finder->isTraversalIgnoringImplicitNodes() &&
+ isa<CXXDefaultArgExpr>(Arg))
+ break;
BoundNodesTreeBuilder Result(*Builder);
if (InnerMatcher.matches(*Arg, Finder, &Result)) {
*Builder = std::move(Result);
@@ -4998,6 +5021,8 @@ AST_POLYMORPHIC_MATCHER_P(hasBody,
CXXForRangeStmt,
FunctionDecl),
internal::Matcher<Stmt>, InnerMatcher) {
+ if (Finder->isTraversalIgnoringImplicitNodes() && isDefaultedHelper(&Node))
+ return false;
const Stmt *const Statement = internal::GetBodyMatcher<NodeType>::get(Node);
return (Statement != nullptr &&
InnerMatcher.matches(*Statement, Finder, Builder));
@@ -6880,6 +6905,8 @@ AST_MATCHER_P(CXXConstructorDecl, forEachConstructorInitializer,
BoundNodesTreeBuilder Result;
bool Matched = false;
for (const auto *I : Node.inits()) {
+ if (Finder->isTraversalIgnoringImplicitNodes() && !I->isWritten())
+ continue;
BoundNodesTreeBuilder InitBuilder(*Builder);
if (InnerMatcher.matches(*I, Finder, &InitBuilder)) {
Matched = true;
diff --git a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
index 7b6293f5eac3..480a12b0dafb 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -789,6 +789,15 @@ IteratorT matchesFirstInPointerRange(const MatcherT &Matcher, IteratorT Start,
return End;
}
+template <typename T, std::enable_if_t<!std::is_base_of<FunctionDecl, T>::value>
+ * = nullptr>
+inline bool isDefaultedHelper(const T *) {
+ return false;
+}
+inline bool isDefaultedHelper(const FunctionDecl *FD) {
+ return FD->isDefaulted();
+}
+
// Metafunction to determine if type T has a member called getDecl.
template <typename Ty>
class has_getDecl {
@@ -1056,6 +1065,8 @@ class HasDeclarationMatcher : public MatcherInterface<T> {
/// is \c NULL.
bool matchesDecl(const Decl *Node, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder) const {
+ if (Finder->isTraversalIgnoringImplicitNodes() && Node->isImplicit())
+ return false;
return Node != nullptr && this->InnerMatcher.matches(
DynTypedNode::create(*Node), Finder, Builder);
}
diff --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
index 7c4ba9bcc2a6..60668af89e97 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -2368,6 +2368,13 @@ struct HasCtorInits : NoSpecialMethods, NonTrivial
HasCtorInits() : NoSpecialMethods(), m_i(42) {}
};
+struct CtorInitsNonTrivial : NonTrivial
+{
+ int m_i;
+ NonTrivial m_nt;
+ CtorInitsNonTrivial() : NonTrivial(), m_i(42) {}
+};
+
)cpp";
{
auto M = cxxRecordDecl(hasName("NoSpecialMethods"),
@@ -2393,6 +2400,35 @@ struct HasCtorInits : NoSpecialMethods, NonTrivial
M = cxxRecordDecl(hasName("NoSpecialMethods"), has(cxxDestructorDecl()));
EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+
+ M = cxxRecordDecl(hasName("NoSpecialMethods"),
+ hasMethod(cxxConstructorDecl(isCopyConstructor())));
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+ EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+
+ M = cxxRecordDecl(hasName("NoSpecialMethods"),
+ hasMethod(cxxMethodDecl(isCopyAssignmentOperator())));
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+ EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+
+ M = cxxRecordDecl(hasName("NoSpecialMethods"),
+ hasMethod(cxxConstructorDecl(isDefaultConstructor())));
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+ EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+
+ M = cxxRecordDecl(hasName("NoSpecialMethods"),
+ hasMethod(cxxDestructorDecl()));
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+ EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+ }
+ {
+ // Because the copy-assignment operator is not spelled in the
+ // source (ie, isImplicit()), we don't match it
+ auto M =
+ cxxOperatorCallExpr(hasType(cxxRecordDecl(hasName("NoSpecialMethods"))),
+ callee(cxxMethodDecl(isCopyAssignmentOperator())));
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+ EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
}
{
// Compiler generates a forStmt to copy the array
@@ -2414,6 +2450,24 @@ struct HasCtorInits : NoSpecialMethods, NonTrivial
EXPECT_TRUE(matches(Code, traverse(TK_AsIs, MDef)));
EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MDef)));
+ auto MBody = cxxMethodDecl(MDecl, hasBody(compoundStmt()));
+
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, MBody)));
+ EXPECT_FALSE(
+ matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MBody)));
+
+ auto MIsDefn = cxxMethodDecl(MDecl, isDefinition());
+
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, MIsDefn)));
+ EXPECT_TRUE(
+ matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MIsDefn)));
+
+ auto MIsInline = cxxMethodDecl(MDecl, isInline());
+
+ EXPECT_FALSE(matches(Code, traverse(TK_AsIs, MIsInline)));
+ EXPECT_FALSE(
+ matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MIsInline)));
+
// The parameter of the defaulted method can still be matched.
auto MParm =
cxxMethodDecl(MDecl, hasParameter(0, parmVarDecl(hasName("other"))));
@@ -2435,19 +2489,34 @@ struct HasCtorInits : NoSpecialMethods, NonTrivial
EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
}
+ {
+ auto M = cxxConstructorDecl(hasName("HasCtorInits"),
+ hasAnyConstructorInitializer(cxxCtorInitializer(
+ forField(hasName("m_nt")))));
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+ EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+ }
+ {
+ auto M =
+ cxxConstructorDecl(hasName("HasCtorInits"),
+ forEachConstructorInitializer(
+ cxxCtorInitializer(forField(hasName("m_nt")))));
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+ EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+ }
{
auto M = cxxConstructorDecl(
- hasName("HasCtorInits"),
- has(cxxCtorInitializer(withInitializer(cxxConstructExpr(hasDeclaration(
- cxxConstructorDecl(hasName("NoSpecialMethods"))))))));
+ hasName("CtorInitsNonTrivial"),
+ has(cxxCtorInitializer(withInitializer(cxxConstructExpr(
+ hasDeclaration(cxxConstructorDecl(hasName("NonTrivial"))))))));
EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
}
{
auto M = cxxConstructorDecl(
hasName("HasCtorInits"),
- has(cxxCtorInitializer(withInitializer(cxxConstructExpr(
- hasDeclaration(cxxConstructorDecl(hasName("NonTrivial"))))))));
+ has(cxxCtorInitializer(withInitializer(cxxConstructExpr(hasDeclaration(
+ cxxConstructorDecl(hasName("NoSpecialMethods"))))))));
EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
}
@@ -2573,13 +2642,37 @@ void callDefaultArg()
hasDefaultArg(42);
}
)cpp";
+ auto hasDefaultArgCall = [](auto InnerMatcher) {
+ return callExpr(callee(functionDecl(hasName("hasDefaultArg"))),
+ InnerMatcher);
+ };
{
- auto M = callExpr(has(integerLiteral(equals(42))));
+ auto M = hasDefaultArgCall(has(integerLiteral(equals(42))));
EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
}
{
- auto M = callExpr(has(cxxDefaultArgExpr()));
+ auto M = hasDefaultArgCall(has(cxxDefaultArgExpr()));
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+ EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+ }
+ {
+ auto M = hasDefaultArgCall(argumentCountIs(2));
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+ EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+ }
+ {
+ auto M = hasDefaultArgCall(argumentCountIs(1));
+ EXPECT_FALSE(matches(Code, traverse(TK_AsIs, M)));
+ EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+ }
+ {
+ auto M = hasDefaultArgCall(hasArgument(1, cxxDefaultArgExpr()));
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+ EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+ }
+ {
+ auto M = hasDefaultArgCall(hasAnyArgument(cxxDefaultArgExpr()));
EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
}
diff --git a/clang/unittests/Tooling/TransformerTest.cpp b/clang/unittests/Tooling/TransformerTest.cpp
index f70dec414b8d..0a09b0dd4eee 100644
--- a/clang/unittests/Tooling/TransformerTest.cpp
+++ b/clang/unittests/Tooling/TransformerTest.cpp
@@ -1245,6 +1245,32 @@ void testIt()
testRuleFailure(makeRule(traverse(TK_AsIs, MatchedLoop), RewriteRule),
RewriteInput);
}
+ {
+ auto MatchedLoop = forStmt(
+ hasLoopInit(declStmt(
+ hasSingleDecl(varDecl(hasInitializer(integerLiteral(equals(0))))
+ .bind("loopVar")))),
+ hasCondition(binaryOperator(hasOperatorName("!="),
+ hasLHS(ignoringImplicit(declRefExpr(to(
+ varDecl(equalsBoundNode("loopVar")))))),
+ hasRHS(expr().bind("upperBoundExpr")))),
+ hasIncrement(unaryOperator(hasOperatorName("++"),
+ hasUnaryOperand(declRefExpr(to(
+ varDecl(equalsBoundNode("loopVar"))))))
+ .bind("incrementOp")));
+
+ auto RewriteRule =
+ changeTo(transformer::enclose(node("loopVar"), node("incrementOp")),
+ cat("auto ", name("loopVar"), " : boost::irange(",
+ node("upperBoundExpr"), ")"));
+
+ testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedLoop),
+ RewriteRule),
+ RewriteInput, RewriteOutput);
+
+ testRuleFailure(makeRule(traverse(TK_AsIs, MatchedLoop), RewriteRule),
+ RewriteInput);
+ }
}
TEST_F(TransformerTest, TemplateInstantiation) {
More information about the cfe-commits
mailing list