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

[Tom Lokovic via Phabricator via cfe-commits]

tdl-g added a comment.

Interesting, in all three of those cases, it's reasonable to replace the entire expression, thus eliminating the macro.  None of those "tear" the macro; if we had a case like

#define FOO(a,b,c,d) ((a).find(b) == std::string::npos ? (c) : (d))
FOO("helo", "x", 5, 6);

I guess in that case we'd want to suppress an edit change, since it would have to modify the macro to make the change.  But I guess all the existing test cases are actually safe to convert with the macro.

Do you want to remove the existing macro cases and add the "tearing" one above to confirm that it doesn't propose an edit?

In D82126#2103934 <https://reviews.llvm.org/D82126#2103934>, @ymandel wrote:

> Tests show that this breaks the test for the clang tidy `abseil-string-find-str-contains`.  Curious if this is a desirable change in behavior (in which case I'll update your test file) or the wrong behavior:
>
> https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp#L246-L252
>
>   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);
>   +  !absl::StrContains(s, "a");
>   +  !absl::StrContains(s, "a");
>   +  !absl::StrContains(s, "a");
>    }
>





Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82126/new/

https://reviews.llvm.org/D82126