ymandel updated this revision to Diff 217146.
ymandel added a comment.
fixed comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66676/new/
https://reviews.llvm.org/D66676
Files:
clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -18,18 +18,18 @@
namespace tidy {
namespace utils {
namespace {
+using namespace ::clang::ast_matchers;
+
using tooling::change;
using tooling::IncludeFormat;
+using tooling::node;
using tooling::RewriteRule;
+using tooling::statement;
using tooling::text;
using tooling::stencil::cat;
// Invert the code of an if-statement, while maintaining its semantics.
RewriteRule invertIf() {
- using namespace ::clang::ast_matchers;
- using tooling::node;
- using tooling::statement;
-
StringRef C = "C", T = "T", E = "E";
RewriteRule Rule = tooling::makeRule(
ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
@@ -67,6 +67,56 @@
EXPECT_EQ(Expected, test::runCheckOnCode<IfInverterCheck>(Input));
}
+class IntLitCheck : public TransformerClangTidyCheck {
+public:
+ IntLitCheck(StringRef Name, ClangTidyContext *Context)
+ : TransformerClangTidyCheck(tooling::makeRule(integerLiteral(),
+ change(text("LIT")),
+ text("no message")),
+ Name, Context) {}
+};
+
+// Tests that two changes in a single macro expansion do not lead to conflicts
+// in applying the changes.
+TEST(TransformerClangTidyCheckTest, TwoChangesInOneMacroExpansion) {
+ const std::string Input = R"cc(
+#define PLUS(a,b) (a) + (b)
+ int f() { return PLUS(3, 4); }
+ )cc";
+ const std::string Expected = R"cc(
+#define PLUS(a,b) (a) + (b)
+ int f() { return PLUS(LIT, LIT); }
+ )cc";
+
+ EXPECT_EQ(Expected, test::runCheckOnCode<IntLitCheck>(Input));
+}
+
+class BinOpCheck : public TransformerClangTidyCheck {
+public:
+ BinOpCheck(StringRef Name, ClangTidyContext *Context)
+ : TransformerClangTidyCheck(
+ tooling::makeRule(
+ binaryOperator(hasOperatorName("+"), hasRHS(expr().bind("r"))),
+ change(node("r"), text("RIGHT")), text("no message")),
+ Name, Context) {}
+};
+
+// Tests case where the rule's match spans both source from the macro and its
+// argument, while the change spans only the argument AND there are two such
+// matches. We verify that both replacements succeed.
+TEST(TransformerClangTidyCheckTest, TwoMatchesInMacroExpansion) {
+ const std::string Input = R"cc(
+#define M(a,b) (1 + a) * (1 + b)
+ int f() { return M(3, 4); }
+ )cc";
+ const std::string Expected = R"cc(
+#define M(a,b) (1 + a) * (1 + b)
+ int f() { return M(RIGHT, RIGHT); }
+ )cc";
+
+ EXPECT_EQ(Expected, test::runCheckOnCode<BinOpCheck>(Input));
+}
+
// A trivial rewrite-rule generator that requires Objective-C code.
Optional<RewriteRule> needsObjC(const LangOptions &LangOpts,
const ClangTidyCheck::OptionsView &Options) {
Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -71,14 +71,6 @@
if (Result.Context->getDiagnostics().hasErrorOccurred())
return;
- // Verify the existence and validity of the AST node that roots this rule.
- const ast_matchers::BoundNodes::IDToNodeMap &NodesMap = Result.Nodes.getMap();
- auto Root = NodesMap.find(RewriteRule::RootID);
- assert(Root != NodesMap.end() && "Transformation failed: missing root node.");
- SourceLocation RootLoc = Result.SourceManager->getExpansionLoc(
- Root->second.getSourceRange().getBegin());
- assert(RootLoc.isValid() && "Invalid location for Root node of match.");
-
assert(Rule && "check() should not fire if Rule is None");
RewriteRule::Case Case = tooling::detail::findSelectedCase(Result, *Rule);
Expected<SmallVector<tooling::detail::Transformation, 1>> Transformations =
@@ -99,10 +91,12 @@
<< llvm::toString(Explanation.takeError()) << "\n";
return;
}
- DiagnosticBuilder Diag = diag(RootLoc, *Explanation);
- for (const auto &T : *Transformations) {
+
+ // Associate the diagnostic with the location of the first change.
+ DiagnosticBuilder Diag =
+ diag((*Transformations)[0].Range.getBegin(), *Explanation);
+ for (const auto &T : *Transformations)
Diag << FixItHint::CreateReplacement(T.Range, T.Replacement);
- }
for (const auto &I : Case.AddedIncludes) {
auto &Header = I.first;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
