https://github.com/irishrover updated https://github.com/llvm/llvm-project/pull/172170
>From 32fccabc6f0052d87982f72d62023dbfe87b744d Mon Sep 17 00:00:00 2001 From: Zinovy Nis <[email protected]> Date: Sat, 13 Dec 2025 13:47:01 +0300 Subject: [PATCH] [clang-tidy] Add a new check 'modernize-use-string-view' Looks for functions returning `std::[w|u8|u16|u32]string` and suggests to change it to `std::[...]string_view` if possible and profitable. Example: ```cpp std::string foo(int i) { // <---- can be replaced to `std::string_view foo(...) {` switch(i) { case 1: return "case1"; case 2: return "case2"; default: return {}; } } ``` --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ModernizeTidyModule.cpp | 3 + .../modernize/UseStringViewCheck.cpp | 153 +++++++ .../clang-tidy/modernize/UseStringViewCheck.h | 49 +++ clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../docs/clang-tidy/checks/list.rst | 1 + .../checks/modernize/use-string-view.rst | 130 ++++++ .../clang-tidy/checkers/Inputs/Headers/string | 22 + .../modernize/use-string-view-custom-view.cpp | 40 ++ .../modernize/use-string-view-cxx20.cpp | 42 ++ .../modernize/use-string-view-ignored.cpp | 40 ++ .../checkers/modernize/use-string-view.cpp | 403 ++++++++++++++++++ 12 files changed, 890 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-string-view.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-custom-view.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-cxx20.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-ignored.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 488c359661018..858cf921f9d34 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -50,6 +50,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseStdFormatCheck.cpp UseStdNumbersCheck.cpp UseStdPrintCheck.cpp + UseStringViewCheck.cpp UseTrailingReturnTypeCheck.cpp UseTransparentFunctorsCheck.cpp UseUncaughtExceptionsCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 800639c6cea16..eb73478b44023 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -50,6 +50,7 @@ #include "UseStdFormatCheck.h" #include "UseStdNumbersCheck.h" #include "UseStdPrintCheck.h" +#include "UseStringViewCheck.h" #include "UseTrailingReturnTypeCheck.h" #include "UseTransparentFunctorsCheck.h" #include "UseUncaughtExceptionsCheck.h" @@ -130,6 +131,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<UseNoexceptCheck>("modernize-use-noexcept"); CheckFactories.registerCheck<UseNullptrCheck>("modernize-use-nullptr"); CheckFactories.registerCheck<UseOverrideCheck>("modernize-use-override"); + CheckFactories.registerCheck<UseStringViewCheck>( + "modernize-use-string-view"); CheckFactories.registerCheck<UseTrailingReturnTypeCheck>( "modernize-use-trailing-return-type"); CheckFactories.registerCheck<UseTransparentFunctorsCheck>( diff --git a/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp new file mode 100644 index 0000000000000..376418431aec3 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.cpp @@ -0,0 +1,153 @@ +//===----------------------------------------------------------------------===// +// +// 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 "UseStringViewCheck.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/ASTDiagnostic.h" +#include "clang/AST/Stmt.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +static constexpr StringRef StringViewClassKey = "string"; +static constexpr StringRef WStringViewClassKey = "wstring"; +static constexpr StringRef U8StringViewClassKey = "u8string"; +static constexpr StringRef U16StringViewClassKey = "u16string"; +static constexpr StringRef U32StringViewClassKey = "u32string"; + +static auto getStringTypeMatcher(StringRef CharType) { + return hasCanonicalType(hasDeclaration(cxxRecordDecl(hasName(CharType)))); +} + +static void fixReturns(const FunctionDecl *FuncDecl, DiagnosticBuilder &Diag, + ASTContext &Context) { + auto Matches = match( + findAll(returnStmt(hasReturnValue(cxxConstructExpr(anyOf( + argumentCountIs(0), cxxTemporaryObjectExpr())))) + .bind("return")), + *FuncDecl->getBody(), Context); + for (const auto &Match : Matches) { + if (const auto *RetValue = + Match.getNodeAs<ReturnStmt>("return")->getRetValue()) + if (const auto ReturnSourceRange = RetValue->getSourceRange(); + ReturnSourceRange.isValid()) + Diag << FixItHint::CreateReplacement(ReturnSourceRange, "{}"); + } +} + +UseStringViewCheck::UseStringViewCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnoredFunctions(utils::options::parseStringList( + Options.get("IgnoredFunctions", "toString$;ToString$;to_string$"))) { + parseReplacementStringViewClass( + Options.get("ReplacementStringViewClass", "")); +} + +void UseStringViewCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoredFunctions", + utils::options::serializeStringList(IgnoredFunctions)); + Options.store(Opts, "ReplacementStringViewClass", + (Twine("") + StringViewClassKey + "=" + StringViewClass + ";" + + WStringViewClassKey + "=" + WStringViewClass + ";" + + U8StringViewClassKey + "=" + U8StringViewClass + ";" + + U16StringViewClassKey + "=" + U16StringViewClass + ";" + + U32StringViewClassKey + "=" + U32StringViewClass) + .str()); +} + +void UseStringViewCheck::registerMatchers(MatchFinder *Finder) { + const auto IsStdString = getStringTypeMatcher("::std::basic_string"); + // TODO: also consider *StringViewClass types + const auto IsStdStringView = getStringTypeMatcher("::std::basic_string_view"); + const auto IgnoredFunctionsMatcher = + matchers::matchesAnyListedName(IgnoredFunctions); + const auto TernaryOperator = conditionalOperator( + hasTrueExpression(ignoringParenImpCasts(stringLiteral())), + hasFalseExpression(ignoringParenImpCasts(stringLiteral()))); + const auto VirtualOrOperator = + cxxMethodDecl(anyOf(cxxConversionDecl(), isVirtual())); + Finder->addMatcher( + functionDecl( + isDefinition(), + unless(anyOf(VirtualOrOperator, IgnoredFunctionsMatcher, + ast_matchers::isExplicitTemplateSpecialization())), + returns(IsStdString), hasDescendant(returnStmt()), + unless(hasDescendant(returnStmt(hasReturnValue(unless( + anyOf(stringLiteral(), hasType(IsStdStringView), TernaryOperator, + cxxConstructExpr(anyOf( + allOf(hasType(IsStdString), argumentCountIs(0)), + allOf(isListInitialization(), + unless(cxxTemporaryObjectExpr()), + hasArgument(0, ignoringParenImpCasts( + stringLiteral())))))))))))) + .bind("func"), + this); +} + +void UseStringViewCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("func"); + assert(MatchedDecl); + bool ShouldAKA = false; + const std::string DesugaredTypeStr = + clang::desugarForDiagnostic( + *Result.Context, QualType(MatchedDecl->getReturnType()), ShouldAKA) + .getAsString(); + const StringRef DestReturnTypeStr = toStringViewTypeStr(DesugaredTypeStr); + + auto Diag = + diag(MatchedDecl->getTypeSpecStartLoc(), + "consider using '%0' to avoid unnecessary copying and allocations") + << DestReturnTypeStr; + + fixReturns(MatchedDecl, Diag, *Result.Context); + + for (const auto *FuncDecl : MatchedDecl->redecls()) + if (const SourceRange ReturnTypeRange = + FuncDecl->getReturnTypeSourceRange(); + ReturnTypeRange.isValid()) + Diag << FixItHint::CreateReplacement(ReturnTypeRange, DestReturnTypeStr); +} + +StringRef UseStringViewCheck::toStringViewTypeStr(StringRef Type) const { + if (Type.contains("wchar_t")) + return WStringViewClass; + if (Type.contains("char8_t")) + return U8StringViewClass; + if (Type.contains("char16_t")) + return U16StringViewClass; + if (Type.contains("char32_t")) + return U32StringViewClass; + return StringViewClass; +} + +void UseStringViewCheck::parseReplacementStringViewClass(StringRef Options) { + if (Options.empty()) + return; + for (const auto &Option : utils::options::parseStringList(Options)) { + const auto Split = Option.split("="); + if (Split.first == StringViewClassKey) + StringViewClass = Split.second; + else if (Split.first == WStringViewClassKey) + WStringViewClass = Split.second; + else if (Split.first == U8StringViewClassKey) + U8StringViewClass = Split.second; + else if (Split.first == U16StringViewClassKey) + U16StringViewClass = Split.second; + else if (Split.first == U32StringViewClassKey) + U32StringViewClass = Split.second; + } +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.h new file mode 100644 index 0000000000000..f5f11edc54824 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseStringViewCheck.h @@ -0,0 +1,49 @@ +//===----------------------------------------------------------------------===// +// +// 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_USESTRINGVIEWCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTRINGVIEWCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::modernize { + +/// Looks for functions returning `std::[w|u8|u16|u32]string` and suggests to +/// change it to `std::[...]string_view` if possible and profitable. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-string-view.html +class UseStringViewCheck : public ClangTidyCheck { +public: + UseStringViewCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus17; + } + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + +private: + StringRef toStringViewTypeStr(StringRef Type) const; + void parseReplacementStringViewClass(StringRef Options); + + 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 + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTRINGVIEWCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 576aedaf427e6..25a5ad3357c68 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -265,6 +265,12 @@ New checks Finds virtual function overrides with different visibility than the function in the base class. +- New :doc:`modernize-use-string-view + <clang-tidy/checks/modernize/use-string-view>` check. + + Looks for functions returning ``std::[w|u8|u16|u32]string`` and suggests to + change it to ``std::[...]string_view`` for performance reasons if possible. + - New :doc:`readability-inconsistent-ifelse-braces <clang-tidy/checks/readability/inconsistent-ifelse-braces>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 3dabf887dc2e1..2e1cde34aa39f 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -331,6 +331,7 @@ Clang-Tidy Checks :doc:`modernize-use-std-format <modernize/use-std-format>`, "Yes" :doc:`modernize-use-std-numbers <modernize/use-std-numbers>`, "Yes" :doc:`modernize-use-std-print <modernize/use-std-print>`, "Yes" + :doc:`modernize-use-string-view <modernize/use-string-view>`, "Yes" :doc:`modernize-use-trailing-return-type <modernize/use-trailing-return-type>`, "Yes" :doc:`modernize-use-transparent-functors <modernize/use-transparent-functors>`, "Yes" :doc:`modernize-use-uncaught-exceptions <modernize/use-uncaught-exceptions>`, "Yes" 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 new file mode 100644 index 0000000000000..8e89e64da7e48 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-string-view.rst @@ -0,0 +1,130 @@ +.. title:: clang-tidy - modernize-use-string-view + +modernize-use-string-view +========================= + +Looks for functions returning ``std::[w|u8|u16|u32]string`` and suggests to +change it to ``std::[...]string_view`` for performance reasons if possible. + +Each time a new ``std::string`` is created from a literal, a copy of that +literal is allocated either in ``std::string``'s internal buffer +(for short literals) or in a heap. + +For the cases where ``std::string`` is returned from a function, +such allocations can sometimes be eliminated by using ``std::string_view`` +as a return type. + +This check looks for such functions returning ``std::string`` baked from the +literals and suggests replacing their return type to ``std::string_view``. + +It handles ``std::string``, ``std::wstring``, ``std::u8string``, +``std::u16string`` and ``std::u32string`` along with their aliases and selects +the proper kind of ``std::string_view`` to return. + +Consider the following example: + +.. code-block:: c++ + + std::string foo(int i) { + switch(i) { + case 1: + return "case 1"; + ... + default: + return "default"; + } + } + +In the code above a new ``std::string`` object is created on each function +invocation, making a copy of a string literal and possibly allocating a memory +in a heap. + +The check gets this code transformed into: + +.. code-block:: c++ + + std::string_view foo(int i) { + switch(i) { + case 1: + return "case 1"; + ... + default: + return "default"; + } + } + +New version re-uses statically allocated literals without additional overhead. + +Hint +---- + +To prevent a warning and a fix wrap your string with ``std::string(...)``: + +.. code-block:: c++ + + std::string foo() { + return "default"; //warning and fix are generated + } + + std::string bar() { + return std::string("default"); //warning and fix are NOT generated + } + +Limitations +----------- + +* No warning and/or fix are generated as for now for these code patterns: + + * ``return std::string("literal");`` - hint: to prevent a warning and a fix + you may wrap your string with ``std::string(...)`` + * ``return std::string{"literal"};`` + * ``return "simpleLiteral"s;`` + * ``auto foo() { return "autoReturn"; }`` + * ``auto Trailing() -> std::string { return "Trailing"; }`` warns, doesn't fix + * returnings from lambda + * complicated macro and templated code + +* In some cases the fixed code can be incorrect. But it's usually easy to fix: + +.. code-block:: c++ + + string foo() { // <--- will be replaced with string_view + return "foo"; + } + + void bar() { + string f = foo(); // <----- error: no viable conversion from + // 'std::string_view' (aka 'basic_string_view<char>') + // to 'std::string' (aka 'basic_string<char>') + } + + +Options +------- + +.. option:: IgnoredFunctions + + A semicolon-separated list of the names of functions or methods to be + ignored. Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches + every type with suffix ``Ref``, ``ref``, ``Reference`` and ``reference``. + + If a name in the list contains the sequence `::` it is matched against the + qualified type name (i.e. ``namespace::Type``), otherwise it is matched + against only the type name (i.e. ``Type``). + + The default is {`toString$;ToString$;to_string$`}. + + +.. option:: ReplacementStringViewClass + + A semicolon-separated list of `string=string_view` pairs for replacing + ``string`` to ``string_view`` counterparts. + + * ``key`` is one of {``string``, ``wstring``, ``u8string``, ``u16string``, + ``u32string``}, + * value is the qualified name for the corresponding string view type, like + ``std:string_view`` or ``StringRef``. + + The default is {`string=std::string_view;wstring=std::wstring_view; + u8string=std::u8string_view;u16string=std::u16string_view; + u32string=std::u32string_view`}. diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string index 6cedda4202f14..c24533fa046f9 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string @@ -22,6 +22,7 @@ struct basic_string { typedef size_t size_type; typedef basic_string<C, T, A> _Type; basic_string(); + basic_string(basic_string_view<C, T>); basic_string(const C *p, const A &a = A()); basic_string(const C *p, size_type count); basic_string(const C *b, const C *e); @@ -90,8 +91,14 @@ struct basic_string { typedef basic_string<char> string; typedef basic_string<wchar_t> wstring; +#if __cplusplus >= 202002L +typedef basic_string<char8_t> u8string; +typedef basic_string<char16_t> u16string; +typedef basic_string<char32_t> u32string; +#else typedef basic_string<char16> u16string; typedef basic_string<char32> u32string; +#endif template <typename C, typename T> struct basic_string_view { @@ -136,8 +143,14 @@ struct basic_string_view { typedef basic_string_view<char> string_view; typedef basic_string_view<wchar_t> wstring_view; +#if __cplusplus >= 202002L +typedef basic_string_view<char8_t> u8string_view; +typedef basic_string_view<char16_t> u16string_view; +typedef basic_string_view<char32_t> u32string_view; +#else typedef basic_string_view<char16> u16string_view; typedef basic_string_view<char32> u32string_view; +#endif std::string operator+(const std::string&, const std::string&); std::string operator+(const std::string&, const char*); @@ -166,6 +179,15 @@ bool operator!=(const char*, const std::string_view&); #endif size_t strlen(const char* str); + +namespace literals { +namespace string_literals { + string operator""s(const char *, size_t); +} +namespace string_view_literals { + string_view operator""sv(const char *, size_t); +} +} } #endif // _STRING_ diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-custom-view.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-custom-view.cpp new file mode 100644 index 0000000000000..f1bb86a5ec699 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-custom-view.cpp @@ -0,0 +1,40 @@ +// RUN: %check_clang_tidy \ +// RUN: -std=c++20-or-later %s modernize-use-string-view %t -- \ +// RUN: --config="{CheckOptions: {modernize-use-string-view.ReplacementStringViewClass: 'string=StringRef;u8string=U8StringRef'}}" \ +// RUN: -- -isystem %clang_tidy_headers + +#include <string> + +// ========================================================== +// Positive tests +// ========================================================== + +std::string Literal() { +// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'StringRef' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: StringRef Literal() { + return "literal"; +} + +std::wstring WLiteral() { +// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::wstring_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: std::wstring_view WLiteral() { + return L"literal"; +} + +std::u8string U8Literal() { +// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'U8StringRef' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: U8StringRef U8Literal() { + return u8"literal"; +} + +std::u16string U16Literal() { +// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::u16string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: std::u16string_view U16Literal() { + return u"literal"; +} + +std::u32string U32Literal() { +// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::u32string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: std::u32string_view U32Literal() { + return U"literal"; +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-cxx20.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-cxx20.cpp new file mode 100644 index 0000000000000..65f94c5354f38 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-cxx20.cpp @@ -0,0 +1,42 @@ +// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-use-string-view %t -- -- -isystem %clang_tidy_headers + +#include <string> + +// ========================================================== +// Positive tests +// ========================================================== + +std::string simpleLiteral() { +// 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 simpleLiteral() { + return "simpleLiteral"; +} + +std::wstring simpleLiteralW() { +// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::wstring_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: std::wstring_view simpleLiteralW() { + return L"wide literal"; +} + +std::u8string simpleLiteral8() { +// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::u8string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: std::u8string_view simpleLiteral8() { + return u8"simpleLiteral"; +} + +std::u16string simpleLiteral16() { +// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::u16string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: std::u16string_view simpleLiteral16() { + return u"simpleLiteral"; +} + +std::u32string simpleLiteral32() { +// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::u32string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: std::u32string_view simpleLiteral32() { + return U"simpleLiteral"; +} + +// ========================================================== +// Negative tests +// ========================================================== + diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-ignored.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-ignored.cpp new file mode 100644 index 0000000000000..56f5e9ce33dda --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view-ignored.cpp @@ -0,0 +1,40 @@ +// RUN: %check_clang_tidy \ +// RUN: -std=c++17-or-later %s modernize-use-string-view %t -- \ +// RUN: --config="{CheckOptions: {modernize-use-string-view.IgnoredFunctions: 'GoodButIgnored;GoodTooButAlsoIgnored'}}" \ +// RUN: -- -isystem %clang_tidy_headers + +#include <string> + +// ========================================================== +// Positive tests +// ========================================================== + +std::string toString() { +// 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 toString() { + return "not ignored by custom options"; +} + +std::string ToString() { +// 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 ToString() { + return "not ignored by custom options"; +} + +std::string to_string() { +// 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 to_string() { + return "not ignored by custom options"; +} + +// ========================================================== +// Negative tests +// ========================================================== + +std::string GoodButIgnored() { + return "ignored by explicit options"; +} + +std::string GoodTooButAlsoIgnored() { + return "also ignored by explicit options"; +} 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 new file mode 100644 index 0000000000000..1cde65eafb857 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-string-view.cpp @@ -0,0 +1,403 @@ +// RUN: %check_clang_tidy \ +// RUN: -std=c++17-or-later %s modernize-use-string-view %t -- \ +// RUN: -- -isystem %clang_tidy_headers + +#include <string> + +namespace std { +template <class T, class U> struct is_same { static constexpr bool value = false; }; +template <class T> struct is_same<T, T> { static constexpr bool value = true; }; +template <class T, class U> constexpr bool is_same_v = is_same<T, U>::value; +} // namespace std + + +// ========================================================== +// Positive tests +// ========================================================== + +std::string simpleLiteral() { +// 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 simpleLiteral() { + return "simpleLiteral"; +} + +std::wstring simpleLiteralW() { +// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::wstring_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: std::wstring_view simpleLiteralW() { + return L"wide literal"; +} + +std::string simpleRLiteral() { +// 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 simpleRLiteral() { + return R"(simpleLiteral)"; +} + +[[nodiscard]] std::string Attributed() { +// CHECK-MESSAGES:[[@LINE-1]]:15: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: {{\[\[nodiscard\]\]}} std::string_view Attributed() { + return "attributed"; +} + +const std::string Const() { +// CHECK-MESSAGES:[[@LINE-1]]:7: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: const std::string_view Const() { + return "Const"; +} + +auto Trailing() -> std::string { +// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// TODO: support fixes for trailing types + return "Trailing"; +} + +std::string initList() { +// 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 initList() { + return {"list"}; +} + +std::string ctorReturn(int i) { +// 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 ctorReturn(int i) { + if( i == 42) { + do { + return std::string(); +// CHECK-FIXES: return {}; + } while(true); + } + else { + for(int k = 0; k < 1000; k++) + if ( i > k ) { + return std::string(); +// CHECK-FIXES: return {}; + } + else { + return "else"; + } + } + return std::string{}; +// CHECK-FIXES: return {}; +} + +std::string ctorWithInitListReturn() { +// 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 ctorWithInitListReturn() { +// CHECK-FIXES: return {}; + return std::string{}; +} + +std::string emptyReturn() { +// 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 emptyReturn() { + return {}; +} + +std::string switchCaseTest(int i) { +// 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 switchCaseTest(int i) { + switch (i) { + case 1: + return "case1"; + case 2: + return "case2"; + case 3: + return {}; + default: + return "default"; + } +} + +std::string ifElseTest(bool flag) { +// 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 ifElseTest(bool flag) { + if (flag) + return "true"; + return "false"; +} + +std::string ternary(bool flag) { +// 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 ternary(bool flag) { + return flag ? "true" : "false"; +} + +class A { + std::string classMethodInt() { return "internal"; } +// CHECK-MESSAGES:[[@LINE-1]]:3: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] +// CHECK-FIXES: std::string_view classMethodInt() { return "internal"; } + + std::string classMethodExt(); +// CHECK-FIXES: std::string_view classMethodExt(); +}; + +std::string A::classMethodExt() { return "external"; } +// 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 A::classMethodExt() { return "external"; } + +#define MACRO "MACRO LITERAL" +std::string macro() { +// 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 macro() { + return MACRO; +} + +#define my_string std::string +my_string macro_type() { +// 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 macro_type() { + return "MACTO LITERAL"; +} + +#define my_definition std::string function_inside_macro() +my_definition { +// CHECK-MESSAGES:[[@LINE-1]]:1: warning: consider using 'std::string_view' to avoid unnecessary copying and allocations [modernize-use-string-view] + return "literal"; +} + +// ========================================================== +// Negative tests +// ========================================================== + +std::string toString() { + return "ignored by default options"; +} + +std::string ToString() { + return "ignored by default options"; +} + +std::string to_string() { + return "ignored by default options"; +} + +std::string localVariable() { + std::string s = "local variable"; + // TODO: extract and return literal + return s; +} + +std::string dynamicCalculation() { + std::string s1 = "hello "; + return s1 + "world"; +} + +std::string mixedReturns(bool flag) { + if (flag) { + return "safe static literal"; + } + std::string s = "unsafe dynamic"; + return s; +} + +std::string stringTernary(bool flag) { + // TODO: extract literals + return flag ? std::string("true") : std::string("false"); +} + +std::string_view alreadyGood() { + return "alreadyGood"; +} + +std::wstring_view alreadyGoodW() { + return L"alreadyGood"; +} + +std::string simpleLiteralS() { + // TODO: replace ""s to literal and return string_view + using namespace std::literals::string_literals; + return "simpleLiteral"s; +} + +std::string_view alreadyGoodSV() { + using namespace std::literals::string_view_literals; + return "alreadyGood"sv; +} + +std::string returnArgCopy(std::string s) { + // Must not be converted to string_view because of use-after-free on stack + return s; +} + +std::string returnIndirection(const char* ptr) { + // Can be unsafe or intentional, like converting string_view into string + return ptr; +} + +std::string localBuffer() { + char buf[] = "local buffer"; + // Must not be converted to string_view because of use-after-free on stack + return buf; +} + +std::string returnConstVar() { + // TODO: seems safe + constexpr auto kVar = "const string"; + return kVar; +} + +std::string passStringView(std::string_view sv) { + // TODO: Can be unsafe or intentional, like converting string_view into string + return std::string(sv); +} + +std::string explicitConstruction() { + // Cannot be std::string_view: returning address of local temporary object + // TODO: extract and return literal + return std::string("explicitConstruction"); +} + +std::string explicitConstructionWithInitList() { + // Cannot be std::string_view: returning address of local temporary object + // TODO: extract and return literal + return std::string{"explicitConstruction"}; +} + +std::string explicitConstructionEmpty() { + return std::string(""); +} + +std::string explicitConstructionWithInitListEmpty() { + return std::string{""}; +} + +struct B { + virtual ~B() = default; + virtual std::string virtMethod1() { return "B::virtual1"; } + virtual std::string virtMethod2(); +}; + + std::string B::virtMethod2() { return "B::virtual2"; } + +struct C: public B { + std::string virtMethod1() override { return "C::virtual"; } + std::string virtMethod2() override; +}; + +std::string C::virtMethod2() { return "C::virtual"; } + +std::string lambda() { + // TODO: extract and return literal from lambda + return []() { + return "lambda"; + }(); +} + +struct TemplateString { + static constexpr char* val = "TEMPLATE"; + template<typename T> + // TODO: extract and return literal + std::string templateFunc() { return T::val; } + std::string templateFuncCall() { + return templateFunc<TemplateString>(); + } +}; + +template <class T> +std::basic_string<T> templateStringConditional() { + if constexpr(std::is_same_v<T, wchar_t>) { + return L"TEMPLATE"; + } else { + return "TEMPLATE"; + } +} + +template <class T> +std::basic_string<T> templateStringMixedConditional() { + if constexpr(std::is_same_v<T, wchar_t>) { + return L"TEMPLATE"; + } else { + std::string s = "haha"; + return s; + } +} + +void UseTemplateStringConditional() { + templateStringConditional<char>(); + templateStringConditional<wchar_t>(); + + templateStringMixedConditional<char>(); + templateStringMixedConditional<wchar_t>(); +} + +std::string& Ref() { + static std::string s = "Ref"; + return s; +} + +const std::string& ConstRef() { + static std::string s = "ConstRef"; + return s; +} + +auto autoReturn() { + // Deduced to const char* + return "autoReturn"; +} + +template <class T> +std::basic_string<T> templateString() { +// Intentionally skip templates + return L"TEMPLATE"; +} +std::wstring returnTemplateString() { + return templateString<wchar_t>(); +} + +template <typename R> R f() { +// Intentionally skip templates + return "str"; +} +template std::string f(); + +struct stringOperator { + operator std::string() const { + return "conversion"; + } +}; + +std::string safeFunctionWithLambda() { + auto lambda = []() -> std::string { + std::string local = "unsafe"; + return local; + }; + // TODO: fix hasDescendant(returnStmt(hasReturnValue... which is too strict + return "safe literal"; +} + +using Handle = std::wstring; + +#ifdef HANDLE_SUPPORTED +Handle handle_or_string(); +#else +std::string handle_or_string(); +#endif + +#ifdef HANDLE_SUPPORTED +Handle +#else +std::string +#endif +my_function() { + return handle_or_string(); +} + + +namespace TemplatedFunctions { +template <typename MojoType> +extern std::string GetErrorString(const MojoType& mojo_type); + +class A{}; +class B{}; + +template <> +std::string GetErrorString(const A& a) { + return "Can be string_view"; +} + +template <> +std::string GetErrorString(const B& a) { + std::string s("Cannot be string_view"); + return s; +} +} // namespace TemplatedFunctions _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
