[clang] d81d69f - [libTooling] Change Transformer's `cat` to handle some cases of text in macros.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2020-06-19T18:48:54Z
New Revision: d81d69f1c0c161e093531cf1ac9c1fa280ab5bf1

URL: https://github.com/llvm/llvm-project/commit/d81d69f1c0c161e093531cf1ac9c1fa280ab5bf1
DIFF: https://github.com/llvm/llvm-project/commit/d81d69f1c0c161e093531cf1ac9c1fa280ab5bf1.diff

LOG: [libTooling] Change Transformer's `cat` to handle some cases of text in macros.

Summary:
Currently, `cat` validates range selections before extracting the corresponding
source text. However, this means that any range inside a macro is rejected as an
error. This patch changes the implementation to first try to map the range to
something reasonable. This makes the behavior consistent with handling of ranges
used for selecting portions of the source to edit.

Also updates a clang-tidy lit-test for one of the checks which was affected by
this change.

Reviewers: gribozavr2, tdl-g

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82126

Added: 
    

Modified: 
    clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
    clang/lib/Tooling/Transformer/Stencil.cpp
    clang/unittests/Tooling/StencilTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp b/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
index 871c830b81cf..9d4b03aa22f2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
@@ -239,16 +239,12 @@ void no_char_param_tests() {
   asv.find('c') == absl::string_view::npos;
 }
 
-#define COMPARE_MACRO(x, y) ((x) == (y))
-#define FIND_MACRO(x, y) ((x).find(y))
-#define FIND_COMPARE_MACRO(x, y, z) ((x).find(y) == (z))
-
-// Confirms that it does not match when a macro is involved.
-void no_macros() {
-  std::string s;
-  COMPARE_MACRO(s.find("a"), std::string::npos);
-  FIND_MACRO(s, "a") == std::string::npos;
-  FIND_COMPARE_MACRO(s, "a", std::string::npos);
+#define FOO(a, b, c, d) ((a).find(b) == std::string::npos ? (c) : (d))
+
+// Confirms that it does not match when a macro would be "torn" by the fix.
+void no_tearing_macros() {
+  std::string h = "helo";
+  FOO(h, "x", 5, 6);
 }
 
 // Confirms that it does not match when the pos parameter is non-zero.

diff  --git a/clang/lib/Tooling/Transformer/Stencil.cpp b/clang/lib/Tooling/Transformer/Stencil.cpp
index da6033efa296..31b6ad9332c8 100644
--- a/clang/lib/Tooling/Transformer/Stencil.cpp
+++ b/clang/lib/Tooling/Transformer/Stencil.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Transformer/SourceCode.h"
 #include "clang/Tooling/Transformer/SourceCodeBuilders.h"
@@ -226,12 +227,14 @@ Error evalData(const UnaryOperationData &Data,
 
 Error evalData(const SelectorData &Data, const MatchFinder::MatchResult &Match,
                std::string *Result) {
-  auto Range = Data.Selector(Match);
-  if (!Range)
-    return Range.takeError();
-  if (auto Err = tooling::validateEditRange(*Range, *Match.SourceManager))
+  auto RawRange = Data.Selector(Match);
+  if (!RawRange)
+    return RawRange.takeError();
+  CharSourceRange Range = Lexer::makeFileCharRange(
+      *RawRange, *Match.SourceManager, Match.Context->getLangOpts());
+  if (auto Err = tooling::validateEditRange(Range, *Match.SourceManager))
     return Err;
-  *Result += tooling::getText(*Range, *Match.Context);
+  *Result += tooling::getText(Range, *Match.Context);
   return Error::success();
 }
 

diff  --git a/clang/unittests/Tooling/StencilTest.cpp b/clang/unittests/Tooling/StencilTest.cpp
index 2f0a970d8552..9408a1e62dfa 100644
--- a/clang/unittests/Tooling/StencilTest.cpp
+++ b/clang/unittests/Tooling/StencilTest.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Tooling/Transformer/Stencil.h"
+#include "clang/AST/ASTTypeTraits.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/FixIt.h"
 #include "clang/Tooling/Tooling.h"
@@ -373,6 +374,48 @@ TEST_F(StencilTest, RunOp) {
   testExpr(Id, "3;", run(SimpleFn), "Bound");
 }
 
+TEST_F(StencilTest, CatOfMacroRangeSucceeds) {
+  StringRef Snippet = R"cpp(
+#define MACRO 3.77
+  double foo(double d);
+  foo(MACRO);)cpp";
+
+  auto StmtMatch =
+      matchStmt(Snippet, callExpr(callee(functionDecl(hasName("foo"))),
+                                  argumentCountIs(1),
+                                  hasArgument(0, expr().bind("arg"))));
+  ASSERT_TRUE(StmtMatch);
+  Stencil S = cat(node("arg"));
+  EXPECT_THAT_EXPECTED(S->eval(StmtMatch->Result), HasValue("MACRO"));
+}
+
+TEST_F(StencilTest, CatOfMacroArgRangeSucceeds) {
+  StringRef Snippet = R"cpp(
+#define MACRO(a, b) a + b
+  MACRO(2, 3);)cpp";
+
+  auto StmtMatch =
+      matchStmt(Snippet, binaryOperator(hasRHS(expr().bind("rhs"))));
+  ASSERT_TRUE(StmtMatch);
+  Stencil S = cat(node("rhs"));
+  EXPECT_THAT_EXPECTED(S->eval(StmtMatch->Result), HasValue("3"));
+}
+
+TEST_F(StencilTest, CatOfMacroArgSubRangeSucceeds) {
+  StringRef Snippet = R"cpp(
+#define MACRO(a, b) a + b
+  int foo(int);
+  MACRO(2, foo(3));)cpp";
+
+  auto StmtMatch = matchStmt(
+      Snippet, binaryOperator(hasRHS(callExpr(
+                   callee(functionDecl(hasName("foo"))), argumentCountIs(1),
+                   hasArgument(0, expr().bind("arg"))))));
+  ASSERT_TRUE(StmtMatch);
+  Stencil S = cat(node("arg"));
+  EXPECT_THAT_EXPECTED(S->eval(StmtMatch->Result), HasValue("3"));
+}
+
 TEST_F(StencilTest, CatOfInvalidRangeFails) {
   StringRef Snippet = R"cpp(
 #define MACRO (3.77)