<div dir="ltr">Hi Malcolm,<div><br></div><div>Matching for the single parameter overload of compare is probably a good idea. I will do that.</div><div><br></div><div>> <span style="color:rgb(117,117,117)">Comment at: test/clang-tidy/misc-string-co</span><span style="color:rgb(117,117,117)">mpare.cpp:9</span></div><div>> <span style="color:rgb(33,33,33)">+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test equality of strings; use the string equality operator instead [misc-string-compare]</span></div><div><span style="color:rgb(33,33,33)">What do you mean by this comment? </span></div><div><span style="color:rgb(33,33,33)"><br></span></div><div><span style="color:rgb(33,33,33)">Best regards,</span></div><div><span style="color:rgb(33,33,33)">Mads Ravn</span></div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Dec 2, 2016 at 10:26 AM Malcolm Parsons via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">malcolm.parsons added inline comments.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: clang-tidy/misc/StringCompareCheck.cpp:25<br class="gmail_msg">
+      callee(cxxMethodDecl(hasName("compare"),<br class="gmail_msg">
+                           ofClass(classTemplateSpecializationDecl(<br class="gmail_msg">
+                               hasName("::std::basic_string"))))),<br class="gmail_msg">
----------------<br class="gmail_msg">
This needs to check that it's one of the single parameter overloads of compare.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: clang-tidy/misc/StringCompareCheck.cpp:27<br class="gmail_msg">
+                               hasName("::std::basic_string"))))),<br class="gmail_msg">
+      hasArgument(0, declRefExpr()), callee(memberExpr()));<br class="gmail_msg">
+<br class="gmail_msg">
----------------<br class="gmail_msg">
I don't think you need to check what the first argument is.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: clang-tidy/misc/StringCompareCheck.cpp:39<br class="gmail_msg">
+      binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),<br class="gmail_msg">
+                                         hasEitherOperand(strCompare),<br class="gmail_msg">
+                                         hasEitherOperand(integerLiteral(equals(0))))<br class="gmail_msg">
----------------<br class="gmail_msg">
Is this clang-formatted?<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: test/clang-tidy/misc-string-compare.cpp:9<br class="gmail_msg">
+<br class="gmail_msg">
+  if(str1.compare(str2)) {}<br class="gmail_msg">
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test equality of strings; use the string equality operator instead [misc-string-compare]<br class="gmail_msg">
----------------<br class="gmail_msg">
malcolm.parsons wrote:<br class="gmail_msg">
> Some other test ideas:<br class="gmail_msg">
><br class="gmail_msg">
> ```<br class="gmail_msg">
> if (str1.compare("foo")) {}<br class="gmail_msg">
><br class="gmail_msg">
> return str1.compare(str2) == 0;<br class="gmail_msg">
><br class="gmail_msg">
> func(str1.compare(str2) != 0);<br class="gmail_msg">
><br class="gmail_msg">
> if (str2.empty() || str1.compare(str2) != 0) {}<br class="gmail_msg">
> ```<br class="gmail_msg">
There's still no test for the single parameter c-string overload.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D27210" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D27210</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div>