malcolm.parsons added inline comments.
================
Comment at: clang-tidy/misc/StringCompareCheck.cpp:25
+ callee(cxxMethodDecl(hasName("compare"),
+ ofClass(classTemplateSpecializationDecl(
+ hasName("::std::basic_string"))))),
----------------
This needs to check that it's one of the single parameter overloads of compare.
================
Comment at: clang-tidy/misc/StringCompareCheck.cpp:27
+ hasName("::std::basic_string"))))),
+ hasArgument(0, declRefExpr()), callee(memberExpr()));
+
----------------
I don't think you need to check what the first argument is.
================
Comment at: clang-tidy/misc/StringCompareCheck.cpp:39
+ binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+ hasEitherOperand(strCompare),
+
hasEitherOperand(integerLiteral(equals(0))))
----------------
Is this clang-formatted?
================
Comment at: test/clang-tidy/misc-string-compare.cpp:9
+
+ if(str1.compare(str2)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test
equality of strings; use the string equality operator instead
[misc-string-compare]
----------------
malcolm.parsons wrote:
> Some other test ideas:
>
> ```
> if (str1.compare("foo")) {}
>
> return str1.compare(str2) == 0;
>
> func(str1.compare(str2) != 0);
>
> if (str2.empty() || str1.compare(str2) != 0) {}
> ```
There's still no test for the single parameter c-string overload.
https://reviews.llvm.org/D27210
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
