ymandel updated this revision to Diff 266729.
ymandel added a comment.
adds a fix to transformer
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80606/new/
https://reviews.llvm.org/D80606
Files:
clang/include/clang/Tooling/Transformer/RewriteRule.h
clang/lib/Tooling/Transformer/RewriteRule.cpp
clang/unittests/Tooling/TransformerTest.cpp
Index: clang/unittests/Tooling/TransformerTest.cpp
===================================================================
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -571,6 +571,59 @@
testRule(Rule, Input, Expected);
}
+// Verifies that a rule with a matcher for an implicit node (like
+// `implicitCastExpr`) does not change the code, when the traversal kind is not
+// explicitly set ti TK_AsIs). In this test, only the rule with the
+// explicit-node matcher will fire.
+TEST_F(TransformerTest, OrderedRuleImplicitIgnored) {
+ std::string Input = R"cc(
+ void f1();
+ int f2();
+ void call_f1() { f1(); }
+ float call_f2() { return f2(); }
+ )cc";
+ std::string Expected = R"cc(
+ void f1();
+ int f2();
+ void call_f1() { REPLACE_F1; }
+ float call_f2() { return f2(); }
+ )cc";
+
+ RewriteRule ReplaceF1 =
+ makeRule(callExpr(callee(functionDecl(hasName("f1")))),
+ changeTo(cat("REPLACE_F1")));
+ RewriteRule ReplaceF2 =
+ makeRule(implicitCastExpr(hasSourceExpression(callExpr())),
+ changeTo(cat("REPLACE_F2")));
+ testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
+// Verifies that explicitly setting the traversal kind fixes the problem in the
+// previous test.
+TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
+ std::string Input = R"cc(
+ void f1();
+ int f2();
+ void call_f1() { f1(); }
+ float call_f2() { return f2(); }
+ )cc";
+ std::string Expected = R"cc(
+ void f1();
+ int f2();
+ void call_f1() { REPLACE_F1; }
+ float call_f2() { return REPLACE_F2; }
+ )cc";
+
+ RewriteRule ReplaceF1 = makeRule(
+ traverse(clang::TK_AsIs, callExpr(callee(functionDecl(hasName("f1"))))),
+ changeTo(cat("REPLACE_F1")));
+ RewriteRule ReplaceF2 =
+ makeRule(traverse(clang::TK_AsIs,
+ implicitCastExpr(hasSourceExpression(callExpr()))),
+ changeTo(cat("REPLACE_F2")));
+ testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
//
// Negative tests (where we expect no transformation to occur).
//
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===================================================================
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -116,10 +116,13 @@
#endif
// Binds each rule's matcher to a unique (and deterministic) tag based on
-// `TagBase` and the id paired with the case.
+// `TagBase` and the id paired with the case. All of the returned matchers have
+// their traversal kind explicitly set, either based on a pre-set kind or taken
+// from `DefaultTraversalKind`.
static std::vector<DynTypedMatcher> taggedMatchers(
StringRef TagBase,
- const SmallVectorImpl<std::pair<size_t, RewriteRule::Case>> &Cases) {
+ const SmallVectorImpl<std::pair<size_t, RewriteRule::Case>> &Cases,
+ ast_type_traits::TraversalKind DefaultTraversalKind) {
std::vector<DynTypedMatcher> Matchers;
Matchers.reserve(Cases.size());
for (const auto &Case : Cases) {
@@ -127,8 +130,11 @@
// HACK: Many matchers are not bindable, so ensure that tryBind will work.
DynTypedMatcher BoundMatcher(Case.second.Matcher);
BoundMatcher.setAllowBind(true);
- auto M = BoundMatcher.tryBind(Tag);
- Matchers.push_back(*std::move(M));
+ auto M = *BoundMatcher.tryBind(Tag);
+ Matchers.push_back(!M.getTraversalKind()
+ ? DynTypedMatcher::constructWithTraversalKind(
+ std::move(M), DefaultTraversalKind)
+ : std::move(M));
}
return Matchers;
}
@@ -143,8 +149,9 @@
return R;
}
-std::vector<DynTypedMatcher>
-transformer::detail::buildMatchers(const RewriteRule &Rule) {
+std::vector<DynTypedMatcher> transformer::detail::buildMatchers(
+ const RewriteRule &Rule,
+ ast_type_traits::TraversalKind DefaultTraversalKind) {
// Map the cases into buckets of matchers -- one for each "root" AST kind,
// which guarantees that they can be combined in a single anyOf matcher. Each
// case is paired with an identifying number that is converted to a string id
@@ -162,10 +169,11 @@
for (const auto &Bucket : Buckets) {
DynTypedMatcher M = DynTypedMatcher::constructVariadic(
DynTypedMatcher::VO_AnyOf, Bucket.first,
- taggedMatchers("Tag", Bucket.second));
+ taggedMatchers("Tag", Bucket.second, DefaultTraversalKind));
M.setAllowBind(true);
// `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
- Matchers.push_back(*M.tryBind(RewriteRule::RootID));
+ Matchers.push_back(DynTypedMatcher::constructWithTraversalKind(
+ *M.tryBind(RewriteRule::RootID), TK_AsIs));
}
return Matchers;
}
Index: clang/include/clang/Tooling/Transformer/RewriteRule.h
===================================================================
--- clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -273,13 +273,18 @@
/// supports mixing matchers of different kinds.
ast_matchers::internal::DynTypedMatcher buildMatcher(const RewriteRule &Rule);
-/// Builds a set of matchers that cover the rule (one for each distinct node
-/// matcher base kind: Stmt, Decl, etc.). Node-matchers for `QualType` and
-/// `Type` are not permitted, since such nodes carry no source location
-/// information and are therefore not relevant for rewriting. If any such
-/// matchers are included, will return an empty vector.
+/// Builds a set of matchers that cover the rule.
+///
+/// One matcher is built for each distinct node matcher base kind: Stmt, Decl,
+/// etc. Node-matchers for `QualType` and `Type` are not permitted, since such
+/// nodes carry no source location information and are therefore not relevant
+/// for rewriting. If any such matchers are included, will return an empty
+/// vector. \c DefaultTraversalKind specifies how to interpret any matchers that
+/// don't have an explicit traversal kind set.
std::vector<ast_matchers::internal::DynTypedMatcher>
-buildMatchers(const RewriteRule &Rule);
+buildMatchers(const RewriteRule &Rule,
+ ast_type_traits::TraversalKind DefaultTraversalKind =
+ TK_IgnoreUnlessSpelledInSource);
/// Gets the beginning location of the source matched by a rewrite rule. If the
/// match occurs within a macro expansion, returns the beginning of the
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
