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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D82126#2104088 <https://reviews.llvm.org/D82126#2104088>, @tdl-g wrote:

> 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?


Done.

> 
> 
> 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