[clang-tidy] Add new modernize-use-starts-ends-with check #72385
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
nicovank
force-pushed
the
modernize-string-find-startswith
branch
from
5e5a2ba to
58e1392
Compare
|
@llvm/pr-subscribers-clang-tidy Author: Nicolas van Kempen (nicovank) ChangesMatchers are copied over from abseil-string-find-startswith, only the error message is different and suggests Full diff: https://github.com/llvm/llvm-project/pull/72385.diff 11 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.h b/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.h
index 923b5caece5439b..5019b7000f1d062 100644
--- a/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.h
+++ b/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.h
@@ -21,7 +21,6 @@ namespace clang::tidy::abseil {
// Find string.find(...) == 0 comparisons and suggest replacing with StartsWith.
// FIXME(niko): Add similar check for EndsWith
-// FIXME(niko): Add equivalent modernize checks for C++20's std::starts_With
class StringFindStartswithCheck : public ClangTidyCheck {
public:
using ClangTidyCheck::ClangTidyCheck;
@@ -31,6 +30,10 @@ class StringFindStartswithCheck : public ClangTidyCheck {
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ // Prefer modernize-string-find-startswith when C++20 is available.
+ return LangOpts.CPlusPlus && !LangOpts.CPlusPlus20;
+ }
private:
const std::vector<StringRef> StringLikeClasses;
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index 717c400c4790330..541f58304119856 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -25,6 +25,7 @@ add_clang_library(clangTidyModernizeModule
ReplaceRandomShuffleCheck.cpp
ReturnBracedInitListCheck.cpp
ShrinkToFitCheck.cpp
+ StringFindStartswithCheck.cpp
TypeTraitsCheck.cpp
UnaryStaticAssertCheck.cpp
UseAutoCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index 73751cf2705068d..1fcbff79ddc6f96 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -26,6 +26,7 @@
#include "ReplaceRandomShuffleCheck.h"
#include "ReturnBracedInitListCheck.h"
#include "ShrinkToFitCheck.h"
+#include "StringFindStartswithCheck.h"
#include "TypeTraitsCheck.h"
#include "UnaryStaticAssertCheck.h"
#include "UseAutoCheck.h"
@@ -66,6 +67,8 @@ class ModernizeModule : public ClangTidyModule {
CheckFactories.registerCheck<MakeSharedCheck>("modernize-make-shared");
CheckFactories.registerCheck<MakeUniqueCheck>("modernize-make-unique");
CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
+ CheckFactories.registerCheck<StringFindStartswithCheck>(
+ "modernize-string-find-startswith");
CheckFactories.registerCheck<UseStdPrintCheck>("modernize-use-std-print");
CheckFactories.registerCheck<RawStringLiteralCheck>(
"modernize-raw-string-literal");
diff --git a/clang-tools-extra/clang-tidy/modernize/StringFindStartswithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/StringFindStartswithCheck.cpp
new file mode 100644
index 000000000000000..74f59b9fcea1982
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/StringFindStartswithCheck.cpp
@@ -0,0 +1,111 @@
+//===--- StringFindStartswithCheck.cpp - clang-tidy -----------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "StringFindStartswithCheck.h"
+
+#include "../utils/OptionsUtils.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+const auto DefaultStringLikeClasses =
+ "::std::basic_string;::std::basic_string_view";
+
+StringFindStartswithCheck::StringFindStartswithCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ StringLikeClasses(utils::options::parseStringList(
+ Options.get("StringLikeClasses", DefaultStringLikeClasses))) {}
+
+void StringFindStartswithCheck::registerMatchers(MatchFinder *Finder) {
+ const auto ZeroLiteral = integerLiteral(equals(0));
+ const auto StringClassMatcher = cxxRecordDecl(hasAnyName(StringLikeClasses));
+ const auto StringType = hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(StringClassMatcher)));
+
+ const auto StringFind = cxxMemberCallExpr(
+ // .find()-call on a string...
+ callee(cxxMethodDecl(hasName("find")).bind("findfun")),
+ on(hasType(StringType)),
+ // ... with some search expression ...
+ hasArgument(0, expr().bind("needle")),
+ // ... and either "0" as second argument or the default argument (also 0).
+ anyOf(hasArgument(1, ZeroLiteral), hasArgument(1, cxxDefaultArgExpr())));
+
+ Finder->addMatcher(
+ // Match [=!]= with a zero on one side and a string.find on the other.
+ binaryOperator(
+ hasAnyOperatorName("==", "!="),
+ hasOperands(ignoringParenImpCasts(ZeroLiteral),
+ ignoringParenImpCasts(StringFind.bind("findexpr"))))
+ .bind("expr"),
+ this);
+
+ const auto StringRFind = cxxMemberCallExpr(
+ // .rfind()-call on a string...
+ callee(cxxMethodDecl(hasName("rfind")).bind("findfun")),
+ on(hasType(StringType)),
+ // ... with some search expression ...
+ hasArgument(0, expr().bind("needle")),
+ // ... and "0" as second argument.
+ hasArgument(1, ZeroLiteral));
+
+ Finder->addMatcher(
+ // Match [=!]= with a zero on one side and a string.rfind on the other.
+ binaryOperator(
+ hasAnyOperatorName("==", "!="),
+ hasOperands(ignoringParenImpCasts(ZeroLiteral),
+ ignoringParenImpCasts(StringRFind.bind("findexpr"))))
+ .bind("expr"),
+ this);
+}
+
+void StringFindStartswithCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto &Context = *Result.Context;
+ const auto &Source = Context.getSourceManager();
+
+ const auto *ComparisonExpr = Result.Nodes.getNodeAs<BinaryOperator>("expr");
+ const auto *Needle = Result.Nodes.getNodeAs<Expr>("needle");
+ const auto *Haystack = Result.Nodes.getNodeAs<CXXMemberCallExpr>("findexpr")
+ ->getImplicitObjectArgument();
+ const auto *FindFun = Result.Nodes.getNodeAs<CXXMethodDecl>("findfun");
+
+ if (ComparisonExpr->getBeginLoc().isMacroID()) {
+ return;
+ }
+
+ const auto Rev = FindFun->getName().contains("rfind");
+ const auto Neg = ComparisonExpr->getOpcode() == BO_NE;
+
+ const auto NeedleExprCode = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Needle->getSourceRange()), Source,
+ Context.getLangOpts());
+ const auto HaystackExprCode = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Haystack->getSourceRange()), Source,
+ Context.getLangOpts());
+
+ const auto ReplacementCode = (Neg ? "!" : "") + HaystackExprCode +
+ ".starts_with(" + NeedleExprCode + ")";
+
+ diag(ComparisonExpr->getBeginLoc(),
+ "use starts_with "
+ "instead of %select{find()|rfind()}0 %select{==|!=}1 0")
+ << Rev << Neg
+ << FixItHint::CreateReplacement(ComparisonExpr->getSourceRange(),
+ ReplacementCode.str());
+}
+
+void StringFindStartswithCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StringLikeClasses",
+ utils::options::serializeStringList(StringLikeClasses));
+}
+
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/StringFindStartswithCheck.h b/clang-tools-extra/clang-tidy/modernize/StringFindStartswithCheck.h
new file mode 100644
index 000000000000000..fc12749de82cb3c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/StringFindStartswithCheck.h
@@ -0,0 +1,41 @@
+//===--- StringFindStartswithCheck.h - clang-tidy ---------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_STRINGFINDSTARTSWITHCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_STRINGFINDSTARTSWITHCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+#include <vector>
+
+namespace clang::tidy::modernize {
+
+/// Checks whether a ``std::string::find()`` or ``std::string::rfind()`` (and
+/// corresponding ``std::string_view`` methods) result is compared with 0, and
+/// suggests replacing with ``starts_with()``. This is both a readability and a
+/// performance issue.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/string-find-startswith.html
+class StringFindStartswithCheck : public ClangTidyCheck {
+public:
+ StringFindStartswithCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus20;
+ }
+
+private:
+ const std::vector<StringRef> StringLikeClasses;
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_STRINGFINDSTARTSWITHCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 353c6fe20269274..8f4ab856bf0a696 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -181,6 +181,14 @@ New checks
points in a coroutine. Such hostile types include scoped-lockable types and
types belonging to a configurable denylist.
+- New :doc:`modernize-string-find-startswith
+ <clang-tidy/checks/modernize/string-find-startswith>` check.
+
+ Checks whether a ``std::string::find()`` or ``std::string::rfind()`` (and
+ corresponding ``std::string_view`` methods) result is compared with 0, and
+ suggests replacing with ``starts_with()``. This is both a readability and a
+ performance issue.
+
- New :doc:`modernize-use-constraints
<clang-tidy/checks/modernize/use-constraints>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/abseil/string-find-startswith.rst b/clang-tools-extra/docs/clang-tidy/checks/abseil/string-find-startswith.rst
index c82c38772a5c9a8..a22192a37f07818 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/abseil/string-find-startswith.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/abseil/string-find-startswith.rst
@@ -8,6 +8,10 @@ corresponding ``std::string_view`` methods) result is compared with 0, and
suggests replacing with ``absl::StartsWith()``. This is both a readability and
performance issue.
+``starts_with`` was added as a built-in function on those types in C++20. If
+available, prefer enabling modernize-string-find-startswith instead of this
+check.
+
.. code-block:: c++
string s = "...";
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 6f987ba1672e3f2..ecf5c38a62d1a39 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -279,6 +279,7 @@ Clang-Tidy Checks
:doc:`modernize-replace-random-shuffle <modernize/replace-random-shuffle>`, "Yes"
:doc:`modernize-return-braced-init-list <modernize/return-braced-init-list>`, "Yes"
:doc:`modernize-shrink-to-fit <modernize/shrink-to-fit>`, "Yes"
+ :doc:`modernize-string-find-startswith <modernize/string-find-startswith>`, "Yes"
:doc:`modernize-type-traits <modernize/type-traits>`, "Yes"
:doc:`modernize-unary-static-assert <modernize/unary-static-assert>`, "Yes"
:doc:`modernize-use-auto <modernize/use-auto>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/string-find-startswith.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/string-find-startswith.rst
new file mode 100644
index 000000000000000..997703dd59c3b3c
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/string-find-startswith.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - modernize-string-find-startswith
+
+modernize-string-find-startswith
+================================
+
+Checks whether a ``std::string::find()`` or ``std::string::rfind()`` (and
+corresponding ``std::string_view`` methods) result is compared with 0, and
+suggests replacing with ``starts_with()``. This is both a readability and a
+performance issue.
+
+.. code-block:: c++
+
+ string s = "...";
+ if (s.find("Hello World") == 0) { /* do something */ }
+ if (s.rfind("Hello World", 0) == 0) { /* do something */ }
+
+becomes
+
+.. code-block:: c++
+
+ string s = "...";
+ if (s.starts_with("Hello World")) { /* do something */ }
+ if (s.starts_with("Hello World")) { /* do something */ }
+
+
+Options
+-------
+
+.. option:: StringLikeClasses
+
+ Semicolon-separated list of names of string-like classes. By default both
+ ``std::basic_string`` and ``std::basic_string_view`` are considered.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/abseil/string-find-startswith.cpp b/clang-tools-extra/test/clang-tidy/checkers/abseil/string-find-startswith.cpp
index 417598790bc007f..aabb30fe34f782c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/abseil/string-find-startswith.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/abseil/string-find-startswith.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s abseil-string-find-startswith %t -- \
+// RUN: %check_clang_tidy -std=c++17 %s abseil-string-find-startswith %t -- \
// RUN: -config="{CheckOptions: \
// RUN: {abseil-string-find-startswith.StringLikeClasses: \
// RUN: '::std::basic_string;::std::basic_string_view;::basic_string'}}" \
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/string-find-startswith.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/string-find-startswith.cpp
new file mode 100644
index 000000000000000..50e5c0ed93ea066
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/string-find-startswith.cpp
@@ -0,0 +1,77 @@
+// RUN: %check_clang_tidy -std=c++20 %s modernize-string-find-startswith %t -- -- -isystem %clang_tidy_headers
+
+#include <string>
+
+std::string foo(std::string);
+std::string bar();
+
+#define A_MACRO(x, y) ((x) == (y))
+
+void test(std::string s, std::string_view sv) {
+ s.find("a") == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of find() == 0
+ // CHECK-FIXES: s.starts_with("a");
+
+ s.find(s) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: s.starts_with(s);
+
+ s.find("aaa") != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !s.starts_with("aaa");
+
+ s.find(foo(foo(bar()))) != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !s.starts_with(foo(foo(bar())));
+
+ if (s.find("....") == 0) { /* do something */ }
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: if (s.starts_with("...."))
+
+ 0 != s.find("a");
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !s.starts_with("a");
+
+ s.rfind("a", 0) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of rfind() == 0
+ // CHECK-FIXES: s.starts_with("a");
+
+ s.rfind(s, 0) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: s.starts_with(s);
+
+ s.rfind("aaa", 0) != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !s.starts_with("aaa");
+
+ s.rfind(foo(foo(bar())), 0) != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !s.starts_with(foo(foo(bar())));
+
+ if (s.rfind("....", 0) == 0) { /* do something */ }
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use starts_with
+ // CHECK-FIXES: if (s.starts_with("...."))
+
+ 0 != s.rfind("a", 0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !s.starts_with("a");
+
+ sv.find("a") == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: sv.starts_with("a");
+
+ sv.rfind("a", 0) != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !sv.starts_with("a");
+
+ // Expressions that don't trigger the check are here.
+ A_MACRO(s.find("a"), 0);
+ A_MACRO(s.rfind("a", 0), 0);
+ s.find("a", 1) == 0;
+ s.find("a", 1) == 1;
+ s.find("a") == 1;
+ s.rfind("a", 1) == 0;
+ s.rfind("a", 1) == 1;
+ s.rfind("a") == 0;
+ s.rfind("a") == 1;
+}
|
nicovank
force-pushed
the
modernize-string-find-startswith
branch
2 times, most recently
from
2b01f9d to
d9995b3
Compare
|
I wonder if this should also detect the Not sure if it would have a performance impact to use |
nicovank
force-pushed
the
modernize-string-find-startswith
branch
from
d9995b3 to
d31148a
Compare
|
would be to support also endswith in same check, so i'm not so sure about check name. |
clang-tools-extra/clang-tidy/modernize/StringFindStartswithCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/StringFindStartswithCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/StringFindStartswithCheck.h
Outdated
Show resolved
Hide resolved
+1 On a side note: I will be looking into the related patterns and their performance soon as I am getting very strange code/performance when they are used outside of a benchmark - especially with Clang. |
This is actually in my notes for making this check / codemod. No performance impact so not tackling yet, maybe in the future.
Similar as above, matching for that length argument is a bit trickier than just looking for 0. Not tackling for now. Any thoughts on open-ended check name instead? |
Would this also be the check you would implement the suggested use of |
|
Maybe modernize-use-starts-ends-with |
We already got one check for contains, its readability-container-contains, not sure how it apply for std::string, but maybe instead of calling this check modernize it could be readability, otherwise all checks would be modernize. |
|
I like |
clang-tools-extra/clang-tidy/modernize/StringFindStartswithCheck.cpp
Outdated
Show resolved
Hide resolved
|
Performance is fine, go ahead. |
nicovank
force-pushed
the
modernize-string-find-startswith
branch
4 times, most recently
from
5d66e40 to
fd3e09f
Compare
|
Addressed feedback, renamed to |
nicovank
changed the title
nicovank
force-pushed
the
modernize-string-find-startswith
branch
from
fd3e09f to
96e0037
Compare
nicovank
force-pushed
the
modernize-string-find-startswith
branch
2 times, most recently
from
22caf9b to
e0074d2
Compare
|
Updates. I've been running into issues when the begin location of the entire expression is a macro. The prefix removal, which should include the macro text, is not working as I expect it to. If you know of any checks that have similar logic I can take a look and copy. I just disabled it if EDIT: This is also what readability-container-contains does, I think it's fine to leave as-is. |
nicovank
force-pushed
the
modernize-string-find-startswith
branch
from
e0074d2 to
0dc3f43
Compare
|
I think |
|
@EugeneZelenko We've had this discussion, see here and the couple comments below: #72385 (comment). I guess I can rename one more time if there is consensus. |
|
@firewave, @carlosgalvezp: What do you think? |
|
Problem that I see with "modernize" category is that it overlap with other categories, for example modernize-make-shared is mainly a performance check, modernize-redundant-void-arg is an readability check. We basically lack category for code design (rule of 5, limiting class/file size, complexity), we lack category for security issues (things that are clearly a bug). And we got generic checks in some codding standard categories, that almost every project may need to enable like google-explicit-constructor. If we would have actual check aliases or check tags, so some checks could be target for example with "google" or "cert" tag then it would be different story. But with current speed of delivering changes to clang-tidy, it will never happen. |
|
Historically, |
|
Ping. @carlosgalvezp could you please weigh in? modernize / performance? Anyone else? I have no real preference either way. Now that modernize is fully integrated in Clang-Tidy, I don't think historical reasons should lead to modernize overbloat. If this check was only replacing |
|
Apologies for the late reply, need to fix my mail filter so I don't miss the notifications! IMHO either modernize or readability are suitable categories. Performance can vary/be negligible depending on context I suppose? |
nicovank
force-pushed
the
modernize-string-find-startswith
branch
from
0dc3f43 to
316b45d
Compare
nicovank
changed the title
|
Thank you!! |
clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst
Outdated
Show resolved
Hide resolved
PiotrZSL
left a comment
PiotrZSL
left a comment
There was a problem hiding this comment.
For a starts_with looks fine, consider now adding support for ends_with, maybe in separate push request once this will be merged.
nicovank
force-pushed
the
modernize-string-find-startswith
branch
from
316b45d to
86292b1
Compare
Planning to look into it and other I do not have commit access, please merge for me ( |
|
@nicovank I run check on llvm, reported 3 issues, no false-positives. There is warning reported for FindFun variable because is used only in assert, check if you could do something with it, maybe add if for checking agains null, to silent warning or removing this variable and assert. |
|
Thanks! There should be no more warning. I tried to edit my GitHub settings, great if it worked, otherwise GitHub or personal email is fine. |
nicovank
force-pushed
the
modernize-string-find-startswith
branch
from
86292b1 to
c6b5aac
Compare
|
@nicovank "[email protected]" is fine ? To enable "meta.com" you may need to add this email in github and confirm it. Then it will allow you to select it as "default". I will merge this today.... |
|
I've made it my primary email, if it's not an option, 
|
|
@nicovank Interesting, still shows fb as main email, most probably because commit were done from that one, you could try to change commit email and do force push, could help. Sad thing is that if you would have ability to click squash and merge, then it would show you option to select email, I do not have such option. |
Match .find() and .rfind() calls compared to 0, and suggests replacing them with starts_with.
nicovank
force-pushed
the
modernize-string-find-startswith
branch
from
c6b5aac to
2b77483
Compare
|
Changed author to |
6 participants
Make a modernize version of abseil-string-find-startswith using the available C++20
std::string::starts_withandstd::string_view::starts_with. Following up from #72283.Possible reviewers: @PiotrZSL @njames93