Hi Piotr,
Thank you for your detailed comments :)
I would love some help with the other fixit. I have some notes on it at
home. But my main catch is that is an implicit cast to boolean from
str.compare(str) with maybe an ! in front of it. And I need to fix that to
str.compare(str) == 0 or str.compare(str) != 0.
Where should I put the warning in a static const global variable? Is it
still in StringCompare.cpp or do we have a joint files for these?
Best regards,
Mads Ravn
On Sun, Dec 18, 2016 at 11:26 PM Piotr Padlewski via Phabricator <
revi...@reviews.llvm.org> wrote:
> Prazek added a comment.
>
> Do you need some help with implementing the other fixit, or you just need
> some extra time? It seems to be almost the same as your second fixit
>
>
>
> ================
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:69-70
> + diag(Matched->getLocStart(),
> + "do not use 'compare' to test equality of strings; "
> + "use the string equality operator instead");
> +
> ----------------
> Take this warning to some static const global variable
>
>
> ================
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:71
> + "use the string equality operator instead");
> +
> + if (const auto *Matched = Result.Nodes.getNodeAs<Stmt>("match2")) {
> ----------------
> match1 and match2 are in different matchers (passed to register matchers)?
>
> If so put return here after diag to finish control flow for this case.
>
>
> ================
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:81
> + auto Diag = diag(Matched->getLocStart(),
> + "do not use 'compare' to test equality of strings;
> "
> + "use the string equality operator instead");
> ----------------
> and use it here
>
>
> ================
> Comment at: clang-tidy/misc/StringCompareCheck.h:10-11
> +
> +#ifndef STRING_COMPARE_CHECK_H
> +#define STRING_COMPARE_CHECK_H
> +
> ----------------
> This should be much longer string. Do you know about ./add_new_check?
>
> Please make one similar to other checks
>
>
> ================
> Comment at: clang-tidy/misc/StringCompareCheck.h:36
> +
> +#endif // STRING_COMPARE_CHECK_H
> ----------------
> DITTO
>
>
> ================
> Comment at: test/clang-tidy/misc-string-compare.cpp:35-40
> + if (str1.compare(str2)) {
> + }
> + // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test
> equality of strings; use the string equality operator instead
> [misc-string-compare]
> + if (!str1.compare(str2)) {
> + }
> + // CHECK-MESSAGES: [[@LINE-2]]:8: warning: do not use 'compare' to test
> equality of strings; use the string equality operator instead
> [misc-string-compare]
> ----------------
> Why this one doesn't have fixit hint?
>
>
> ================
> Comment at: test/clang-tidy/misc-string-compare.cpp:70
> + }
> + // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test
> equality of strings;
> + if (str3->compare(str2) == 0) {
> ----------------
> no fixit?
>
>
> https://reviews.llvm.org/D27210
>
>
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
