[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 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
[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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
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
