llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Nicolas van Kempen (nicovank) <details> <summary>Changes</summary> In one of the cases recently added to this check in #<!-- -->110448, the error message is no longer accurate as the comparison is not with zero. #<!-- -->116033 brought my attention to this as it may add another comparison without zero. Remove the `[!=] 0` part of the diagnostic. This is in line with some other checks e.g. modernize-use-emplace. ``` > cat tmp.cpp #include <string> bool f(std::string u, std::string v) { return u.rfind(v) == u.size() - v.size(); } # Before. > ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20 tmp.cpp:3:12: warning: use ends_with instead of rfind() == 0 [modernize-use-starts-ends-with] 3 | return u.rfind(v) == u.size() - v.size(); | ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~ | ends_with( ) # After. > ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20 tmp.cpp:3:12: warning: use ends_with instead of rfind [modernize-use-starts-ends-with] 3 | return u.rfind(v) == u.size() - v.size(); | ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~ | ends_with( ) ``` --- Full diff: https://github.com/llvm/llvm-project/pull/116132.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp (+3-6) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+1-1) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp (+4-4) ``````````diff diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 1231f954298adc..4d8f6dbc2cc11b 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -189,11 +189,8 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) { if (ComparisonExpr->getBeginLoc().isMacroID()) return; - const bool Neg = ComparisonExpr->getOpcode() == BO_NE; - - auto Diagnostic = - diag(FindExpr->getExprLoc(), "use %0 instead of %1() %select{==|!=}2 0") - << ReplacementFunction->getName() << FindFun->getName() << Neg; + auto Diagnostic = diag(FindExpr->getExprLoc(), "use %0 instead of %1") + << ReplacementFunction->getName() << FindFun->getName(); // Remove possible arguments after search expression and ' [!=]= .+' suffix. Diagnostic << FixItHint::CreateReplacement( @@ -215,7 +212,7 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) { (ReplacementFunction->getName() + "(").str()); // Add possible negation '!'. - if (Neg) + if (ComparisonExpr->getOpcode() == BO_NE) Diagnostic << FixItHint::CreateInsertion(FindExpr->getBeginLoc(), "!"); } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index db971f08ca3dbc..a6c2ced1a56783 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -244,7 +244,7 @@ Changes in existing checks - Improved :doc:`modernize-use-starts-ends-with <clang-tidy/checks/modernize/use-starts-ends-with>` check to handle two cases - that can be replaced with ``ends_with`` + that can be replaced with ``ends_with``. Minor change to error message. - Improved :doc:`modernize-use-std-format <clang-tidy/checks/modernize/use-std-format>` check to support replacing diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp index 91477241e82e54..788de297a01c01 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp @@ -36,7 +36,7 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss, string_like sl, string_like_camel slc, prefer_underscore_version puv, prefer_underscore_version_flip puvf) { s.find("a") == 0; - // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of find() == 0 + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of find // CHECK-FIXES: s.starts_with("a"); (((((s)).find("a")))) == ((0)); @@ -68,7 +68,7 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss, // 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-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of rfind // CHECK-FIXES: s.starts_with("a"); s.rfind(s, 0) == 0; @@ -149,11 +149,11 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss, // CHECK-FIXES: puvf.starts_with("a"); s.compare(0, 1, "a") == 0; - // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of compare() == 0 + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of compare // CHECK-FIXES: s.starts_with("a"); s.compare(0, 1, "a") != 0; - // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of compare() != 0 + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of compare // CHECK-FIXES: !s.starts_with("a"); s.compare(0, strlen("a"), "a") == 0; `````````` </details> https://github.com/llvm/llvm-project/pull/116132 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits