Firstly, please respond in phabricator if it is possible. When you send
email it doesn't appear in phabricator, it's probably a bug.
2016-12-19 8:00 GMT+01:00 Mads Ravn <madsr...@gmail.com>:
> 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.
>
> Why fix it to something, that you will want to fix it again to str == str
and str != str?
I guess you just have to match parent of this expr is negation or anything,
bind negation to some name and then check if you matched to the negation or
not.
> 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?
>
> Yep, StringCompare.cpp, just like in other checks.
> 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
