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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
