llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Helmut Januschka (hjanuschka)
<details>
<summary>Changes</summary>
Finds function parameters declared as `const T&` that are unconditionally
copied into local variables, and suggests passing by value with `std::move`.
When a `const T&` parameter is always copied, passing by value instead
allows callers with rvalues to avoid the copy entirely through move
construction.
```cpp
// Before
void process(const std::string &s) {
std::string local = s;
// use local...
}
// After
void process(std::string s) {
std::string local = std::move(s);
// use local...
}
```
The check only triggers when:
- The parameter type is a non-trivially-copyable class or struct
- The type has a non-deleted move constructor
- The parameter is copied via copy construction into a local variable
Constructor member initializer lists are not handled, as that case is already
covered by `modernize-pass-by-value`.
Fixes #<!-- -->94798.
---
Full diff: https://github.com/llvm/llvm-project/pull/182024.diff
8 Files Affected:
- (modified) clang-tools-extra/clang-tidy/performance/CMakeLists.txt (+1)
- (added) clang-tools-extra/clang-tidy/performance/ConstRefCopyCheck.cpp (+104)
- (added) clang-tools-extra/clang-tidy/performance/ConstRefCopyCheck.h (+35)
- (modified) clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
(+3)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6)
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
- (added)
clang-tools-extra/docs/clang-tidy/checks/performance/const-ref-copy.rst (+39)
- (added)
clang-tools-extra/test/clang-tidy/checkers/performance/const-ref-copy.cpp (+84)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index 4dba117e1ee54..ee40862509325 100644
--- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangTidyPerformanceModule STATIC
AvoidEndlCheck.cpp
+ ConstRefCopyCheck.cpp
EnumSizeCheck.cpp
FasterStringFindCheck.cpp
ForRangeCopyCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/ConstRefCopyCheck.cpp
b/clang-tools-extra/clang-tidy/performance/ConstRefCopyCheck.cpp
new file mode 100644
index 0000000000000..403c34cf98142
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/ConstRefCopyCheck.cpp
@@ -0,0 +1,104 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "ConstRefCopyCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::performance {
+
+void ConstRefCopyCheck::registerMatchers(MatchFinder *Finder) {
+ // Match a function/method/constructor with a const T& parameter where
+ // T is a non-trivially-copyable record type, and the parameter is used
+ // to copy-construct a local variable or initialize a member field.
+
+ const auto ConstRefParam =
+ parmVarDecl(hasType(qualType(references(qualType(isConstQualified())))),
+ unless(isExpansionInSystemHeader()));
+
+ // Case 1: Local variable copy-initialized from the parameter.
+ // void f(const T& t) { T local = t; ... }
+ Finder->addMatcher(
+ varDecl(hasLocalStorage(),
+ hasInitializer(ignoringImplicit(
+ cxxConstructExpr(
+ hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
+ hasArgument(0, ignoringImplicit(declRefExpr(
+ to(ConstRefParam.bind("param"))))))
+ .bind("construct"))),
+ unless(isExpansionInSystemHeader()))
+ .bind("localVar"),
+ this);
+
+ // Case 2: Constructor member initializer copying the parameter.
+ // S(const T& t) : member(t) {}
+ Finder->addMatcher(
+ cxxConstructorDecl(
+ forEachConstructorInitializer(
+ cxxCtorInitializer(
+ isMemberInitializer(),
+ withInitializer(ignoringImplicit(
+ cxxConstructExpr(
+ hasDeclaration(
+ cxxConstructorDecl(isCopyConstructor())),
+ hasArgument(0, ignoringImplicit(declRefExpr(
+
to(ConstRefParam.bind("param"))))))
+ .bind("construct"))))
+ .bind("memberInit")))
+ .bind("ctor"),
+ this);
+}
+
+void ConstRefCopyCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param");
+ if (!Param)
+ return;
+
+ // Get the underlying type (strip the reference and const).
+ const QualType ParamType = Param->getType().getNonReferenceType();
+ const QualType UnqualType = ParamType.getUnqualifiedType();
+
+ // Only warn for non-trivially-copyable types where move is beneficial.
+ if (UnqualType->isBuiltinType())
+ return;
+ const CXXRecordDecl *Record = UnqualType->getAsCXXRecordDecl();
+ if (!Record || !Record->hasDefinition())
+ return;
+ if (Record->isTriviallyCopyable())
+ return;
+ // Must have a non-deleted move constructor for the suggestion to be
+ // beneficial.
+ bool HasUsableMove = false;
+ if (Record->needsImplicitMoveConstructor()) {
+ HasUsableMove = true;
+ } else {
+ for (const CXXConstructorDecl *Ctor : Record->ctors())
+ if (Ctor->isMoveConstructor() && !Ctor->isDeleted()) {
+ HasUsableMove = true;
+ break;
+ }
+ }
+ if (!HasUsableMove)
+ return;
+
+ if (const auto *LocalVar = Result.Nodes.getNodeAs<VarDecl>("localVar")) {
+ diag(Param->getLocation(),
+ "parameter %0 is copied into local variable %1; "
+ "consider passing by value and using 'std::move'")
+ << Param << LocalVar;
+ diag(LocalVar->getLocation(), "copy is here", DiagnosticIDs::Note);
+ } else if (Result.Nodes.getNodeAs<CXXCtorInitializer>("memberInit")) {
+ // modernize-pass-by-value already handles constructor cases.
+ // Skip to avoid overlapping diagnostics.
+ return;
+ }
+}
+
+} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/clang-tidy/performance/ConstRefCopyCheck.h
b/clang-tools-extra/clang-tidy/performance/ConstRefCopyCheck.h
new file mode 100644
index 0000000000000..93f9a27b68c24
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/ConstRefCopyCheck.h
@@ -0,0 +1,35 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_PERFORMANCE_CONSTREFCOPYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_CONSTREFCOPYCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::performance {
+
+/// Finds function parameters declared as 'const T&' that are unconditionally
+/// copied into local variables or member fields, and suggests changing the
+/// parameter to pass-by-value with 'std::move'.
+///
+/// For the user-facing documentation see:
+///
https://clang.llvm.org/extra/clang-tidy/checks/performance/const-ref-copy.html
+class ConstRefCopyCheck : public ClangTidyCheck {
+public:
+ ConstRefCopyCheck(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.CPlusPlus11;
+ }
+};
+
+} // namespace clang::tidy::performance
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_CONSTREFCOPYCHECK_H
diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
index 294a209e4c602..04830a5fd9b8b 100644
--- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -9,6 +9,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "AvoidEndlCheck.h"
+#include "ConstRefCopyCheck.h"
#include "EnumSizeCheck.h"
#include "FasterStringFindCheck.h"
#include "ForRangeCopyCheck.h"
@@ -37,6 +38,8 @@ class PerformanceModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<AvoidEndlCheck>("performance-avoid-endl");
+ CheckFactories.registerCheck<ConstRefCopyCheck>(
+ "performance-const-ref-copy");
CheckFactories.registerCheck<EnumSizeCheck>("performance-enum-size");
CheckFactories.registerCheck<FasterStringFindCheck>(
"performance-faster-string-find");
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 68bab88146241..53e3f29eb8940 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -133,6 +133,12 @@ New checks
Finds places where structured bindings could be used to decompose pairs and
suggests replacing them.
+- New :doc:`performance-const-ref-copy
+ <clang-tidy/checks/performance/const-ref-copy>` check.
+
+ Finds ``const T&`` parameters that are unconditionally copied into
+ local variables and suggests passing by value with ``std::move``.
+
- New :doc:`performance-string-view-conversions
<clang-tidy/checks/performance/string-view-conversions>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index c475870ed7b31..fdfa8efd69e68 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -350,6 +350,7 @@ Clang-Tidy Checks
:doc:`openmp-exception-escape <openmp/exception-escape>`,
:doc:`openmp-use-default-none <openmp/use-default-none>`,
:doc:`performance-avoid-endl <performance/avoid-endl>`, "Yes"
+ :doc:`performance-const-ref-copy <performance/const-ref-copy>`,
:doc:`performance-enum-size <performance/enum-size>`,
:doc:`performance-faster-string-find <performance/faster-string-find>`,
"Yes"
:doc:`performance-for-range-copy <performance/for-range-copy>`, "Yes"
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/performance/const-ref-copy.rst
b/clang-tools-extra/docs/clang-tidy/checks/performance/const-ref-copy.rst
new file mode 100644
index 0000000000000..20cf04b9735ca
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/const-ref-copy.rst
@@ -0,0 +1,39 @@
+.. title:: clang-tidy - performance-const-ref-copy
+
+performance-const-ref-copy
+==========================
+
+Finds function parameters declared as ``const T&`` that are
+unconditionally copied into a local variable, and suggests
+changing the parameter to pass-by-value with ``std::move``.
+
+When a ``const T&`` parameter is always copied, passing by value
+instead allows callers with rvalues to avoid the copy entirely
+through move construction.
+
+For example:
+
+.. code-block:: c++
+
+ // Before
+ void process(const std::string &s) {
+ std::string local = s;
+ // use local...
+ }
+
+ // After
+ void process(std::string s) {
+ std::string local = std::move(s);
+ // use local...
+ }
+
+The check only triggers when:
+
+- The parameter type is a non-trivially-copyable class or struct.
+- The type has a non-deleted move constructor.
+- The parameter is copied via copy construction into a local
+ variable.
+
+This check does not handle constructor member initializer lists,
+as that case is covered by :doc:`modernize-pass-by-value
+<../modernize/pass-by-value>`.
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/performance/const-ref-copy.cpp
b/clang-tools-extra/test/clang-tidy/checkers/performance/const-ref-copy.cpp
new file mode 100644
index 0000000000000..c879ae3fa06e4
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/const-ref-copy.cpp
@@ -0,0 +1,84 @@
+// RUN: %check_clang_tidy %s performance-const-ref-copy %t
+
+namespace std {
+template <typename T>
+class basic_string {
+public:
+ basic_string();
+ basic_string(const basic_string &);
+ basic_string(basic_string &&);
+ basic_string &operator=(const basic_string &);
+ basic_string &operator=(basic_string &&);
+ ~basic_string();
+ const char *c_str() const;
+ int size() const;
+};
+using string = basic_string<char>;
+
+template <typename T>
+class vector {
+public:
+ vector();
+ vector(const vector &);
+ vector(vector &&);
+ vector &operator=(const vector &);
+ vector &operator=(vector &&);
+ ~vector();
+ int size() const;
+};
+} // namespace std
+
+// Positive: const string& copied into local variable.
+void take_string(const std::string &s) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: parameter 's' is copied into
local variable 'copy'; consider passing by value and using 'std::move'
[performance-const-ref-copy]
+ std::string copy = s;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: note: copy is here
+ copy.c_str();
+}
+
+// Positive: const vector& copied into local.
+void take_vector(const std::vector<int> &v) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: parameter 'v' is copied into
local variable 'local'; consider passing by value and using 'std::move'
[performance-const-ref-copy]
+ std::vector<int> local = v;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: note: copy is here
+ local.size();
+}
+
+// Negative: trivially copyable type.
+struct Trivial {
+ int x;
+ int y;
+};
+
+void take_trivial(const Trivial &t) {
+ Trivial copy = t;
+}
+
+// Negative: no copy, just using the reference.
+void use_ref(const std::string &s) {
+ s.c_str();
+}
+
+// Negative: type has no move constructor (deleted).
+struct NoMove {
+ NoMove();
+ NoMove(const NoMove &);
+ NoMove &operator=(const NoMove &);
+ ~NoMove();
+ NoMove(NoMove &&) = delete;
+};
+
+void take_nomove(const NoMove &n) {
+ NoMove copy = n;
+}
+
+// Negative: not a const ref parameter, just a local.
+void local_copy() {
+ std::string a;
+ std::string b = a;
+}
+
+// Negative: parameter is not const ref.
+void take_value(std::string s) {
+ std::string copy = s;
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/182024
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
