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