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 <
revi...@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
>
>
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
