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

Mads Ravn via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 1 08:42:47 PST 2016


Hi Alexander,

I have now implemented your suggestions - all but the fixit one. If I have
added bindings for str1 and str2 in ast matcher, how would I go about
creating a replacement for the entire implicitCastExpr or binaryOperator? I
can't find any example in the code for clang-tidy to suggest how I would
build a new node for the AST. Or I am looking at it from a wrong direction?

Could you hint me in the right direction as to how I would make the fixit?

Best regards,
Mads Ravn

On Thu, Dec 1, 2016 at 3:41 PM Alexander Kornienko via Phabricator <
reviews at reviews.llvm.org> wrote:

> alexfh requested changes to this revision.
> alexfh added inline comments.
> This revision now requires changes to proceed.
>
>
> ================
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:29
> +
> +  // First and second case: cast str.compare(str) to boolean
> +  Finder->addMatcher(
> ----------------
> Please add trailing periods here and elsewhere.
>
>
> ================
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:36-50
> +  // Third case: str.compare(str) == 0
> +  Finder->addMatcher(
> +      binaryOperator(hasOperatorName("=="),
> +                                         hasEitherOperand(strCompare),
> +
>  hasEitherOperand(integerLiteral(equals(0))))
> +          .bind("match"),
> +      this);
> ----------------
> These two matchers can be merged to avoid repetition.
>
>
> ================
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:55-57
> +    diag(Matched->getLocStart(),
> +         "do not use compare to test equality of strings; "
> +         "use the string equality operator instead");
> ----------------
> It looks like it's relatively straightforward to implement fixit hints.
> WDYT?
>
>
> ================
> Comment at: test/clang-tidy/misc-string-compare.cpp:29
> +  if(!str1.compare(str2)) {}
> +  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: do not use compare to test
> equality of strings; use the string equality operator instead
> [misc-string-compare]
> +  if(str1.compare(str2) == 0) {}
> ----------------
> Truncate all CHECK patterns after the first one after `of strings;`
>
>
> https://reviews.llvm.org/D27210
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161201/c38fc44c/attachment-0001.html>


More information about the cfe-commits mailing list