[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
https://github.com/PiotrZSL closed https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/89530 >From c6706922f31d4a7eedecb483346f99bffef67539 Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sun, 21 Apr 2024 05:17:19 + Subject: [PATCH] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() --- .../modernize/UseStartsEndsWithCheck.cpp | 103 +++--- .../modernize/UseStartsEndsWithCheck.h| 7 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../checks/modernize/use-starts-ends-with.rst | 8 +- .../clang-tidy/checkers/Inputs/Headers/string | 4 + .../checkers/Inputs/Headers/string.h | 1 + .../abseil/redundant-strcat-calls.cpp | 2 - .../modernize/use-starts-ends-with.cpp| 55 ++ 8 files changed, 162 insertions(+), 22 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 062f6e9911dbed..89ee45faecd7f3 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -43,7 +43,9 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("find")).bind("find_fun")), // ... on a class with a starts_with function. on(hasType( - hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction); + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // Bind search expression. + hasArgument(0, expr().bind("search_expr"))); const auto RFindExpr = cxxMemberCallExpr( // A method call with a second argument of zero... @@ -52,15 +54,68 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")), // ... on a class with a starts_with function. on(hasType( - hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction); + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // Bind search expression. + hasArgument(0, expr().bind("search_expr"))); + + // Match a string literal and an integer or strlen() call matching the length. + const auto HasStringLiteralAndLengthArgs = [](const auto StringArgIndex, +const auto LengthArgIndex) { +return allOf( +hasArgument(StringArgIndex, stringLiteral().bind("string_literal_arg")), +hasArgument(LengthArgIndex, +anyOf(integerLiteral().bind("integer_literal_size_arg"), + callExpr(callee(functionDecl(parameterCountIs(1), + hasName("strlen"))), + hasArgument(0, stringLiteral().bind( + "strlen_arg")); + }; + + // Match a string variable and a call to length() or size(). + const auto HasStringVariableAndSizeCallArgs = [](const auto StringArgIndex, + const auto LengthArgIndex) { +return allOf( +hasArgument(StringArgIndex, declRefExpr(hasDeclaration( +decl().bind("string_var_decl", +hasArgument(LengthArgIndex, +cxxMemberCallExpr( +callee(cxxMethodDecl(isConst(), parameterCountIs(0), + hasAnyName("size", "length"))), +on(declRefExpr( +to(decl(equalsBoundNode("string_var_decl"; + }; - const auto FindOrRFindExpr = - cxxMemberCallExpr(anyOf(FindExpr, RFindExpr)).bind("find_expr"); + // Match either one of the two cases above. + const auto HasStringAndLengthArgs = + [HasStringLiteralAndLengthArgs, HasStringVariableAndSizeCallArgs]( + const auto StringArgIndex, const auto LengthArgIndex) { +return anyOf( +HasStringLiteralAndLengthArgs(StringArgIndex, LengthArgIndex), +HasStringVariableAndSizeCallArgs(StringArgIndex, LengthArgIndex)); + }; + + const auto CompareExpr = cxxMemberCallExpr( + // A method call with three arguments... + argumentCountIs(3), + // ... where the first argument is zero... + hasArgument(0, ZeroLiteral), + // ... named compare... + callee(cxxMethodDecl(hasName("compare")).bind("find_fun")), + // ... on a class with a starts_with function... + on(hasType( + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // ... where the third argument is some string and the second a length. + HasStringAndLengthArgs(2, 1), + // Bind search expression. + hasArgument(2, expr().bind("search_expr"))); Finder->addMatcher( - // Match [=!]= with a zero on one side and a string.(r?)find on
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
@@ -3,15 +3,16 @@ modernize-use-starts-ends-with == -Checks whether a ``find`` or ``rfind`` result is compared with 0 and suggests -replacing with ``starts_with`` when the method exists in the class. Notably, +Checks for common roundabout ways to express `starts_with` and `ends_with` and nicovank wrote: Missed that one -- thanks. https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
@@ -3,15 +3,16 @@ modernize-use-starts-ends-with == -Checks whether a ``find`` or ``rfind`` result is compared with 0 and suggests -replacing with ``starts_with`` when the method exists in the class. Notably, +Checks for common roundabout ways to express `starts_with` and `ends_with` and EugeneZelenko wrote: ```suggestion Checks for common roundabout ways to express ``starts_with`` and ``ends_with`` and ``` Double back-ticks are used for language constructs, single back-ticks - for option names and values. https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
nicovank wrote: Got it, thanks! I'm somewhat familiar using InnerMatchers but didn't think that solution fit quite right here. This new version I think is much better. Kept matcher construction in `registerMatchers`, added a little bit of logic in `check` to make sure some sizes match. https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/89530 >From 98af89a36d4f112c3b31e450a5e6df8b6593aed4 Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sun, 21 Apr 2024 05:17:19 + Subject: [PATCH] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() --- .../modernize/UseStartsEndsWithCheck.cpp | 103 +++--- .../modernize/UseStartsEndsWithCheck.h| 4 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../checks/modernize/use-starts-ends-with.rst | 6 +- .../clang-tidy/checkers/Inputs/Headers/string | 4 + .../checkers/Inputs/Headers/string.h | 1 + .../abseil/redundant-strcat-calls.cpp | 2 - .../modernize/use-starts-ends-with.cpp| 55 ++ 8 files changed, 159 insertions(+), 20 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 062f6e9911dbed..89ee45faecd7f3 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -43,7 +43,9 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("find")).bind("find_fun")), // ... on a class with a starts_with function. on(hasType( - hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction); + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // Bind search expression. + hasArgument(0, expr().bind("search_expr"))); const auto RFindExpr = cxxMemberCallExpr( // A method call with a second argument of zero... @@ -52,15 +54,68 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")), // ... on a class with a starts_with function. on(hasType( - hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction); + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // Bind search expression. + hasArgument(0, expr().bind("search_expr"))); + + // Match a string literal and an integer or strlen() call matching the length. + const auto HasStringLiteralAndLengthArgs = [](const auto StringArgIndex, +const auto LengthArgIndex) { +return allOf( +hasArgument(StringArgIndex, stringLiteral().bind("string_literal_arg")), +hasArgument(LengthArgIndex, +anyOf(integerLiteral().bind("integer_literal_size_arg"), + callExpr(callee(functionDecl(parameterCountIs(1), + hasName("strlen"))), + hasArgument(0, stringLiteral().bind( + "strlen_arg")); + }; + + // Match a string variable and a call to length() or size(). + const auto HasStringVariableAndSizeCallArgs = [](const auto StringArgIndex, + const auto LengthArgIndex) { +return allOf( +hasArgument(StringArgIndex, declRefExpr(hasDeclaration( +decl().bind("string_var_decl", +hasArgument(LengthArgIndex, +cxxMemberCallExpr( +callee(cxxMethodDecl(isConst(), parameterCountIs(0), + hasAnyName("size", "length"))), +on(declRefExpr( +to(decl(equalsBoundNode("string_var_decl"; + }; - const auto FindOrRFindExpr = - cxxMemberCallExpr(anyOf(FindExpr, RFindExpr)).bind("find_expr"); + // Match either one of the two cases above. + const auto HasStringAndLengthArgs = + [HasStringLiteralAndLengthArgs, HasStringVariableAndSizeCallArgs]( + const auto StringArgIndex, const auto LengthArgIndex) { +return anyOf( +HasStringLiteralAndLengthArgs(StringArgIndex, LengthArgIndex), +HasStringVariableAndSizeCallArgs(StringArgIndex, LengthArgIndex)); + }; + + const auto CompareExpr = cxxMemberCallExpr( + // A method call with three arguments... + argumentCountIs(3), + // ... where the first argument is zero... + hasArgument(0, ZeroLiteral), + // ... named compare... + callee(cxxMethodDecl(hasName("compare")).bind("find_fun")), + // ... on a class with a starts_with function... + on(hasType( + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // ... where the third argument is some string and the second a length. + HasStringAndLengthArgs(2, 1), + // Bind search expression. + hasArgument(2, expr().bind("search_expr"))); Finder->addMatcher( - // Match [=!]= with a zero on one side and a string.(r?)find on
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
@@ -16,6 +16,75 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { +namespace { +// Given two argument indices X and Y, matches when a call expression has a +// string at index X with an expression representing that string's length at +// index Y. The string can be a string literal or a variable. The length can be +// matched via an integer literal or a call to strlen() in the case of a string +// literal, and by a call to size() or length() in the string variable case. +AST_POLYMORPHIC_MATCHER_P2(hasStringAndLengthArguments, + AST_POLYMORPHIC_SUPPORTED_TYPES( + CallExpr, CXXConstructExpr, + CXXUnresolvedConstructExpr, ObjCMessageExpr), + unsigned, StringArgIndex, unsigned, LengthArgIndex) { + if (StringArgIndex >= Node.getNumArgs() || + LengthArgIndex >= Node.getNumArgs()) { +return false; + } + + const Expr *StringArgExpr = + Node.getArg(StringArgIndex)->IgnoreParenImpCasts(); + const Expr *LengthArgExpr = + Node.getArg(LengthArgIndex)->IgnoreParenImpCasts(); + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match an integer literal equal to the string length or a call to strlen. + +static const auto Matcher = expr(anyOf( +integerLiteral().bind("integer_literal_size"), +callExpr(callee(functionDecl(hasName("strlen"))), argumentCountIs(1), + hasArgument(0, stringLiteral().bind("strlen_arg"); + +if (!Matcher.matches(*LengthArgExpr, Finder, Builder)) { + return false; +} + +return Builder->removeBindings( +[&](const ast_matchers::internal::BoundNodesMap &Nodes) { + const auto *IntegerLiteralSize = + Nodes.getNodeAs("integer_literal_size"); + const auto *StrlenArg = Nodes.getNodeAs("strlen_arg"); + if (IntegerLiteralSize) { +return IntegerLiteralSize->getValue().getZExtValue() != + StringArg->getLength(); + } + return StrlenArg->getLength() != StringArg->getLength(); +}); + } + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match a call to size() or length() on the same variable. + +static const auto Matcher = cxxMemberCallExpr( +on(declRefExpr(to(varDecl().bind("string_var_decl", +callee(cxxMethodDecl(hasAnyName("size", "length"), isConst(), + parameterCountIs(0; PiotrZSL wrote: instead of making them static, pass them to this matcher as two "InnerMatcher", check how other checks/matchers do that. Static matchers could make problems with object lifetime, and code like this is not allowed per codding standard. https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
nicovank wrote: Update following feedback. I rewrote `hasStringAndLengthArguments` to only build the matchers once (`static`), with necessary information being saved in variable bindings. @PiotrZSL This should be better, right? https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/89530 >From ff7ebb3086d5467685e54435f3eabe86c76c24b0 Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sun, 21 Apr 2024 05:17:19 + Subject: [PATCH] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() --- .../modernize/UseStartsEndsWithCheck.cpp | 118 +++--- .../modernize/UseStartsEndsWithCheck.h| 4 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../checks/modernize/use-starts-ends-with.rst | 6 +- .../clang-tidy/checkers/Inputs/Headers/string | 4 + .../checkers/Inputs/Headers/string.h | 1 + .../abseil/redundant-strcat-calls.cpp | 2 - .../modernize/use-starts-ends-with.cpp| 45 +++ 8 files changed, 163 insertions(+), 21 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 062f6e9911dbed..2374213911beb4 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -16,6 +16,75 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { +namespace { +// Given two argument indices X and Y, matches when a call expression has a +// string at index X with an expression representing that string's length at +// index Y. The string can be a string literal or a variable. The length can be +// matched via an integer literal or a call to strlen() in the case of a string +// literal, and by a call to size() or length() in the string variable case. +AST_POLYMORPHIC_MATCHER_P2(hasStringAndLengthArguments, + AST_POLYMORPHIC_SUPPORTED_TYPES( + CallExpr, CXXConstructExpr, + CXXUnresolvedConstructExpr, ObjCMessageExpr), + unsigned, StringArgIndex, unsigned, LengthArgIndex) { + if (StringArgIndex >= Node.getNumArgs() || + LengthArgIndex >= Node.getNumArgs()) { +return false; + } + + const Expr *StringArgExpr = + Node.getArg(StringArgIndex)->IgnoreParenImpCasts(); + const Expr *LengthArgExpr = + Node.getArg(LengthArgIndex)->IgnoreParenImpCasts(); + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match an integer literal equal to the string length or a call to strlen. + +static const auto Matcher = expr(anyOf( +integerLiteral().bind("integer_literal_size"), +callExpr(callee(functionDecl(hasName("strlen"))), argumentCountIs(1), + hasArgument(0, stringLiteral().bind("strlen_arg"); + +if (!Matcher.matches(*LengthArgExpr, Finder, Builder)) { + return false; +} + +return Builder->removeBindings( +[&](const ast_matchers::internal::BoundNodesMap &Nodes) { + const auto *IntegerLiteralSize = + Nodes.getNodeAs("integer_literal_size"); + const auto *StrlenArg = Nodes.getNodeAs("strlen_arg"); + if (IntegerLiteralSize) { +return IntegerLiteralSize->getValue().getZExtValue() != + StringArg->getLength(); + } + return StrlenArg->getLength() != StringArg->getLength(); +}); + } + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match a call to size() or length() on the same variable. + +static const auto Matcher = cxxMemberCallExpr( +on(declRefExpr(to(varDecl().bind("string_var_decl", +callee(cxxMethodDecl(hasAnyName("size", "length"), isConst(), + parameterCountIs(0; + +if (!Matcher.matches(*LengthArgExpr, Finder, Builder)) { + return false; +} + +return Builder->removeBindings( +[&](const ast_matchers::internal::BoundNodesMap &Nodes) { + const auto *StringVarDecl = + Nodes.getNodeAs("string_var_decl"); + return StringVarDecl != StringArg->getDecl(); +}); + } + + return false; +} +} // namespace UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name, ClangTidyContext *Context) @@ -43,7 +112,9 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("find")).bind("find_fun")), // ... on a class with a starts_with function. on(hasType( - hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction); + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // Bind search expression. + hasArgument(0, expr().bind("search_expr"))); const auto RFindExpr = cxxMemberCallExpr( // A method call with a second argument of zero... @@ -52,15 +123,30 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")), // ... on a class with a starts_wi
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
@@ -16,6 +16,49 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { +namespace { +// Given two argument indices X and Y, matches when a call expression has a +// string at index X with an expression representing that string's length at +// index Y. The string can be a string literal or a variable. The length can be +// matched via an integer literal or a call to strlen() in the case of a string +// literal, and by a call to size() or length() in the string variable case. +AST_POLYMORPHIC_MATCHER_P2(HasStringAndLengthArguments, + AST_POLYMORPHIC_SUPPORTED_TYPES( + CallExpr, CXXConstructExpr, + CXXUnresolvedConstructExpr, ObjCMessageExpr), + unsigned, StringArgIndex, unsigned, LengthArgIndex) { + if (StringArgIndex >= Node.getNumArgs() || + LengthArgIndex >= Node.getNumArgs()) { +return false; + } + + const Expr *StringArgExpr = + Node.getArg(StringArgIndex)->IgnoreParenImpCasts(); + const Expr *LengthArgExpr = + Node.getArg(LengthArgIndex)->IgnoreParenImpCasts(); + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match an integer literal equal to the string length or a call to strlen. +const auto Matcher = expr(anyOf( +integerLiteral(equals(StringArg->getLength())), +callExpr( +callee(functionDecl(hasName("strlen"))), argumentCountIs(1), +hasArgument(0, stringLiteral(hasSize(StringArg->getLength())); +return Matcher.matches(*LengthArgExpr, Finder, Builder); PiotrZSL wrote: i do not recommend constructing matcher in matcher, mainly due to performance reasons (constant construction of matcher). What you could do is to pass matcher to this matcher as "inner matcher", and then just call it here, or convert it to simple function. https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
@@ -16,6 +16,49 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { +namespace { +// Given two argument indices X and Y, matches when a call expression has a +// string at index X with an expression representing that string's length at +// index Y. The string can be a string literal or a variable. The length can be +// matched via an integer literal or a call to strlen() in the case of a string +// literal, and by a call to size() or length() in the string variable case. +AST_POLYMORPHIC_MATCHER_P2(HasStringAndLengthArguments, + AST_POLYMORPHIC_SUPPORTED_TYPES( + CallExpr, CXXConstructExpr, + CXXUnresolvedConstructExpr, ObjCMessageExpr), + unsigned, StringArgIndex, unsigned, LengthArgIndex) { + if (StringArgIndex >= Node.getNumArgs() || + LengthArgIndex >= Node.getNumArgs()) { +return false; + } + + const Expr *StringArgExpr = + Node.getArg(StringArgIndex)->IgnoreParenImpCasts(); + const Expr *LengthArgExpr = + Node.getArg(LengthArgIndex)->IgnoreParenImpCasts(); + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match an integer literal equal to the string length or a call to strlen. +const auto Matcher = expr(anyOf( +integerLiteral(equals(StringArg->getLength())), +callExpr( +callee(functionDecl(hasName("strlen"))), argumentCountIs(1), +hasArgument(0, stringLiteral(hasSize(StringArg->getLength())); +return Matcher.matches(*LengthArgExpr, Finder, Builder); PiotrZSL wrote: same in line 52 https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
@@ -298,6 +298,10 @@ Changes in existing checks check by resolving fix-it overlaps in template code by disregarding implicit instances. +- Improved :doc:`modernize-use-starts-ends-with + ` check to also handle + cases using `compare()`. PiotrZSL wrote: 'to also handle calls to ``compare`` method`, or something like that https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
https://github.com/PiotrZSL approved this pull request. Looks +- fine, I just worry a little bit about performance. But as "HasStringAndLengthArguments" will be executed only for an compare methods, then probably it could be fine. https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
@@ -16,6 +16,49 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { +namespace { +// Given two argument indices X and Y, matches when a call expression has a +// string at index X with an expression representing that string's length at +// index Y. The string can be a string literal or a variable. The length can be +// matched via an integer literal or a call to strlen() in the case of a string +// literal, and by a call to size() or length() in the string variable case. +AST_POLYMORPHIC_MATCHER_P2(HasStringAndLengthArguments, PiotrZSL wrote: HasStringAndLengthArguments -> hasStringAndLengthArguments https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
@@ -94,11 +155,12 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) { Diagnostic << FixItHint::CreateRemoval(CharSourceRange::getCharRange( ComparisonExpr->getBeginLoc(), FindExpr->getBeginLoc())); - // Replace '(r?)find' with 'starts_with'. + // Replace method name by 'starts_with'. + // Remove possible arguments before search expression. Diagnostic << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(FindExpr->getExprLoc(), - FindExpr->getExprLoc()), - StartsWithFunction->getName()); + CharSourceRange::getCharRange(FindExpr->getExprLoc(), +SearchExpr->getBeginLoc()), + StartsWithFunction->getNameAsString() + "("); PiotrZSL wrote: consider llvm::Twine for string merging. https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
@@ -298,6 +298,10 @@ Changes in existing checks check by resolving fix-it overlaps in template code by disregarding implicit instances. +- Improved :doc:`modernize-use-starts-ends-with EugeneZelenko wrote: Please keep alphabetical order in this section (by check name). https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
@@ -298,6 +298,10 @@ Changes in existing checks check by resolving fix-it overlaps in template code by disregarding implicit instances. +- Improved :doc:`modernize-use-starts-ends-with + ` check to also handle + cases using `compare()`. EugeneZelenko wrote: Please use double back-ticks for language constructs. https://github.com/llvm/llvm-project/pull/89530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/89530 >From 7a2ff84f113959bc89f50b6eef7ee9762e6f5d2f Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sun, 21 Apr 2024 05:17:19 + Subject: [PATCH] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() --- .../modernize/UseStartsEndsWithCheck.cpp | 92 --- .../modernize/UseStartsEndsWithCheck.h| 4 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../checks/modernize/use-starts-ends-with.rst | 6 +- .../clang-tidy/checkers/Inputs/Headers/string | 4 + .../checkers/Inputs/Headers/string.h | 1 + .../abseil/redundant-strcat-calls.cpp | 2 - .../modernize/use-starts-ends-with.cpp| 45 + 8 files changed, 137 insertions(+), 21 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 062f6e9911dbed..38fe1984ac494e 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -16,6 +16,49 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { +namespace { +// Given two argument indices X and Y, matches when a call expression has a +// string at index X with an expression representing that string's length at +// index Y. The string can be a string literal or a variable. The length can be +// matched via an integer literal or a call to strlen() in the case of a string +// literal, and by a call to size() or length() in the string variable case. +AST_POLYMORPHIC_MATCHER_P2(HasStringAndLengthArguments, + AST_POLYMORPHIC_SUPPORTED_TYPES( + CallExpr, CXXConstructExpr, + CXXUnresolvedConstructExpr, ObjCMessageExpr), + unsigned, StringArgIndex, unsigned, LengthArgIndex) { + if (StringArgIndex >= Node.getNumArgs() || + LengthArgIndex >= Node.getNumArgs()) { +return false; + } + + const Expr *StringArgExpr = + Node.getArg(StringArgIndex)->IgnoreParenImpCasts(); + const Expr *LengthArgExpr = + Node.getArg(LengthArgIndex)->IgnoreParenImpCasts(); + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match an integer literal equal to the string length or a call to strlen. +const auto Matcher = expr(anyOf( +integerLiteral(equals(StringArg->getLength())), +callExpr( +callee(functionDecl(hasName("strlen"))), argumentCountIs(1), +hasArgument(0, stringLiteral(hasSize(StringArg->getLength())); +return Matcher.matches(*LengthArgExpr, Finder, Builder); + } + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match a call to size() or length() on the same variable. +const auto Matcher = cxxMemberCallExpr( +on(declRefExpr(to(varDecl(equalsNode(StringArg->getDecl()), +callee(cxxMethodDecl(hasAnyName("size", "length"), isConst(), + parameterCountIs(0; +return Matcher.matches(*LengthArgExpr, Finder, Builder); + } + + return false; +} +} // namespace UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name, ClangTidyContext *Context) @@ -43,7 +86,9 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("find")).bind("find_fun")), // ... on a class with a starts_with function. on(hasType( - hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction); + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // Bind search expression. + hasArgument(0, expr().bind("search_expr"))); const auto RFindExpr = cxxMemberCallExpr( // A method call with a second argument of zero... @@ -52,15 +97,30 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")), // ... on a class with a starts_with function. on(hasType( - hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction); - - const auto FindOrRFindExpr = - cxxMemberCallExpr(anyOf(FindExpr, RFindExpr)).bind("find_expr"); + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // Bind search expression. + hasArgument(0, expr().bind("search_expr"))); + + const auto CompareExpr = cxxMemberCallExpr( + // A method call with a first argument of zero... + hasArgument(0, ZeroLiteral), + // ... named compare... + callee(cxxMethodDecl(hasName("compare")).bind("find_fun")), + // ... on a class with a starts_with function... + on(hasType( + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // ... where the third argument is some string and the second
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/89530 >From 5f20627f74103d3b2b5adf484c902b85228006dd Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sun, 21 Apr 2024 05:17:19 + Subject: [PATCH] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() --- .../modernize/UseStartsEndsWithCheck.cpp | 92 --- .../modernize/UseStartsEndsWithCheck.h| 4 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../checks/modernize/use-starts-ends-with.rst | 6 +- .../clang-tidy/checkers/Inputs/Headers/string | 4 + .../checkers/Inputs/Headers/string.h | 1 + .../modernize/use-starts-ends-with.cpp| 45 + 7 files changed, 137 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 062f6e9911dbed..38fe1984ac494e 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -16,6 +16,49 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { +namespace { +// Given two argument indices X and Y, matches when a call expression has a +// string at index X with an expression representing that string's length at +// index Y. The string can be a string literal or a variable. The length can be +// matched via an integer literal or a call to strlen() in the case of a string +// literal, and by a call to size() or length() in the string variable case. +AST_POLYMORPHIC_MATCHER_P2(HasStringAndLengthArguments, + AST_POLYMORPHIC_SUPPORTED_TYPES( + CallExpr, CXXConstructExpr, + CXXUnresolvedConstructExpr, ObjCMessageExpr), + unsigned, StringArgIndex, unsigned, LengthArgIndex) { + if (StringArgIndex >= Node.getNumArgs() || + LengthArgIndex >= Node.getNumArgs()) { +return false; + } + + const Expr *StringArgExpr = + Node.getArg(StringArgIndex)->IgnoreParenImpCasts(); + const Expr *LengthArgExpr = + Node.getArg(LengthArgIndex)->IgnoreParenImpCasts(); + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match an integer literal equal to the string length or a call to strlen. +const auto Matcher = expr(anyOf( +integerLiteral(equals(StringArg->getLength())), +callExpr( +callee(functionDecl(hasName("strlen"))), argumentCountIs(1), +hasArgument(0, stringLiteral(hasSize(StringArg->getLength())); +return Matcher.matches(*LengthArgExpr, Finder, Builder); + } + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match a call to size() or length() on the same variable. +const auto Matcher = cxxMemberCallExpr( +on(declRefExpr(to(varDecl(equalsNode(StringArg->getDecl()), +callee(cxxMethodDecl(hasAnyName("size", "length"), isConst(), + parameterCountIs(0; +return Matcher.matches(*LengthArgExpr, Finder, Builder); + } + + return false; +} +} // namespace UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name, ClangTidyContext *Context) @@ -43,7 +86,9 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("find")).bind("find_fun")), // ... on a class with a starts_with function. on(hasType( - hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction); + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // Bind search expression. + hasArgument(0, expr().bind("search_expr"))); const auto RFindExpr = cxxMemberCallExpr( // A method call with a second argument of zero... @@ -52,15 +97,30 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")), // ... on a class with a starts_with function. on(hasType( - hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction); - - const auto FindOrRFindExpr = - cxxMemberCallExpr(anyOf(FindExpr, RFindExpr)).bind("find_expr"); + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // Bind search expression. + hasArgument(0, expr().bind("search_expr"))); + + const auto CompareExpr = cxxMemberCallExpr( + // A method call with a first argument of zero... + hasArgument(0, ZeroLiteral), + // ... named compare... + callee(cxxMethodDecl(hasName("compare")).bind("find_fun")), + // ... on a class with a starts_with function... + on(hasType( + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // ... where the third argument is some string and the second its length. + HasStringAndLengthArguments(2, 1),
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
llvmbot wrote: @llvm/pr-subscribers-clang-tidy Author: Nicolas van Kempen (nicovank) Changes Using `compare` is the next most common roundabout way to express `starts_with` before it was added to the standard. In this case, using `starts_with` is a readability improvement. Extend existing `modernize-use-starts-ends-with` to cover this case. ``` // The following will now be replaced by starts_with(). string.compare(0, strlen("prefix"), "prefix") == 0; string.compare(0, 6, "prefix") == 0; string.compare(0, prefix.length(), prefix) == 0; string.compare(0, prefix.size(), prefix) == 0; ``` There are no such instances in llvm-project, maybe more will surface when the C++ default is changed to 20 for `std::string`. Other build issues come up when trying to override it. Running this on llvm-project and dolphin: - https://github.com/llvm/llvm-project/pull/89140 (no additional instances) - https://github.com/dolphin-emu/dolphin/pull/12718 --- Full diff: https://github.com/llvm/llvm-project/pull/89530.diff 7 Files Affected: - (modified) clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp (+77-15) - (modified) clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h (+2-2) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) - (modified) clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst (+4-2) - (modified) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string (+4) - (modified) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string.h (+1) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp (+45) ``diff diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 062f6e9911dbed..38fe1984ac494e 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -16,6 +16,49 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { +namespace { +// Given two argument indices X and Y, matches when a call expression has a +// string at index X with an expression representing that string's length at +// index Y. The string can be a string literal or a variable. The length can be +// matched via an integer literal or a call to strlen() in the case of a string +// literal, and by a call to size() or length() in the string variable case. +AST_POLYMORPHIC_MATCHER_P2(HasStringAndLengthArguments, + AST_POLYMORPHIC_SUPPORTED_TYPES( + CallExpr, CXXConstructExpr, + CXXUnresolvedConstructExpr, ObjCMessageExpr), + unsigned, StringArgIndex, unsigned, LengthArgIndex) { + if (StringArgIndex >= Node.getNumArgs() || + LengthArgIndex >= Node.getNumArgs()) { +return false; + } + + const Expr *StringArgExpr = + Node.getArg(StringArgIndex)->IgnoreParenImpCasts(); + const Expr *LengthArgExpr = + Node.getArg(LengthArgIndex)->IgnoreParenImpCasts(); + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match an integer literal equal to the string length or a call to strlen. +const auto Matcher = expr(anyOf( +integerLiteral(equals(StringArg->getLength())), +callExpr( +callee(functionDecl(hasName("strlen"))), argumentCountIs(1), +hasArgument(0, stringLiteral(hasSize(StringArg->getLength())); +return Matcher.matches(*LengthArgExpr, Finder, Builder); + } + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match a call to size() or length() on the same variable. +const auto Matcher = cxxMemberCallExpr( +on(declRefExpr(to(varDecl(equalsNode(StringArg->getDecl()), +callee(cxxMethodDecl(hasAnyName("size", "length"), isConst(), + parameterCountIs(0; +return Matcher.matches(*LengthArgExpr, Finder, Builder); + } + + return false; +} +} // namespace UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name, ClangTidyContext *Context) @@ -43,7 +86,9 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("find")).bind("find_fun")), // ... on a class with a starts_with function. on(hasType( - hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction); + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // Bind search expression. + hasArgument(0, expr().bind("search_expr"))); const auto RFindExpr = cxxMemberCallExpr( // A method call with a second argument of zero... @@ -52,15 +97,30 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")), // ... on a class with a
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
llvmbot wrote: @llvm/pr-subscribers-clang-tools-extra Author: Nicolas van Kempen (nicovank) Changes Using `compare` is the next most common roundabout way to express `starts_with` before it was added to the standard. In this case, using `starts_with` is a readability improvement. Extend existing `modernize-use-starts-ends-with` to cover this case. ``` // The following will now be replaced by starts_with(). string.compare(0, strlen("prefix"), "prefix") == 0; string.compare(0, 6, "prefix") == 0; string.compare(0, prefix.length(), prefix) == 0; string.compare(0, prefix.size(), prefix) == 0; ``` There are no such instances in llvm-project, maybe more will surface when the C++ default is changed to 20 for `std::string`. Other build issues come up when trying to override it. Running this on llvm-project and dolphin: - https://github.com/llvm/llvm-project/pull/89140 (no additional instances) - https://github.com/dolphin-emu/dolphin/pull/12718 --- Full diff: https://github.com/llvm/llvm-project/pull/89530.diff 7 Files Affected: - (modified) clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp (+77-15) - (modified) clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h (+2-2) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) - (modified) clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst (+4-2) - (modified) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string (+4) - (modified) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string.h (+1) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp (+45) ``diff diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 062f6e9911dbed..38fe1984ac494e 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -16,6 +16,49 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { +namespace { +// Given two argument indices X and Y, matches when a call expression has a +// string at index X with an expression representing that string's length at +// index Y. The string can be a string literal or a variable. The length can be +// matched via an integer literal or a call to strlen() in the case of a string +// literal, and by a call to size() or length() in the string variable case. +AST_POLYMORPHIC_MATCHER_P2(HasStringAndLengthArguments, + AST_POLYMORPHIC_SUPPORTED_TYPES( + CallExpr, CXXConstructExpr, + CXXUnresolvedConstructExpr, ObjCMessageExpr), + unsigned, StringArgIndex, unsigned, LengthArgIndex) { + if (StringArgIndex >= Node.getNumArgs() || + LengthArgIndex >= Node.getNumArgs()) { +return false; + } + + const Expr *StringArgExpr = + Node.getArg(StringArgIndex)->IgnoreParenImpCasts(); + const Expr *LengthArgExpr = + Node.getArg(LengthArgIndex)->IgnoreParenImpCasts(); + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match an integer literal equal to the string length or a call to strlen. +const auto Matcher = expr(anyOf( +integerLiteral(equals(StringArg->getLength())), +callExpr( +callee(functionDecl(hasName("strlen"))), argumentCountIs(1), +hasArgument(0, stringLiteral(hasSize(StringArg->getLength())); +return Matcher.matches(*LengthArgExpr, Finder, Builder); + } + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match a call to size() or length() on the same variable. +const auto Matcher = cxxMemberCallExpr( +on(declRefExpr(to(varDecl(equalsNode(StringArg->getDecl()), +callee(cxxMethodDecl(hasAnyName("size", "length"), isConst(), + parameterCountIs(0; +return Matcher.matches(*LengthArgExpr, Finder, Builder); + } + + return false; +} +} // namespace UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name, ClangTidyContext *Context) @@ -43,7 +86,9 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("find")).bind("find_fun")), // ... on a class with a starts_with function. on(hasType( - hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction); + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // Bind search expression. + hasArgument(0, expr().bind("search_expr"))); const auto RFindExpr = cxxMemberCallExpr( // A method call with a second argument of zero... @@ -52,15 +97,30 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")), // ... on a class
[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() (PR #89530)
https://github.com/nicovank created https://github.com/llvm/llvm-project/pull/89530 Using `compare` is the next most common roundabout way to express `starts_with` before it was added to the standard. In this case, using `starts_with` is a readability improvement. Extend existing `modernize-use-starts-ends-with` to cover this case. ``` // The following will now be replaced by starts_with(). string.compare(0, strlen("prefix"), "prefix") == 0; string.compare(0, 6, "prefix") == 0; string.compare(0, prefix.length(), prefix) == 0; string.compare(0, prefix.size(), prefix) == 0; ``` There are no such instances in llvm-project, maybe more will surface when the C++ default is changed to 20 for `std::string`. Other build issues come up when trying to override it. Running this on llvm-project and dolphin: - https://github.com/llvm/llvm-project/pull/89140 (no additional instances) - https://github.com/dolphin-emu/dolphin/pull/12718 >From decc4b58c4d899e619ea63c50fa873c7bc605baf Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sun, 21 Apr 2024 05:17:19 + Subject: [PATCH] [clang-tidy][modernize-use-starts-ends-with] Add support for compare() --- .../modernize/UseStartsEndsWithCheck.cpp | 92 --- .../modernize/UseStartsEndsWithCheck.h| 4 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../checks/modernize/use-starts-ends-with.rst | 6 +- .../clang-tidy/checkers/Inputs/Headers/string | 4 + .../checkers/Inputs/Headers/string.h | 1 + .../modernize/use-starts-ends-with.cpp| 45 + 7 files changed, 137 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 062f6e9911dbed..38fe1984ac494e 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -16,6 +16,49 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { +namespace { +// Given two argument indices X and Y, matches when a call expression has a +// string at index X with an expression representing that string's length at +// index Y. The string can be a string literal or a variable. The length can be +// matched via an integer literal or a call to strlen() in the case of a string +// literal, and by a call to size() or length() in the string variable case. +AST_POLYMORPHIC_MATCHER_P2(HasStringAndLengthArguments, + AST_POLYMORPHIC_SUPPORTED_TYPES( + CallExpr, CXXConstructExpr, + CXXUnresolvedConstructExpr, ObjCMessageExpr), + unsigned, StringArgIndex, unsigned, LengthArgIndex) { + if (StringArgIndex >= Node.getNumArgs() || + LengthArgIndex >= Node.getNumArgs()) { +return false; + } + + const Expr *StringArgExpr = + Node.getArg(StringArgIndex)->IgnoreParenImpCasts(); + const Expr *LengthArgExpr = + Node.getArg(LengthArgIndex)->IgnoreParenImpCasts(); + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match an integer literal equal to the string length or a call to strlen. +const auto Matcher = expr(anyOf( +integerLiteral(equals(StringArg->getLength())), +callExpr( +callee(functionDecl(hasName("strlen"))), argumentCountIs(1), +hasArgument(0, stringLiteral(hasSize(StringArg->getLength())); +return Matcher.matches(*LengthArgExpr, Finder, Builder); + } + + if (const auto *StringArg = dyn_cast(StringArgExpr)) { +// Match a call to size() or length() on the same variable. +const auto Matcher = cxxMemberCallExpr( +on(declRefExpr(to(varDecl(equalsNode(StringArg->getDecl()), +callee(cxxMethodDecl(hasAnyName("size", "length"), isConst(), + parameterCountIs(0; +return Matcher.matches(*LengthArgExpr, Finder, Builder); + } + + return false; +} +} // namespace UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name, ClangTidyContext *Context) @@ -43,7 +86,9 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("find")).bind("find_fun")), // ... on a class with a starts_with function. on(hasType( - hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction); + hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction, + // Bind search expression. + hasArgument(0, expr().bind("search_expr"))); const auto RFindExpr = cxxMemberCallExpr( // A method call with a second argument of zero... @@ -52,15 +97,30 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")), // ... on a class with a starts_with function. on(hasTy