https://github.com/irishrover updated https://github.com/llvm/llvm-project/pull/196387
>From fa9cc2eb20afdef626a56fe862d6d48c4db589c7 Mon Sep 17 00:00:00 2001 From: Zinovy Nis <[email protected]> Date: Sat, 25 Apr 2026 12:49:57 +0300 Subject: [PATCH] [clang-tidy] Reland "An option for conditional skipping overloaded functions in modernize-use-string-view" --- .../modernize/UseStringViewCheck.cpp | 6 +- .../clang-tidy/modernize/UseStringViewCheck.h | 5 +- .../checks/modernize/use-string-view.rst | 10 + .../modernize/use-string-view-overloaded.cpp | 185 ++++++++++++++++++ .../checkers/modernize/use-string-view.cpp | 91 --------- 5 files changed, 203 insertions(+), 94 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-overloaded.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp index 29e5bdb65632e..9892870279b55 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp @@ -80,6 +80,7 @@ static void fixReturns(const FunctionDecl *FuncDecl, DiagnosticBuilder &Diag, UseStringViewCheck::UseStringViewCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), + CheckOverloadedFunctions(Options.get("CheckOverloadedFunctions", false)), IgnoredFunctions(utils::options::parseStringList( Options.get("IgnoredFunctions", "toString$;ToString$;to_string$"))) { parseReplacementStringViewClass( @@ -87,6 +88,7 @@ UseStringViewCheck::UseStringViewCheck(StringRef Name, } void UseStringViewCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "CheckOverloadedFunctions", CheckOverloadedFunctions); Options.store(Opts, "IgnoredFunctions", utils::options::serializeStringList(IgnoredFunctions)); Options.store(Opts, "ReplacementStringViewClass", @@ -109,11 +111,13 @@ void UseStringViewCheck::registerMatchers(MatchFinder *Finder) { hasFalseExpression(ignoringParenImpCasts(stringLiteral()))); const auto VirtualOrOperator = cxxMethodDecl(anyOf(cxxConversionDecl(), isVirtual())); + const auto CheckOverloaded = + CheckOverloadedFunctions ? unless(anything()) : isOverloaded(); Finder->addMatcher( functionDecl( isDefinition(), unless(anyOf(VirtualOrOperator, IgnoredFunctionsMatcher, - isOverloaded(), + CheckOverloaded, ast_matchers::isExplicitTemplateSpecialization())), returns(IsStdString), hasDescendant(returnStmt()), unless(hasDescendant(returnStmt(hasReturnValue(unless( diff --git a/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.h index f5f11edc54824..275ce904290ac 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.h @@ -35,13 +35,14 @@ class UseStringViewCheck : public ClangTidyCheck { StringRef toStringViewTypeStr(StringRef Type) const; void parseReplacementStringViewClass(StringRef Options); + bool CheckOverloadedFunctions = false; + const std::vector<StringRef> IgnoredFunctions; + StringRef StringViewClass = "std::string_view"; StringRef WStringViewClass = "std::wstring_view"; StringRef U8StringViewClass = "std::u8string_view"; StringRef U16StringViewClass = "std::u16string_view"; StringRef U32StringViewClass = "std::u32string_view"; - - const std::vector<StringRef> IgnoredFunctions; }; } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-string-view.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-string-view.rst index c72a0480c0eb8..f6b0cb37134e7 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-string-view.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-string-view.rst @@ -108,6 +108,16 @@ simply make an explicit conversion. Options ------- +.. option:: CheckOverloadedFunctions + If ``true``, the check will also consider overloaded functions for + ``string_view`` conversion suggestions. + + If ``false``, overloaded functions are skipped to avoid potential issues + with ambiguous conversions. + + Default is ``false``. + + .. option:: IgnoredFunctions A semicolon-separated list of the names of functions or methods to be diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-overloaded.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-overloaded.cpp new file mode 100644 index 0000000000000..bdb30473fda2b --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-overloaded.cpp @@ -0,0 +1,185 @@ +// RUN: %check_clang_tidy -check-suffix=DONTCHECK \ +// RUN: -std=c++17-or-later %s modernize-use-string-view %t -- \ +// RUN: --config="{CheckOptions: {modernize-use-string-view.CheckOverloadedFunctions: false}}" + +// RUN: %check_clang_tidy -check-suffix=CHECK \ +// RUN: -std=c++17-or-later %s modernize-use-string-view %t -- \ +// RUN: --config="{CheckOptions: {modernize-use-string-view.CheckOverloadedFunctions: true}}" + +// RUN: %check_clang_tidy -check-suffix=CHECK-CXX20 \ +// RUN: -std=c++20-or-later %s modernize-use-string-view %t -- \ +// RUN: --config="{CheckOptions: {modernize-use-string-view.CheckOverloadedFunctions: true}}" \ +// RUN: -- -DUSE_CXX20=1 + +#include <string> +#include <utility> + +namespace overload_funcs_redeclared { +std::basic_string<char> overload(int); +std::string overload(int); +std::string overload(int) { return "int"; } +// CHECK-MESSAGES-DONTCHECK:[[@LINE-1]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK:[[@LINE-2]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-3]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-DONTCHECK: std::string_view overload(int) { return "int"; } +// CHECK-FIXES-CHECK: std::string_view overload(int) { return "int"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(int) { return "int"; } +} + +namespace overload_non_func { +struct overload {}; +std::string overload(int) { return "int"; } +// CHECK-MESSAGES-DONTCHECK:[[@LINE-1]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK:[[@LINE-2]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-3]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-DONTCHECK: std::string_view overload(int) { return "int"; } +// CHECK-FIXES-CHECK: std::string_view overload(int) { return "int"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(int) { return "int"; } +} + +namespace overload_with_inline { + inline namespace inline_namespace { + std::string overload1(int) { return "int"; } +// CHECK-MESSAGES-DONTCHECK:[[@LINE-1]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK:[[@LINE-2]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-3]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-DONTCHECK: std::string_view overload1(int) { return "int"; } +// CHECK-FIXES-CHECK: std::string_view overload1(int) { return "int"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload1(int) { return "int"; } + } + inline namespace regular_namespace { + std::string overload1(int) { return "int"; } +// CHECK-MESSAGES-DONTCHECK:[[@LINE-1]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK:[[@LINE-2]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-3]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-DONTCHECK: std::string_view overload1(int) { return "int"; } +// CHECK-FIXES-CHECK: std::string_view overload1(int) { return "int"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload1(int) { return "int"; } + } +} + +namespace overload_funcs { +std::string dbl2str(double f); +std::string overload(int) { return "int"; } +// CHECK-MESSAGES-CHECK:[[@LINE-1]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-2]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK: std::string_view overload(int) { return "int"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(int) { return "int"; } +std::string overload(double f) { return "f=" + dbl2str(f); } +std::string overload(std::string) { return "string"; } +// CHECK-MESSAGES-CHECK:[[@LINE-1]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-2]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK: std::string_view overload(std::string) { return "string"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(std::string) { return "string"; } +} + +namespace overload_methods { +struct Foo { + // Skip overloaded methods + std::string overload(int) { return "int"; } +// CHECK-MESSAGES-CHECK:[[@LINE-1]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-2]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK: std::string_view overload(int) { return "int"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(int) { return "int"; } + std::string overload(double f) { return "double"; } +// CHECK-MESSAGES-CHECK:[[@LINE-1]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-2]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK: std::string_view overload(double f) { return "double"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(double f) { return "double"; } + std::string overload(std::string) { return "string"; } +// CHECK-MESSAGES-CHECK:[[@LINE-1]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-2]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK: std::string_view overload(std::string) { return "string"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(std::string) { return "string"; } +}; +} + +namespace overload_methods_nested_classes { +struct Bar { + std::string overload(int) { return "int"; } +// CHECK-MESSAGES-CHECK:[[@LINE-1]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-2]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK: std::string_view overload(int) { return "int"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(int) { return "int"; } + std::string overload(std::string) { return "string"; } +// CHECK-MESSAGES-CHECK:[[@LINE-1]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-2]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK: std::string_view overload(std::string) { return "string"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(std::string) { return "string"; } + struct FooBar { + std::string overload(char*) { return "char*"; } +// CHECK-MESSAGES-CHECK:[[@LINE-1]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-2]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK: std::string_view overload(char*) { return "char*"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(char*) { return "char*"; } + std::string overload(double f) { return "double"; } +// CHECK-MESSAGES-CHECK:[[@LINE-1]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-2]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK: std::string_view overload(double f) { return "double"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(double f) { return "double"; } + }; +}; +} + +namespace two_overloads_with_inline { + inline namespace inline_namespace { + std::string overload(int) { return "int"; } +// CHECK-MESSAGES-CHECK:[[@LINE-1]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-2]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK: std::string_view overload(int) { return "int"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(int) { return "int"; } + std::string overload(double) { return "double"; } +// CHECK-MESSAGES-CHECK:[[@LINE-1]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-2]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK: std::string_view overload(double) { return "double"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(double) { return "double"; } + } + std::string overload(int) { return "int"; } +// CHECK-MESSAGES-CHECK:[[@LINE-1]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-2]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK: std::string_view overload(int) { return "int"; } +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(int) { return "int"; } +} + +#if USE_CXX20 + +namespace overload_with_outer { +namespace overload_with_templates { + template <typename T> + std::string overload(T) { return "T"; } +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-1]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] + std::string overload(std::string) { return "string"; } +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-1]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(std::string) { return "string"; } +} +using overload_with_templates::overload; +std::string overload(char*) { return "char*"; } +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-1]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(char*) { return "char*"; } +} + +namespace overload_methods_nested_namespaces { +namespace foo { + std::string overload(int) { return "int"; } +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-1]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(int) { return "int"; } + std::string overload(std::string) { return "string"; } +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-1]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(std::string) { return "string"; } +} +using foo::overload; +std::string overload(char*) { return "char*"; } +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-1]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(char*) { return "char*"; } +} + +namespace overload_methods_templated { + template <typename T> + std::string overload(T value) { return "T";} +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-1]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(T value) { return "T";} + std::string overload(int value) { return "int"; } +// CHECK-MESSAGES-CHECK-CXX20:[[@LINE-1]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES-CHECK-CXX20: std::string_view overload(int value) { return "int"; } +} +#endif diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view.cpp index 07c7eaff4a4c3..26a72c1c242e1 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view.cpp @@ -203,44 +203,6 @@ MyString<wchar_t> aliasedWChar() { return L"aliasedWChar"; } -namespace overload_funcs_redeclared { -std::basic_string<char> overload(int); -std::string overload(int); -std::string overload(int) { return "int"; } -// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] -// CHECK-FIXES: std::string_view overload(int) { return "int"; } -} - -namespace overload_non_func { -struct overload {}; -std::string overload(int) { return "int"; } -// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] -// CHECK-FIXES: std::string_view overload(int) { return "int"; } -} - -namespace overload_with_inline { - inline namespace inline_namespace { - std::string overload(int) { return "int"; } -// CHECK-MESSAGES:[[@LINE-1]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] -// CHECK-FIXES: std::string_view overload(int) { return "int"; } - } - inline namespace regular_namespace { - std::string overload(int) { return "int"; } -// CHECK-MESSAGES:[[@LINE-1]]:5: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] -// CHECK-FIXES: std::string_view overload(int) { return "int"; } - } -} - -namespace overload_with_outer { -namespace overload_with_templates { - template <typename T> - std::string overload(T) { return "T"; } - std::string overload(std::string) { return "string"; } -} -using overload_with_templates::overload; -std::string overload(char*) { return "char*"; } -} - // ========================================================== // Negative tests // ========================================================== @@ -395,59 +357,6 @@ std::string lambda() { }(); } -namespace overload_funcs { -std::string dbl2str(double f); -// Skip overloaded functions -std::string overload(int) { return "int"; } -// Because of this overload (non-literal return) the fix should not be applied -std::string overload(double f) { return "f=" + dbl2str(f); } -std::string overload(std::string) { return "string"; } -} - -namespace overload_methods { -struct Foo { - // Skip overloaded methods - std::string overload(int) { return "int"; } - std::string overload(double f) { return "double"; } - std::string overload(std::string) { return "string"; } -}; -} - -namespace overload_methods_nested_classes { -struct Bar { - std::string overload(int) { return "int"; } - std::string overload(std::string) { return "string"; } - - struct FooBar { - std::string overload(char*) { return "char*"; } - std::string overload(double f) { return "double"; } - }; -}; -} - -namespace overload_methods_nested_namespaces { -namespace foo { - std::string overload(int) { return "int"; } - std::string overload(std::string) { return "string"; } -} -using foo::overload; -std::string overload(char*) { return "char*"; } -} - -namespace overload_methods_templated { - template <typename T> - std::string overload(T value) { return "T";} - std::string overload(int value) { return "int"; } -} - -namespace two_overloads_with_inline { - inline namespace inline_namespace { - std::string overload(int) { return "int"; } - std::string overload(double) { return "double"; } - } - std::string overload(int) { return "int"; } -} - struct TemplateString { static constexpr char* val = "TEMPLATE"; template<typename T> _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
