[PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via cfe-commits]

Hi Malcolm,

Matching for the single parameter overload of compare is probably a good
idea. I will do that.

> Comment at: test/clang-tidy/misc-string-compare.cpp:9
> +  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test
equality of strings; use the string equality operator instead
[misc-string-compare]
What do you mean by this comment?

Best regards,
Mads Ravn

On Fri, Dec 2, 2016 at 10:26 AM Malcolm Parsons via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161202/933dac61/attachment.html>