[clang] d4f3903 - [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.
Yitzhak Mandelbaum via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 3 07:40:58 PDT 2020
Author: Yitzhak Mandelbaum
Date: 2020-09-03T14:39:50Z
New Revision: d4f3903131292d36b3bc22c28798b8e9dae20af6
URL: https://github.com/llvm/llvm-project/commit/d4f3903131292d36b3bc22c28798b8e9dae20af6
DIFF: https://github.com/llvm/llvm-project/commit/d4f3903131292d36b3bc22c28798b8e9dae20af6.diff
LOG: [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.
The new overloads apply directly to a node, like the
`clang::ast_matchers::match` functions, Rather than generating an
`EditGenerator` combinator.
Differential Revision: https://reviews.llvm.org/D87031
Added:
Modified:
clang/include/clang/Tooling/Transformer/RewriteRule.h
clang/lib/Tooling/Transformer/RewriteRule.cpp
clang/unittests/Tooling/TransformerTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Tooling/Transformer/RewriteRule.h b/clang/include/clang/Tooling/Transformer/RewriteRule.h
index 9700d1ff539d..4bdcc8d5c329 100644
--- a/clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ b/clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -380,6 +380,38 @@ EditGenerator rewriteDescendants(std::string NodeId, RewriteRule Rule);
// RewriteRule API. Recast them as such. Or, just declare these functions
// public and well-supported and move them out of `detail`.
namespace detail {
+/// The following overload set is a version of `rewriteDescendants` that
+/// operates directly on the AST, rather than generating a Transformer
+/// combinator. It applies `Rule` to all descendants of `Node`, although not
+/// `Node` itself. `Rule` can refer to nodes bound in `Result`.
+///
+/// For example, assuming that "body" is bound to a function body in MatchResult
+/// `Results`, this will produce edits to change all appearances of `x` in that
+/// body to `3`.
+/// ```
+/// auto InlineX =
+/// makeRule(declRefExpr(to(varDecl(hasName("x")))), changeTo(cat("3")));
+/// const auto *Node = Results.Nodes.getNodeAs<Stmt>("body");
+/// auto Edits = rewriteDescendants(*Node, InlineX, Results);
+/// ```
+/// @{
+llvm::Expected<SmallVector<Edit, 1>>
+rewriteDescendants(const Decl &Node, RewriteRule Rule,
+ const ast_matchers::MatchFinder::MatchResult &Result);
+
+llvm::Expected<SmallVector<Edit, 1>>
+rewriteDescendants(const Stmt &Node, RewriteRule Rule,
+ const ast_matchers::MatchFinder::MatchResult &Result);
+
+llvm::Expected<SmallVector<Edit, 1>>
+rewriteDescendants(const TypeLoc &Node, RewriteRule Rule,
+ const ast_matchers::MatchFinder::MatchResult &Result);
+
+llvm::Expected<SmallVector<Edit, 1>>
+rewriteDescendants(const DynTypedNode &Node, RewriteRule Rule,
+ const ast_matchers::MatchFinder::MatchResult &Result);
+/// @}
+
/// Builds a single matcher for the rule, covering all of the rule's cases.
/// Only supports Rules whose cases' matchers share the same base "kind"
/// (`Stmt`, `Decl`, etc.) Deprecated: use `buildMatchers` instead, which
diff --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index 594e22f56b87..03921e0ea7de 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -242,7 +242,7 @@ class ApplyRuleCallback : public MatchFinder::MatchCallback {
} // namespace
template <typename T>
-static llvm::Expected<SmallVector<clang::transformer::Edit, 1>>
+llvm::Expected<SmallVector<clang::transformer::Edit, 1>>
rewriteDescendantsImpl(const T &Node, RewriteRule Rule,
const MatchResult &Result) {
ApplyRuleCallback Callback(std::move(Rule));
@@ -252,10 +252,43 @@ rewriteDescendantsImpl(const T &Node, RewriteRule Rule,
return std::move(Callback.Edits);
}
+llvm::Expected<SmallVector<clang::transformer::Edit, 1>>
+transformer::detail::rewriteDescendants(const Decl &Node, RewriteRule Rule,
+ const MatchResult &Result) {
+ return rewriteDescendantsImpl(Node, std::move(Rule), Result);
+}
+
+llvm::Expected<SmallVector<clang::transformer::Edit, 1>>
+transformer::detail::rewriteDescendants(const Stmt &Node, RewriteRule Rule,
+ const MatchResult &Result) {
+ return rewriteDescendantsImpl(Node, std::move(Rule), Result);
+}
+
+llvm::Expected<SmallVector<clang::transformer::Edit, 1>>
+transformer::detail::rewriteDescendants(const TypeLoc &Node, RewriteRule Rule,
+ const MatchResult &Result) {
+ return rewriteDescendantsImpl(Node, std::move(Rule), Result);
+}
+
+llvm::Expected<SmallVector<clang::transformer::Edit, 1>>
+transformer::detail::rewriteDescendants(const DynTypedNode &DNode,
+ RewriteRule Rule,
+ const MatchResult &Result) {
+ if (const auto *Node = DNode.get<Decl>())
+ return rewriteDescendantsImpl(*Node, std::move(Rule), Result);
+ if (const auto *Node = DNode.get<Stmt>())
+ return rewriteDescendantsImpl(*Node, std::move(Rule), Result);
+ if (const auto *Node = DNode.get<TypeLoc>())
+ return rewriteDescendantsImpl(*Node, std::move(Rule), Result);
+
+ return llvm::make_error<llvm::StringError>(
+ llvm::errc::invalid_argument,
+ "type unsupported for recursive rewriting, Kind=" +
+ DNode.getNodeKind().asStringRef());
+}
+
EditGenerator transformer::rewriteDescendants(std::string NodeId,
RewriteRule Rule) {
- // FIXME: warn or return error if `Rule` contains any `AddedIncludes`, since
- // these will be dropped.
return [NodeId = std::move(NodeId),
Rule = std::move(Rule)](const MatchResult &Result)
-> llvm::Expected<SmallVector<clang::transformer::Edit, 1>> {
@@ -265,17 +298,7 @@ EditGenerator transformer::rewriteDescendants(std::string NodeId,
if (It == NodesMap.end())
return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
"ID not bound: " + NodeId);
- if (auto *Node = It->second.get<Decl>())
- return rewriteDescendantsImpl(*Node, std::move(Rule), Result);
- if (auto *Node = It->second.get<Stmt>())
- return rewriteDescendantsImpl(*Node, std::move(Rule), Result);
- if (auto *Node = It->second.get<TypeLoc>())
- return rewriteDescendantsImpl(*Node, std::move(Rule), Result);
-
- return llvm::make_error<llvm::StringError>(
- llvm::errc::invalid_argument,
- "type unsupported for recursive rewriting, ID=\"" + NodeId +
- "\", Kind=" + It->second.getNodeKind().asStringRef());
+ return detail::rewriteDescendants(It->second, std::move(Rule), Result);
};
}
diff --git a/clang/unittests/Tooling/TransformerTest.cpp b/clang/unittests/Tooling/TransformerTest.cpp
index 2c9bd7dfd32d..a8d6d3dd851d 100644
--- a/clang/unittests/Tooling/TransformerTest.cpp
+++ b/clang/unittests/Tooling/TransformerTest.cpp
@@ -25,6 +25,7 @@ using ::testing::ElementsAre;
using ::testing::IsEmpty;
using transformer::cat;
using transformer::changeTo;
+using transformer::rewriteDescendants;
using transformer::RewriteRule;
constexpr char KHeaderContents[] = R"cc(
@@ -568,6 +569,88 @@ TEST_F(TransformerTest, RewriteDescendantsInvalidNodeType) {
EXPECT_EQ(ErrorCount, 1);
}
+//
+// We include one test per typed overload. We don't test extensively since that
+// is already covered by the tests above.
+//
+
+TEST_F(TransformerTest, RewriteDescendantsTypedStmt) {
+ // Add an unrelated definition to the header that also has a variable named
+ // "x", to test that the rewrite is limited to the scope we intend.
+ appendToHeader(R"cc(int g(int x) { return x; })cc");
+ std::string Input =
+ "int f(int x) { int y = x; { int z = x * x; } return x; }";
+ std::string Expected =
+ "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+ auto InlineX =
+ makeRule(declRefExpr(to(varDecl(hasName("x")))), changeTo(cat("3")));
+ testRule(makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+ [&InlineX](const MatchFinder::MatchResult &R) {
+ const auto *Node = R.Nodes.getNodeAs<Stmt>("body");
+ assert(Node != nullptr && "body must be bound");
+ return transformer::detail::rewriteDescendants(
+ *Node, InlineX, R);
+ }),
+ Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedDecl) {
+ std::string Input =
+ "int f(int x) { int y = x; { int z = x * x; } return x; }";
+ std::string Expected =
+ "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+ auto InlineX =
+ makeRule(declRefExpr(to(varDecl(hasName("x")))), changeTo(cat("3")));
+ testRule(makeRule(functionDecl(hasName("f")).bind("fun"),
+ [&InlineX](const MatchFinder::MatchResult &R) {
+ const auto *Node = R.Nodes.getNodeAs<Decl>("fun");
+ assert(Node != nullptr && "fun must be bound");
+ return transformer::detail::rewriteDescendants(
+ *Node, InlineX, R);
+ }),
+ Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedTypeLoc) {
+ std::string Input = "int f(int *x) { return *x; }";
+ std::string Expected = "int f(char *x) { return *x; }";
+ auto IntToChar =
+ makeRule(typeLoc(loc(qualType(isInteger(), builtinType()))).bind("loc"),
+ changeTo(cat("char")));
+ testRule(
+ makeRule(
+ functionDecl(
+ hasName("f"),
+ hasParameter(0, varDecl(hasTypeLoc(typeLoc().bind("parmType"))))),
+ [&IntToChar](const MatchFinder::MatchResult &R) {
+ const auto *Node = R.Nodes.getNodeAs<TypeLoc>("parmType");
+ assert(Node != nullptr && "parmType must be bound");
+ return transformer::detail::rewriteDescendants(*Node, IntToChar, R);
+ }),
+ Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedDynTyped) {
+ // Add an unrelated definition to the header that also has a variable named
+ // "x", to test that the rewrite is limited to the scope we intend.
+ appendToHeader(R"cc(int g(int x) { return x; })cc");
+ std::string Input =
+ "int f(int x) { int y = x; { int z = x * x; } return x; }";
+ std::string Expected =
+ "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+ auto InlineX =
+ makeRule(declRefExpr(to(varDecl(hasName("x")))), changeTo(cat("3")));
+ testRule(
+ makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+ [&InlineX](const MatchFinder::MatchResult &R) {
+ auto It = R.Nodes.getMap().find("body");
+ assert(It != R.Nodes.getMap().end() && "body must be bound");
+ return transformer::detail::rewriteDescendants(It->second,
+ InlineX, R);
+ }),
+ Input, Expected);
+}
+
TEST_F(TransformerTest, InsertBeforeEdit) {
std::string Input = R"cc(
int f() {
More information about the cfe-commits
mailing list