llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Zinovy Nis (irishrover) <details> <summary>Changes</summary> Looks for redundant conversions from ``std::[w|u8|u16|u32]string_view`` to ``std::[...]string`` in call expressions expecting ``std::[...]string_view``. And fixes them. --- Full diff: https://github.com/llvm/llvm-project/pull/174288.diff 9 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+3) - (modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1) - (added) clang-tools-extra/clang-tidy/bugprone/RedundantStringViewConversionsCheck.cpp (+87) - (added) clang-tools-extra/clang-tidy/bugprone/RedundantStringViewConversionsCheck.h (+34) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+13-6) - (added) clang-tools-extra/docs/clang-tidy/checks/bugprone/redundant-string-view-conversions.rst (+34) - (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) - (modified) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string (+2) - (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/redundant-string-view-conversions.cpp (+45) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 4150442c25d61..0ca5105e2fb34 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -67,6 +67,7 @@ #include "RandomGeneratorSeedCheck.h" #include "RawMemoryCallOnNonTrivialTypeCheck.h" #include "RedundantBranchConditionCheck.h" +#include "RedundantStringViewConversionsCheck.h" #include "ReservedIdentifierCheck.h" #include "ReturnConstRefFromParameterCheck.h" #include "SharedPtrArrayMismatchCheck.h" @@ -176,6 +177,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-incorrect-enable-if"); CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>( "bugprone-incorrect-enable-shared-from-this"); + CheckFactories.registerCheck<RedundantStringViewConversionsCheck>( + "bugprone-redundant-string-view-conversions"); CheckFactories.registerCheck<UnintendedCharOstreamOutputCheck>( "bugprone-unintended-char-ostream-output"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index db1256d91d311..8153564c02674 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -69,6 +69,7 @@ add_clang_library(clangTidyBugproneModule STATIC RandomGeneratorSeedCheck.cpp RawMemoryCallOnNonTrivialTypeCheck.cpp RedundantBranchConditionCheck.cpp + RedundantStringViewConversionsCheck.cpp ReservedIdentifierCheck.cpp ReturnConstRefFromParameterCheck.cpp SharedPtrArrayMismatchCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/RedundantStringViewConversionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/RedundantStringViewConversionsCheck.cpp new file mode 100644 index 0000000000000..e39eb2f0dafb8 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/RedundantStringViewConversionsCheck.cpp @@ -0,0 +1,87 @@ +//===----------------------------------------------------------------------===// +// +// 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 "RedundantStringViewConversionsCheck.h" +#include "clang/AST/Expr.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +static auto getStringTypeMatcher(StringRef CharType) { + return hasCanonicalType(hasDeclaration(cxxRecordDecl(hasName(CharType)))); +} + +void RedundantStringViewConversionsCheck::registerMatchers( + MatchFinder *Finder) { + const auto IsStdString = getStringTypeMatcher("::std::basic_string"); + const auto IsStdStringView = getStringTypeMatcher("::std::basic_string_view"); + + // Выражение, которое может быть напрямую преобразовано в string_view: + // - string_view + // - строковый литерал + // - const char* + const auto ImplicitlyConvertibleToStringView = + expr(anyOf(hasType(IsStdStringView), stringLiteral(), + hasType(pointerType(pointee(isAnyCharacter()))))) + .bind("originalStringView"); + + Finder->addMatcher( + callExpr(forEachArgumentWithParam( + expr(hasType(IsStdStringView), + hasDescendant( + cxxFunctionalCastExpr( + hasType(IsStdString), + hasDescendant(cxxConstructExpr( + hasType(IsStdString), + hasArgument(0, + ignoringImplicit( + ImplicitlyConvertibleToStringView)), + unless(hasDeclaration( + cxxConstructorDecl(isCopyConstructor()))), + unless(hasDeclaration( + cxxConstructorDecl(isMoveConstructor())))))) + .bind("functionalCast"))) + .bind("expr"), + parmVarDecl(hasType(IsStdStringView)))), + this); +} + +void RedundantStringViewConversionsCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *ParamExpr = Result.Nodes.getNodeAs<Expr>("expr"); + if (!ParamExpr) + return; + + const auto *FuncCast = + Result.Nodes.getNodeAs<CXXFunctionalCastExpr>("functionalCast"); + if (!FuncCast) + return; + + const auto *OriginalExpr = Result.Nodes.getNodeAs<Expr>("originalStringView"); + if (!OriginalExpr) + return; + + StringRef OriginalText = Lexer::getSourceText( + CharSourceRange::getTokenRange(OriginalExpr->getSourceRange()), + *Result.SourceManager, getLangOpts()); + + if (OriginalText.empty()) + return; + + diag(FuncCast->getBeginLoc(), + "redundant conversion to %0 and then back to %1") + << FuncCast->getType().getAsString() << ParamExpr->getType().getAsString() + << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(FuncCast->getSourceRange()), + OriginalText); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/RedundantStringViewConversionsCheck.h b/clang-tools-extra/clang-tidy/bugprone/RedundantStringViewConversionsCheck.h new file mode 100644 index 0000000000000..6e08caea66ebe --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/RedundantStringViewConversionsCheck.h @@ -0,0 +1,34 @@ +//===----------------------------------------------------------------------===// +// +// 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_BUGPRONE_REDUNDANTSTRINGVIEWCONVERSIONSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_REDUNDANTSTRINGVIEWCONVERSIONSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Looks for redundant conversions from std::string_view to std::string in +/// call expressions expecting std::string_view. And fixes them. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/redundant-string-view-conversions.html +class RedundantStringViewConversionsCheck : public ClangTidyCheck { +public: + RedundantStringViewConversionsCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + 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; + } +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_REDUNDANTSTRINGVIEWCONVERSIONSCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 391c3a6b3db79..faa21b7dc66b2 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -111,10 +111,10 @@ Hover Code completion ^^^^^^^^^^^^^^^ -- Added a new ``MacroFilter`` configuration option to ``Completion`` to - allow fuzzy-matching with the ``FuzzyMatch`` option when suggesting - macros. ``ExactPrefix`` is the default, which retains previous - behavior of suggesting macros which match the prefix exactly. +- Added a new ``MacroFilter`` configuration option to ``Completion`` to + allow fuzzy-matching with the ``FuzzyMatch`` option when suggesting + macros. ``ExactPrefix`` is the default, which retains previous + behavior of suggesting macros which match the prefix exactly. Code actions ^^^^^^^^^^^^ @@ -205,7 +205,7 @@ Improvements to clang-tidy - Improved :program:`clang-tidy` by adding the `--removed-arg` option to remove arguments sent to the compiler when invoking Clang-Tidy. This option was also - added to :program:`run-clang-tidy.py` and :program:`clang-tidy-diff.py` and + added to :program:`run-clang-tidy.py` and :program:`clang-tidy-diff.py` and can be configured in the config file through the `RemovedArgs` option. - Deprecated the :program:`clang-tidy` ``zircon`` module. All checks have been @@ -234,6 +234,13 @@ New checks Detects default initialization (to 0) of variables with ``enum`` type where the enum has no enumerator with value of 0. +- New :doc:`bugprone-redundant-string-view-conversions + <clang-tidy/checks/bugprone/redundant-string-view-conversions>` check. + + Looks for redundant conversions from ``std::[w|u8|u16|u32]string_view`` to + ``std::[...]string`` in call expressions expecting ``std::[...]string_view``. + And fixes them. + - New :doc:`cppcoreguidelines-pro-bounds-avoid-unchecked-container-access <clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-access>` check. @@ -364,7 +371,7 @@ New check aliases keeping initial check as an alias to the new one. - Renamed :doc:`google-build-namespaces <clang-tidy/checks/google/build-namespaces>` to - :doc:`misc-anonymous-namespace-in-header + :doc:`misc-anonymous-namespace-in-header <clang-tidy/checks/misc/anonymous-namespace-in-header>` keeping initial check as an alias to the new one. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/redundant-string-view-conversions.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/redundant-string-view-conversions.rst new file mode 100644 index 0000000000000..9e03af8ad9b34 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/redundant-string-view-conversions.rst @@ -0,0 +1,34 @@ +.. title:: clang-tidy - bugprone-redundant-string-view-conversions + +bugprone-redundant-string-view-conversions +================================================= + +Looks for redundant conversions from ``std::[w|u8|u16|u32]string_view`` to +``std::[...]string`` in call expressions expecting ``std::[...]string_view``. +And fixes them. + + +Before: + + +.. code-block:: c++ + + void foo(int p1, std::string_view p2, double p3); + void bar(std::string_view sv) { + foo(42, std::string(sv), 3.14); // conversion to std::string is + // redundant as std::string_view + // is expected + foo(42, std::string("foo"), 3.14); // conversion to std::string is + // redundant as std::string_view + // is expected + } + +After: + +.. code-block:: c++ + + void foo(int p1, std::string_view p2, double p3); + void bar(std::string_view sv) { + foo(42, sv, 3.14); + foo(42, "foo", 3.14); + } diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index e0de2b9c2dada..9042d98235478 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -136,6 +136,7 @@ Clang-Tidy Checks :doc:`bugprone-random-generator-seed <bugprone/random-generator-seed>`, :doc:`bugprone-raw-memory-call-on-non-trivial-type <bugprone/raw-memory-call-on-non-trivial-type>`, :doc:`bugprone-redundant-branch-condition <bugprone/redundant-branch-condition>`, "Yes" + :doc:`bugprone-redundant-string-view-conversions <bugprone/redundant-string-view-conversions>`, "Yes" :doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes" :doc:`bugprone-return-const-ref-from-parameter <bugprone/return-const-ref-from-parameter>`, :doc:`bugprone-shared-ptr-array-mismatch <bugprone/shared-ptr-array-mismatch>`, "Yes" 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..c90a21df51479 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string @@ -25,6 +25,7 @@ struct basic_string { 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); + basic_string(basic_string_view<C, T> sv); ~basic_string(); @@ -100,6 +101,7 @@ struct basic_string_view { const C *str; constexpr basic_string_view(const C* s) : str(s) {} + basic_string_view(const basic_string<C, T>&) {} const C *data() const; diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/redundant-string-view-conversions.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/redundant-string-view-conversions.cpp new file mode 100644 index 0000000000000..68ba0fb613f3b --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/redundant-string-view-conversions.cpp @@ -0,0 +1,45 @@ +// RUN: %check_clang_tidy -std=c++17 %s bugprone-redundant-string-view-conversions %t -- \ +// RUN: -- -isystem %clang_tidy_headers + +#include <string> + +void foo1(int p1, std::string_view p2, double p3); +void foo2(int p1, std::wstring_view p2, double p3); +void foo3(int p1, const std::string& p2, double p3); + +void bar_positive(std::string_view sv, std::wstring_view wsv) { + // string(string_view) + // + foo1(42, std::string(sv), 3.14); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant conversion to std::string and then back to std::string_view [bugprone-redundant-string-view-conversions] + // CHECK-FIXES: foo1(42, sv, 3.14); + + foo1(42, std::string("hello, world"), 3.14); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant conversion to std::string and then back to std::string_view [bugprone-redundant-string-view-conversions] + // CHECK-FIXES: foo1(42, "hello, world", 3.14); + + const char *ptr = "Hello, world"; + foo1(42, std::string(ptr), 3.14); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant conversion to std::string and then back to std::string_view [bugprone-redundant-string-view-conversions] + // CHECK-FIXES: foo1(42, ptr, 3.14); + + // wstring(wstring_view) + // + foo2(42, std::wstring(wsv), 3.14); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant conversion to std::wstring and then back to std::wstring_view [bugprone-redundant-string-view-conversions] + // CHECK-FIXES: foo2(42, wsv, 3.14); + + const wchar_t *wptr = L"Hello, world"; + foo2(42, std::wstring(wptr), 3.14); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant conversion to std::wstring and then back to std::wstring_view [bugprone-redundant-string-view-conversions] + // CHECK-FIXES: foo2(42, wptr, 3.14); +} + +void bar_negaitve(std::string_view sv, std::wstring_view wsv) { + // No warnings expected: already string_view + foo1(42, sv, 3.14); + + // No warnings expected: string parameter, not string-view + foo3(42, std::string(sv), 3.14); + foo3(42, std::string("hello, world"), 3.14); +} `````````` </details> https://github.com/llvm/llvm-project/pull/174288 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
