[clang-tools-extra] [clang-tidy] Add check performance-lost-std-move (PR #139525)
Vasiliy Kulikov via cfe-commits
cfe-commits at lists.llvm.org
Sun Jun 1 01:20:30 PDT 2025
https://github.com/segoon updated https://github.com/llvm/llvm-project/pull/139525
>From 3abbce9f817f6d09f9a5b7549a8122c80821eaf8 Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Mon, 12 May 2025 13:05:43 +0300
Subject: [PATCH 01/16] [clang-tidy] Add check performance-lost-std-move
---
.../clang-tidy/performance/CMakeLists.txt | 1 +
.../performance/LostStdMoveCheck.cpp | 96 ++++++++++++++++
.../clang-tidy/performance/LostStdMoveCheck.h | 37 ++++++
.../performance/PerformanceTidyModule.cpp | 2 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../checks/performance/lost-std-move.rst | 14 +++
.../checkers/performance/lost-std-move.cpp | 108 ++++++++++++++++++
8 files changed, 264 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index 81128ff086021..333abd10a583a 100644
--- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -12,6 +12,7 @@ add_clang_library(clangTidyPerformanceModule
InefficientAlgorithmCheck.cpp
InefficientStringConcatenationCheck.cpp
InefficientVectorOperationCheck.cpp
+ LostStdMoveCheck.cpp
MoveConstArgCheck.cpp
MoveConstructorInitCheck.cpp
NoAutomaticMoveCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
new file mode 100644
index 0000000000000..26148e1d26de9
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
@@ -0,0 +1,96 @@
+//===--- LostStdMoveCheck.cpp - clang-tidy --------------------------------===//
+//
+// 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 "LostStdMoveCheck.h"
+#include "../utils/DeclRefExprUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::performance {
+
+using utils::decl_ref_expr::allDeclRefExprs;
+
+AST_MATCHER(CXXRecordDecl, hasTrivialMoveConstructor) {
+ return Node.hasDefinition() && Node.hasTrivialMoveConstructor();
+}
+
+void LostStdMoveCheck::registerMatchers(MatchFinder *Finder) {
+ auto returnParent =
+ hasParent(expr(hasParent(cxxConstructExpr(hasParent(returnStmt())))));
+
+ Finder->addMatcher(
+ declRefExpr(
+ // not "return x;"
+ unless(returnParent),
+
+ unless(hasType(namedDecl(hasName("::std::string_view")))),
+
+ // non-trivial type
+ hasType(hasCanonicalType(hasDeclaration(cxxRecordDecl()))),
+
+ // non-trivial X(X&&)
+ unless(hasType(hasCanonicalType(
+ hasDeclaration(cxxRecordDecl(hasTrivialMoveConstructor()))))),
+
+ // Not in a cycle
+ unless(hasAncestor(forStmt())), unless(hasAncestor(doStmt())),
+ unless(hasAncestor(whileStmt())),
+
+ // only non-X&
+ unless(hasDeclaration(
+ varDecl(hasType(qualType(lValueReferenceType()))))),
+
+ hasDeclaration(
+ varDecl(hasAncestor(functionDecl().bind("func"))).bind("decl")),
+
+ hasParent(expr(hasParent(cxxConstructExpr())).bind("use_parent")))
+ .bind("use"),
+ this);
+}
+
+const Expr *LostStdMoveCheck::getLastVarUsage(const VarDecl &Var,
+ const Decl &Func,
+ ASTContext &Context) {
+ auto Exprs = allDeclRefExprs(Var, Func, Context);
+
+ const Expr *LastExpr = nullptr;
+ for (const auto &Expr : Exprs) {
+ if (!LastExpr)
+ LastExpr = Expr;
+
+ if (LastExpr->getBeginLoc() < Expr->getBeginLoc())
+ LastExpr = Expr;
+ }
+
+ return LastExpr;
+}
+
+void LostStdMoveCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("decl");
+ const auto *MatchedFunc = Result.Nodes.getNodeAs<FunctionDecl>("func");
+ const auto *MatchedUse = Result.Nodes.getNodeAs<Expr>("use");
+ const auto *MatchedUseCall = Result.Nodes.getNodeAs<CallExpr>("use_parent");
+
+ if (MatchedUseCall)
+ return;
+
+ const auto *LastUsage =
+ getLastVarUsage(*MatchedDecl, *MatchedFunc, *Result.Context);
+ if (LastUsage == nullptr)
+ return;
+
+ if (LastUsage->getBeginLoc() > MatchedUse->getBeginLoc()) {
+ // "use" is not the last reference to x
+ return;
+ }
+
+ diag(LastUsage->getBeginLoc(), "Could be std::move()");
+}
+
+} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h
new file mode 100644
index 0000000000000..c62c3f6448a82
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h
@@ -0,0 +1,37 @@
+//===--- LostStdMoveCheck.h - clang-tidy ------------------------*- C++ -*-===//
+//
+// 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_LOSTSTDMOVECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_LOSTSTDMOVECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::performance {
+
+/// Search for lost std::move().
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance/lost-std-move.html
+class LostStdMoveCheck : public ClangTidyCheck {
+public:
+ LostStdMoveCheck(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.CPlusPlus;
+ }
+
+private:
+ const Expr *getLastVarUsage(const VarDecl &Var, const Decl &Func,
+ ASTContext &Context);
+};
+
+} // namespace clang::tidy::performance
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_LOSTSTDMOVECHECK_H
diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
index 9e0fa6f88b36a..6c45f8678fe63 100644
--- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -17,6 +17,7 @@
#include "InefficientAlgorithmCheck.h"
#include "InefficientStringConcatenationCheck.h"
#include "InefficientVectorOperationCheck.h"
+#include "LostStdMoveCheck.h"
#include "MoveConstArgCheck.h"
#include "MoveConstructorInitCheck.h"
#include "NoAutomaticMoveCheck.h"
@@ -49,6 +50,7 @@ class PerformanceModule : public ClangTidyModule {
"performance-inefficient-string-concatenation");
CheckFactories.registerCheck<InefficientVectorOperationCheck>(
"performance-inefficient-vector-operation");
+ CheckFactories.registerCheck<LostStdMoveCheck>("performance-lost-std-move");
CheckFactories.registerCheck<MoveConstArgCheck>(
"performance-move-const-arg");
CheckFactories.registerCheck<MoveConstructorInitCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d91748e4cef1..17da163ff041c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -212,6 +212,11 @@ New checks
Recommends the smallest possible underlying type for an ``enum`` or ``enum``
class based on the range of its enumerators.
+- New :doc:`performance-lost-std-move
+ <clang-tidy/checks/performance/lost-std-move>` check.
+
+ Searches for lost std::move().
+
- New :doc:`readability-reference-to-constructed-temporary
<clang-tidy/checks/readability/reference-to-constructed-temporary>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 31f0e090db1d7..2eba4aacb2c33 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -321,6 +321,7 @@ Clang-Tidy Checks
:doc:`performance-inefficient-algorithm <performance/inefficient-algorithm>`, "Yes"
:doc:`performance-inefficient-string-concatenation <performance/inefficient-string-concatenation>`,
:doc:`performance-inefficient-vector-operation <performance/inefficient-vector-operation>`, "Yes"
+ :doc:`performance-lost-std-move <performance/lost-std-move>`, "Yes"
:doc:`performance-move-const-arg <performance/move-const-arg>`, "Yes"
:doc:`performance-move-constructor-init <performance/move-constructor-init>`,
:doc:`performance-no-automatic-move <performance/no-automatic-move>`,
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst
new file mode 100644
index 0000000000000..ded49de7b8126
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - performance-lost-std-move
+
+performance-lost-std-move
+=========================
+
+The check warns if copy constructor is used instead of std::move().
+
+.. code-block:: c++
+
+ void f(X);
+
+ void g(X x) {
+ f(x); // warning: Could be std::move() [performance-lost-std-move]
+ }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
new file mode 100644
index 0000000000000..ce2d1b972dbd5
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
@@ -0,0 +1,108 @@
+// RUN: %check_clang_tidy %s performance-lost-std-move %t
+
+namespace std {
+
+template<typename T>
+class shared_ptr {
+public:
+ T& operator*() { return reinterpret_cast<T&>(*this); }
+ shared_ptr() {}
+ shared_ptr(const shared_ptr<T>&) {}
+};
+
+template<typename T>
+T&& move(T&)
+{
+}
+
+} // namespace std
+
+int f(std::shared_ptr<int>);
+
+void f_arg(std::shared_ptr<int> ptr)
+{
+ if (*ptr)
+ f(ptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Could be std::move() [performance-lost-std-move]
+}
+
+void f_rvalue_ref(std::shared_ptr<int>&& ptr)
+{
+ if (*ptr)
+ f(ptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Could be std::move() [performance-lost-std-move]
+}
+
+using SharedPtr = std::shared_ptr<int>;
+void f_using(SharedPtr ptr)
+{
+ if (*ptr)
+ f(ptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Could be std::move() [performance-lost-std-move]
+}
+
+void f_local()
+{
+ std::shared_ptr<int> ptr;
+ if (*ptr)
+ f(ptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Could be std::move() [performance-lost-std-move]
+}
+
+void f_move()
+{
+ std::shared_ptr<int> ptr;
+ if (*ptr)
+ f(std::move(ptr));
+}
+
+void f_ref(std::shared_ptr<int> &ptr)
+{
+ if (*ptr)
+ f(ptr);
+}
+
+std::shared_ptr<int> f_return()
+{
+ std::shared_ptr<int> ptr;
+ return ptr;
+}
+
+void f_still_used(std::shared_ptr<int> ptr)
+{
+ if (*ptr)
+ f(ptr);
+
+ *ptr = 1;
+ *ptr = *ptr;
+}
+
+void f_cycle1()
+{
+ std::shared_ptr<int> ptr;
+ for(;;)
+ f(ptr);
+}
+
+void f_cycle2()
+{
+ std::shared_ptr<int> ptr;
+ for(int i=0; i<5; i++)
+ f(ptr);
+}
+
+void f_cycle3()
+{
+ std::shared_ptr<int> ptr;
+ while (*ptr) {
+ f(ptr);
+ }
+}
+
+void f_cycle4()
+{
+ std::shared_ptr<int> ptr;
+ do {
+ f(ptr);
+ } while (*ptr);
+}
>From 4a1a77baacaec68c577e8fd6bb4cfe58d7496ac0 Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Mon, 12 May 2025 15:13:14 +0300
Subject: [PATCH 02/16] f(x, x)
---
.../performance/LostStdMoveCheck.cpp | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
index 26148e1d26de9..228d654b39aef 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
@@ -24,6 +24,9 @@ void LostStdMoveCheck::registerMatchers(MatchFinder *Finder) {
auto returnParent =
hasParent(expr(hasParent(cxxConstructExpr(hasParent(returnStmt())))));
+ auto outermostExpr = expr(unless(hasParent(expr())));
+ auto leafStatement = stmt(outermostExpr, unless(hasDescendant(outermostExpr)));
+
Finder->addMatcher(
declRefExpr(
// not "return x;"
@@ -46,6 +49,8 @@ void LostStdMoveCheck::registerMatchers(MatchFinder *Finder) {
unless(hasDeclaration(
varDecl(hasType(qualType(lValueReferenceType()))))),
+ hasAncestor(leafStatement.bind("leaf_statement")),
+
hasDeclaration(
varDecl(hasAncestor(functionDecl().bind("func"))).bind("decl")),
@@ -71,11 +76,19 @@ const Expr *LostStdMoveCheck::getLastVarUsage(const VarDecl &Var,
return LastExpr;
}
+template <typename Node>
+void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
+ llvm::SmallPtrSet<const Node *, 16> &Nodes) {
+ for (const auto &Match : Matches)
+ Nodes.insert(Match.getNodeAs<Node>(ID));
+}
+
void LostStdMoveCheck::check(const MatchFinder::MatchResult &Result) {
const auto *MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("decl");
const auto *MatchedFunc = Result.Nodes.getNodeAs<FunctionDecl>("func");
const auto *MatchedUse = Result.Nodes.getNodeAs<Expr>("use");
const auto *MatchedUseCall = Result.Nodes.getNodeAs<CallExpr>("use_parent");
+ const auto *MatchedLeafStatement = Result.Nodes.getNodeAs<Stmt>("leaf_statement");
if (MatchedUseCall)
return;
@@ -90,6 +103,15 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult &Result) {
return;
}
+ // Calculate X usage count in the statement
+ llvm::SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
+ auto Matches = match(findAll(declRefExpr(to(varDecl(equalsNode(MatchedDecl)))).bind("ref")), *MatchedLeafStatement, *Result.Context);
+ extractNodesByIdTo(Matches, "ref", DeclRefs);
+ if (DeclRefs.size() > 1) {
+ // Unspecified order of evaluation, e.g. f(x, x)
+ return;
+ }
+
diag(LastUsage->getBeginLoc(), "Could be std::move()");
}
>From f74066dffc49aef6c77417021c134c001820fa8b Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Mon, 12 May 2025 15:14:18 +0300
Subject: [PATCH 03/16] test
---
.../test/clang-tidy/checkers/performance/lost-std-move.cpp | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
index ce2d1b972dbd5..a4a2550971cac 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
@@ -106,3 +106,9 @@ void f_cycle4()
f(ptr);
} while (*ptr);
}
+
+int f_multiple_usages()
+{
+ std::shared_ptr<int> ptr;
+ return f(ptr) + f(ptr);
+}
>From edd4800851de89dc3e70b5b483e89fb3c0220fbc Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Mon, 26 May 2025 12:37:03 +0300
Subject: [PATCH 04/16] review fixes
---
.../performance/LostStdMoveCheck.cpp | 73 +++++++++----------
.../clang-tidy/performance/LostStdMoveCheck.h | 6 +-
2 files changed, 37 insertions(+), 42 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
index 228d654b39aef..cb6700fca8489 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
@@ -16,16 +16,31 @@ namespace clang::tidy::performance {
using utils::decl_ref_expr::allDeclRefExprs;
+static const Expr* getLastVarUsage(const VarDecl& Var, const Decl& Func,
+ ASTContext& Context) {
+ auto Exprs = allDeclRefExprs(Var, Func, Context);
+
+ const Expr* LastExpr = nullptr;
+ for (const clang::DeclRefExpr* Expr : Exprs) {
+ if (!LastExpr) LastExpr = Expr;
+
+ if (LastExpr->getBeginLoc() < Expr->getBeginLoc()) LastExpr = Expr;
+ }
+
+ return LastExpr;
+}
+
AST_MATCHER(CXXRecordDecl, hasTrivialMoveConstructor) {
return Node.hasDefinition() && Node.hasTrivialMoveConstructor();
}
-void LostStdMoveCheck::registerMatchers(MatchFinder *Finder) {
+void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
auto returnParent =
hasParent(expr(hasParent(cxxConstructExpr(hasParent(returnStmt())))));
auto outermostExpr = expr(unless(hasParent(expr())));
- auto leafStatement = stmt(outermostExpr, unless(hasDescendant(outermostExpr)));
+ auto leafStatement =
+ stmt(outermostExpr, unless(hasDescendant(outermostExpr)));
Finder->addMatcher(
declRefExpr(
@@ -49,7 +64,7 @@ void LostStdMoveCheck::registerMatchers(MatchFinder *Finder) {
unless(hasDeclaration(
varDecl(hasType(qualType(lValueReferenceType()))))),
- hasAncestor(leafStatement.bind("leaf_statement")),
+ hasAncestor(leafStatement.bind("leaf_statement")),
hasDeclaration(
varDecl(hasAncestor(functionDecl().bind("func"))).bind("decl")),
@@ -59,44 +74,26 @@ void LostStdMoveCheck::registerMatchers(MatchFinder *Finder) {
this);
}
-const Expr *LostStdMoveCheck::getLastVarUsage(const VarDecl &Var,
- const Decl &Func,
- ASTContext &Context) {
- auto Exprs = allDeclRefExprs(Var, Func, Context);
-
- const Expr *LastExpr = nullptr;
- for (const auto &Expr : Exprs) {
- if (!LastExpr)
- LastExpr = Expr;
-
- if (LastExpr->getBeginLoc() < Expr->getBeginLoc())
- LastExpr = Expr;
- }
-
- return LastExpr;
-}
-
template <typename Node>
void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
- llvm::SmallPtrSet<const Node *, 16> &Nodes) {
- for (const auto &Match : Matches)
+ llvm::SmallPtrSet<const Node*, 16>& Nodes) {
+ for (const BoundNodes& Match : Matches)
Nodes.insert(Match.getNodeAs<Node>(ID));
}
-void LostStdMoveCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("decl");
- const auto *MatchedFunc = Result.Nodes.getNodeAs<FunctionDecl>("func");
- const auto *MatchedUse = Result.Nodes.getNodeAs<Expr>("use");
- const auto *MatchedUseCall = Result.Nodes.getNodeAs<CallExpr>("use_parent");
- const auto *MatchedLeafStatement = Result.Nodes.getNodeAs<Stmt>("leaf_statement");
+void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
+ const auto* MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("decl");
+ const auto* MatchedFunc = Result.Nodes.getNodeAs<FunctionDecl>("func");
+ const auto* MatchedUse = Result.Nodes.getNodeAs<Expr>("use");
+ const auto* MatchedUseCall = Result.Nodes.getNodeAs<CallExpr>("use_parent");
+ const auto* MatchedLeafStatement =
+ Result.Nodes.getNodeAs<Stmt>("leaf_statement");
- if (MatchedUseCall)
- return;
+ if (MatchedUseCall) return;
- const auto *LastUsage =
+ const auto* LastUsage =
getLastVarUsage(*MatchedDecl, *MatchedFunc, *Result.Context);
- if (LastUsage == nullptr)
- return;
+ if (LastUsage == nullptr) return;
if (LastUsage->getBeginLoc() > MatchedUse->getBeginLoc()) {
// "use" is not the last reference to x
@@ -104,15 +101,17 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult &Result) {
}
// Calculate X usage count in the statement
- llvm::SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
- auto Matches = match(findAll(declRefExpr(to(varDecl(equalsNode(MatchedDecl)))).bind("ref")), *MatchedLeafStatement, *Result.Context);
+ llvm::SmallPtrSet<const DeclRefExpr*, 16> DeclRefs;
+ ArrayRef<BoundNodes> Matches = match(
+ findAll(declRefExpr(to(varDecl(equalsNode(MatchedDecl)))).bind("ref")),
+ *MatchedLeafStatement, *Result.Context);
extractNodesByIdTo(Matches, "ref", DeclRefs);
if (DeclRefs.size() > 1) {
// Unspecified order of evaluation, e.g. f(x, x)
return;
}
- diag(LastUsage->getBeginLoc(), "Could be std::move()");
+ diag(LastUsage->getBeginLoc(), "could be std::move()");
}
-} // namespace clang::tidy::performance
+} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h
index c62c3f6448a82..d9bdc1eeffdd7 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h
@@ -24,12 +24,8 @@ class LostStdMoveCheck : public ClangTidyCheck {
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.CPlusPlus;
+ return LangOpts.CPlusPlus11;
}
-
-private:
- const Expr *getLastVarUsage(const VarDecl &Var, const Decl &Func,
- ASTContext &Context);
};
} // namespace clang::tidy::performance
>From 5359c69e758d6e2f03f19785d4e3354dc81572f5 Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Mon, 26 May 2025 15:06:26 +0300
Subject: [PATCH 05/16] auto -> Expr
---
clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
index cb6700fca8489..1683584928cde 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
@@ -91,7 +91,7 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
if (MatchedUseCall) return;
- const auto* LastUsage =
+ const Expr* LastUsage =
getLastVarUsage(*MatchedDecl, *MatchedFunc, *Result.Context);
if (LastUsage == nullptr) return;
>From 5bb6af6b3c9f5eef4b662370fec0147ceed2809e Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Mon, 26 May 2025 15:19:12 +0300
Subject: [PATCH 06/16] FixIt
---
.../clang-tidy/performance/LostStdMoveCheck.cpp | 10 +++++++++-
.../clang-tidy/checkers/performance/lost-std-move.cpp | 8 ++++----
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
index 1683584928cde..2f7521a855dff 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
@@ -9,6 +9,7 @@
#include "LostStdMoveCheck.h"
#include "../utils/DeclRefExprUtils.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
using namespace clang::ast_matchers;
@@ -111,7 +112,14 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
return;
}
- diag(LastUsage->getBeginLoc(), "could be std::move()");
+
+ const SourceManager &Source = Result.Context->getSourceManager();
+ const auto Range = CharSourceRange::getTokenRange(LastUsage->getSourceRange());
+ const StringRef NeedleExprCode = Lexer::getSourceText(
+ Range, Source,
+ Result.Context->getLangOpts());
+ diag(LastUsage->getBeginLoc(), "could be std::move()")
+ << FixItHint::CreateReplacement(Range, ("std::move(" + NeedleExprCode + ")").str());
}
} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
index a4a2550971cac..b88faace6de2e 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
@@ -23,14 +23,14 @@ void f_arg(std::shared_ptr<int> ptr)
{
if (*ptr)
f(ptr);
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Could be std::move() [performance-lost-std-move]
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: could be std::move() [performance-lost-std-move]
}
void f_rvalue_ref(std::shared_ptr<int>&& ptr)
{
if (*ptr)
f(ptr);
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Could be std::move() [performance-lost-std-move]
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: could be std::move() [performance-lost-std-move]
}
using SharedPtr = std::shared_ptr<int>;
@@ -38,7 +38,7 @@ void f_using(SharedPtr ptr)
{
if (*ptr)
f(ptr);
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Could be std::move() [performance-lost-std-move]
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: could be std::move() [performance-lost-std-move]
}
void f_local()
@@ -46,7 +46,7 @@ void f_local()
std::shared_ptr<int> ptr;
if (*ptr)
f(ptr);
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Could be std::move() [performance-lost-std-move]
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: could be std::move() [performance-lost-std-move]
}
void f_move()
>From 592d79ba1f4e1bf0b5347565cda50337bbdd3b1e Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Mon, 26 May 2025 15:27:49 +0300
Subject: [PATCH 07/16] f((x))
---
.../performance/LostStdMoveCheck.cpp | 27 +++++++++++++------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
index 2f7521a855dff..e9900b9d15d48 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
@@ -70,7 +70,18 @@ void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
hasDeclaration(
varDecl(hasAncestor(functionDecl().bind("func"))).bind("decl")),
- hasParent(expr(hasParent(cxxConstructExpr())).bind("use_parent")))
+ anyOf(
+
+ // f(x)
+ hasParent(expr(hasParent(cxxConstructExpr())).bind("use_parent")),
+
+ // f((x))
+ hasParent(parenExpr(hasParent(
+ expr(hasParent(cxxConstructExpr())).bind("use_parent"))))
+
+ )
+
+ )
.bind("use"),
this);
}
@@ -112,14 +123,14 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
return;
}
-
- const SourceManager &Source = Result.Context->getSourceManager();
- const auto Range = CharSourceRange::getTokenRange(LastUsage->getSourceRange());
- const StringRef NeedleExprCode = Lexer::getSourceText(
- Range, Source,
- Result.Context->getLangOpts());
+ const SourceManager& Source = Result.Context->getSourceManager();
+ const auto Range =
+ CharSourceRange::getTokenRange(LastUsage->getSourceRange());
+ const StringRef NeedleExprCode =
+ Lexer::getSourceText(Range, Source, Result.Context->getLangOpts());
diag(LastUsage->getBeginLoc(), "could be std::move()")
- << FixItHint::CreateReplacement(Range, ("std::move(" + NeedleExprCode + ")").str());
+ << FixItHint::CreateReplacement(
+ Range, ("std::move(" + NeedleExprCode + ")").str());
}
} // namespace clang::tidy::performance
>From 1710c2f128eb1b1f484923a234fb5bf79ddd0bb4 Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Sun, 1 Jun 2025 00:15:17 +0300
Subject: [PATCH 08/16] w
---
.../performance/LostStdMoveCheck.cpp | 89 +++++++++++++------
1 file changed, 64 insertions(+), 25 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
index e9900b9d15d48..20efc3af9ed7a 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
@@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//
#include "LostStdMoveCheck.h"
-#include "../utils/DeclRefExprUtils.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
@@ -15,11 +14,33 @@ using namespace clang::ast_matchers;
namespace clang::tidy::performance {
-using utils::decl_ref_expr::allDeclRefExprs;
+template <typename Node>
+void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
+ llvm::SmallPtrSet<const Node*, 16>& Nodes) {
+ for (const BoundNodes& Match : Matches)
+ Nodes.insert(Match.getNodeAs<Node>(ID));
+}
+
+llvm::SmallPtrSet<const DeclRefExpr*, 16> allDeclRefExprsHonourLambda(
+ const VarDecl& VarDecl, const Decl& Decl, ASTContext& Context) {
+ auto Matches = match(
+ decl(forEachDescendant(
+ declRefExpr(to(varDecl(equalsNode(&VarDecl))),
+
+ unless(hasAncestor(lambdaExpr(hasAnyCapture(lambdaCapture(
+ capturesVar(varDecl(equalsNode(&VarDecl))))))))
+
+ )
+ .bind("declRef"))),
+ Decl, Context);
+ llvm::SmallPtrSet<const DeclRefExpr*, 16> DeclRefs;
+ extractNodesByIdTo(Matches, "declRef", DeclRefs);
+ return DeclRefs;
+}
static const Expr* getLastVarUsage(const VarDecl& Var, const Decl& Func,
ASTContext& Context) {
- auto Exprs = allDeclRefExprs(Var, Func, Context);
+ auto Exprs = allDeclRefExprsHonourLambda(Var, Func, Context);
const Expr* LastExpr = nullptr;
for (const clang::DeclRefExpr* Expr : Exprs) {
@@ -36,17 +57,16 @@ AST_MATCHER(CXXRecordDecl, hasTrivialMoveConstructor) {
}
void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
- auto returnParent =
+ auto ReturnParent =
hasParent(expr(hasParent(cxxConstructExpr(hasParent(returnStmt())))));
- auto outermostExpr = expr(unless(hasParent(expr())));
- auto leafStatement =
- stmt(outermostExpr, unless(hasDescendant(outermostExpr)));
+ auto OutermostExpr = expr(unless(hasParent(expr())));
+ auto LeafStatement = stmt(OutermostExpr);
Finder->addMatcher(
declRefExpr(
// not "return x;"
- unless(returnParent),
+ unless(ReturnParent),
unless(hasType(namedDecl(hasName("::std::string_view")))),
@@ -58,14 +78,17 @@ void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
hasDeclaration(cxxRecordDecl(hasTrivialMoveConstructor()))))),
// Not in a cycle
- unless(hasAncestor(forStmt())), unless(hasAncestor(doStmt())),
+ unless(hasAncestor(forStmt())),
+
+ unless(hasAncestor(doStmt())),
+
unless(hasAncestor(whileStmt())),
// only non-X&
unless(hasDeclaration(
varDecl(hasType(qualType(lValueReferenceType()))))),
- hasAncestor(leafStatement.bind("leaf_statement")),
+ hasAncestor(LeafStatement.bind("leaf_statement")),
hasDeclaration(
varDecl(hasAncestor(functionDecl().bind("func"))).bind("decl")),
@@ -86,13 +109,6 @@ void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
this);
}
-template <typename Node>
-void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
- llvm::SmallPtrSet<const Node*, 16>& Nodes) {
- for (const BoundNodes& Match : Matches)
- Nodes.insert(Match.getNodeAs<Node>(ID));
-}
-
void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
const auto* MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("decl");
const auto* MatchedFunc = Result.Nodes.getNodeAs<FunctionDecl>("func");
@@ -101,21 +117,35 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
const auto* MatchedLeafStatement =
Result.Nodes.getNodeAs<Stmt>("leaf_statement");
- if (MatchedUseCall) return;
+ if (MatchedUseCall) {
+ return;
+ }
const Expr* LastUsage =
getLastVarUsage(*MatchedDecl, *MatchedFunc, *Result.Context);
- if (LastUsage == nullptr) return;
- if (LastUsage->getBeginLoc() > MatchedUse->getBeginLoc()) {
+ if (LastUsage && LastUsage->getBeginLoc() > MatchedUse->getBeginLoc()) {
// "use" is not the last reference to x
return;
}
+ if (LastUsage &&
+ LastUsage->getSourceRange() != MatchedUse->getSourceRange()) {
+ return;
+ }
+
// Calculate X usage count in the statement
llvm::SmallPtrSet<const DeclRefExpr*, 16> DeclRefs;
ArrayRef<BoundNodes> Matches = match(
- findAll(declRefExpr(to(varDecl(equalsNode(MatchedDecl)))).bind("ref")),
+ findAll(declRefExpr(
+
+ to(varDecl(equalsNode(MatchedDecl))),
+
+ unless(hasAncestor(lambdaExpr(hasAnyCapture(lambdaCapture(
+ capturesVar(varDecl(equalsNode(MatchedDecl))))))))
+
+ )
+ .bind("ref")),
*MatchedLeafStatement, *Result.Context);
extractNodesByIdTo(Matches, "ref", DeclRefs);
if (DeclRefs.size() > 1) {
@@ -125,12 +155,21 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
const SourceManager& Source = Result.Context->getSourceManager();
const auto Range =
- CharSourceRange::getTokenRange(LastUsage->getSourceRange());
+ CharSourceRange::getTokenRange(MatchedUse->getSourceRange());
const StringRef NeedleExprCode =
Lexer::getSourceText(Range, Source, Result.Context->getLangOpts());
- diag(LastUsage->getBeginLoc(), "could be std::move()")
- << FixItHint::CreateReplacement(
- Range, ("std::move(" + NeedleExprCode + ")").str());
+
+ if (NeedleExprCode == "=") {
+
+ diag(MatchedUse->getBeginLoc(), "could be std::move()")
+ << FixItHint::CreateInsertion(
+ MatchedUse->getBeginLoc(),
+ ("std::move(" + MatchedDecl->getName() + "),").str());
+ } else {
+ diag(MatchedUse->getBeginLoc(), "could be std::move()")
+ << FixItHint::CreateReplacement(
+ Range, ("std::move(" + NeedleExprCode + ")").str());
+ }
}
} // namespace clang::tidy::performance
>From 60e4aca43a3e0a343f4ef327d490b95b8c21da9a Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Sun, 1 Jun 2025 00:24:46 +0300
Subject: [PATCH 09/16] thread_local, extern, static
---
.../performance/LostStdMoveCheck.cpp | 2 ++
.../checkers/performance/lost-std-move.cpp | 28 +++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
index 20efc3af9ed7a..b9ef7cb9ad61b 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
@@ -117,6 +117,8 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
const auto* MatchedLeafStatement =
Result.Nodes.getNodeAs<Stmt>("leaf_statement");
+ if (!MatchedDecl->hasLocalStorage()) return;
+
if (MatchedUseCall) {
return;
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
index b88faace6de2e..a6f267129918c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
@@ -41,6 +41,27 @@ void f_using(SharedPtr ptr)
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: could be std::move() [performance-lost-std-move]
}
+void f_thread_local()
+{
+ thread_local std::shared_ptr<int> ptr;
+ if (*ptr)
+ f(ptr);
+}
+
+void f_static()
+{
+ static std::shared_ptr<int> ptr;
+ if (*ptr)
+ f(ptr);
+}
+
+void f_extern()
+{
+ extern std::shared_ptr<int> ptr;
+ if (*ptr)
+ f(ptr);
+}
+
void f_local()
{
std::shared_ptr<int> ptr;
@@ -112,3 +133,10 @@ int f_multiple_usages()
std::shared_ptr<int> ptr;
return f(ptr) + f(ptr);
}
+
+#define FUN(x) f((x))
+int f_macro()
+{
+ std::shared_ptr<int> ptr;
+ return FUN(ptr);
+}
>From a27b1b301ff10a99b6a8532d439d58be16a53479 Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Sun, 1 Jun 2025 00:29:41 +0300
Subject: [PATCH 10/16] better test
---
.../checkers/performance/lost-std-move.cpp | 39 +++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
index a6f267129918c..f6dc90f9a8332 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
@@ -62,6 +62,12 @@ void f_extern()
f(ptr);
}
+std::shared_ptr<int> global;
+void f_global()
+{
+ f(global);
+}
+
void f_local()
{
std::shared_ptr<int> ptr;
@@ -140,3 +146,36 @@ int f_macro()
std::shared_ptr<int> ptr;
return FUN(ptr);
}
+
+void f_lambda_ref()
+{
+ std::shared_ptr<int> ptr;
+ auto Lambda = [&ptr]() mutable {
+ f(ptr);
+ };
+ Lambda();
+}
+
+void f_lambda()
+{
+ std::shared_ptr<int> ptr;
+ auto Lambda = [ptr]() mutable {
+ // CHECK-MESSAGES: [[@LINE-1]]:18: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // CHECK-FIXES: auto Lambda = [Mov]() mutable {
+ // Note: No fix, because a fix requires c++14.
+ f(ptr);
+ };
+ Lambda();
+}
+
+void f_lambda_assign()
+{
+ std::shared_ptr<int> ptr;
+ auto Lambda = [ptr = ptr]() mutable {
+ // CHECK-MESSAGES: [[@LINE-1]]:18: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+ // CHECK-FIXES: auto Lambda = [Mov]() mutable {
+ // Note: No fix, because a fix requires c++14.
+ f(ptr);
+ };
+ Lambda();
+}
>From 6aacd41124ebd053a9b3f09cdcd506ffbf49ce7d Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Sun, 1 Jun 2025 07:52:37 +0300
Subject: [PATCH 11/16] fixes
---
.../clang-tidy/performance/LostStdMoveCheck.cpp | 5 ++++-
.../checkers/performance/lost-std-move.cpp | 17 ++++++++++-------
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
index b9ef7cb9ad61b..47188ea87fc17 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
@@ -21,7 +21,7 @@ void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
Nodes.insert(Match.getNodeAs<Node>(ID));
}
-llvm::SmallPtrSet<const DeclRefExpr*, 16> allDeclRefExprsHonourLambda(
+static llvm::SmallPtrSet<const DeclRefExpr*, 16> allDeclRefExprsHonourLambda(
const VarDecl& VarDecl, const Decl& Decl, ASTContext& Context) {
auto Matches = match(
decl(forEachDescendant(
@@ -84,6 +84,9 @@ void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
unless(hasAncestor(whileStmt())),
+ // Not in a body of lambda
+ unless(hasAncestor(compoundStmt(hasAncestor(lambdaExpr())))),
+
// only non-X&
unless(hasDeclaration(
varDecl(hasType(qualType(lValueReferenceType()))))),
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
index f6dc90f9a8332..23693e578ef1b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
@@ -24,6 +24,7 @@ void f_arg(std::shared_ptr<int> ptr)
if (*ptr)
f(ptr);
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: could be std::move() [performance-lost-std-move]
+ // CHECK-FIXES: f(std::move(ptr));
}
void f_rvalue_ref(std::shared_ptr<int>&& ptr)
@@ -31,6 +32,7 @@ void f_rvalue_ref(std::shared_ptr<int>&& ptr)
if (*ptr)
f(ptr);
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: could be std::move() [performance-lost-std-move]
+ // CHECK-FIXES: f(std::move(ptr));
}
using SharedPtr = std::shared_ptr<int>;
@@ -74,6 +76,7 @@ void f_local()
if (*ptr)
f(ptr);
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: could be std::move() [performance-lost-std-move]
+ // CHECK-FIXES: f(std::move(ptr));
}
void f_move()
@@ -131,7 +134,7 @@ void f_cycle4()
std::shared_ptr<int> ptr;
do {
f(ptr);
- } while (*ptr);
+ } while (true);
}
int f_multiple_usages()
@@ -145,6 +148,8 @@ int f_macro()
{
std::shared_ptr<int> ptr;
return FUN(ptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: could be std::move() [performance-lost-std-move]
+ // CHECK-FIXES: return FUN(std::move(ptr));
}
void f_lambda_ref()
@@ -160,9 +165,8 @@ void f_lambda()
{
std::shared_ptr<int> ptr;
auto Lambda = [ptr]() mutable {
- // CHECK-MESSAGES: [[@LINE-1]]:18: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
- // CHECK-FIXES: auto Lambda = [Mov]() mutable {
- // Note: No fix, because a fix requires c++14.
+ // CHECK-MESSAGES: [[@LINE-1]]:18: warning: could be std::move() [performance-lost-std-move]
+ // CHECK-FIXES: auto Lambda = [std::move(ptr)]() mutable {
f(ptr);
};
Lambda();
@@ -172,9 +176,8 @@ void f_lambda_assign()
{
std::shared_ptr<int> ptr;
auto Lambda = [ptr = ptr]() mutable {
- // CHECK-MESSAGES: [[@LINE-1]]:18: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
- // CHECK-FIXES: auto Lambda = [Mov]() mutable {
- // Note: No fix, because a fix requires c++14.
+ // CHECK-MESSAGES: [[@LINE-1]]:24: warning: could be std::move() [performance-lost-std-move]
+ // CHECK-FIXES: auto Lambda = [ptr = std::move(ptr)]() mutable {
f(ptr);
};
Lambda();
>From 71cd708c34203f4caa7b97b8efa15d98ce1867c4 Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Sun, 1 Jun 2025 08:02:12 +0300
Subject: [PATCH 12/16] [=] test
---
.../clang-tidy/performance/LostStdMoveCheck.cpp | 2 +-
.../clang-tidy/checkers/performance/lost-std-move.cpp | 11 +++++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
index 47188ea87fc17..9699e66023c48 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
@@ -169,7 +169,7 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
diag(MatchedUse->getBeginLoc(), "could be std::move()")
<< FixItHint::CreateInsertion(
MatchedUse->getBeginLoc(),
- ("std::move(" + MatchedDecl->getName() + "),").str());
+ (MatchedDecl->getName() + " = std::move(" + MatchedDecl->getName() + "),").str());
} else {
diag(MatchedUse->getBeginLoc(), "could be std::move()")
<< FixItHint::CreateReplacement(
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
index 23693e578ef1b..a346f6e4a3f65 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp
@@ -182,3 +182,14 @@ void f_lambda_assign()
};
Lambda();
}
+
+void f_lambda_assign_all()
+{
+ std::shared_ptr<int> ptr;
+ auto Lambda = [=]() mutable {
+ // CHECK-MESSAGES: [[@LINE-1]]:18: warning: could be std::move() [performance-lost-std-move]
+ // CHECK-FIXES: auto Lambda = [ptr = std::move(ptr),=]() mutable {
+ f(ptr);
+ };
+ Lambda();
+}
>From 402ba55ed1b536c256c0f08b78a4dceb405a187d Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Sun, 1 Jun 2025 08:03:55 +0300
Subject: [PATCH 13/16] fix
---
clang-tools-extra/docs/ReleaseNotes.rst | 67 -------------------------
1 file changed, 67 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 2f5af851fcbac..e0f81a032c38d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -145,75 +145,8 @@ New checks
- New :doc:`readability-ambiguous-smartptr-reset-call
<clang-tidy/checks/readability/ambiguous-smartptr-reset-call>` check.
-<<<<<<< HEAD
- Detects implicit conversions between pointers of different levels of
- indirection.
-
-- New :doc:`bugprone-optional-value-conversion
- <clang-tidy/checks/bugprone/optional-value-conversion>` check.
-
- Detects potentially unintentional and redundant conversions where a value is
- extracted from an optional-like type and then used to create a new instance
- of the same optional-like type.
-
-- New :doc:`cppcoreguidelines-no-suspend-with-lock
- <clang-tidy/checks/cppcoreguidelines/no-suspend-with-lock>` check.
-
- Flags coroutines that suspend while a lock guard is in scope at the
- suspension point.
-
-- New :doc:`hicpp-ignored-remove-result
- <clang-tidy/checks/hicpp/ignored-remove-result>` check.
-
- Ensure that the result of ``std::remove``, ``std::remove_if`` and
- ``std::unique`` are not ignored according to rule 17.5.1.
-
-- New :doc:`misc-coroutine-hostile-raii
- <clang-tidy/checks/misc/coroutine-hostile-raii>` check.
-
- Detects when objects of certain hostile RAII types persists across suspension
- points in a coroutine. Such hostile types include scoped-lockable types and
- types belonging to a configurable denylist.
-
-- New :doc:`modernize-use-constraints
- <clang-tidy/checks/modernize/use-constraints>` check.
-
- Replace ``enable_if`` with C++20 requires clauses.
-
-- New :doc:`modernize-use-starts-ends-with
- <clang-tidy/checks/modernize/use-starts-ends-with>` check.
-
- Checks whether a ``find`` or ``rfind`` result is compared with 0 and suggests
- replacing with ``starts_with`` when the method exists in the class. Notably,
- this will work with ``std::string`` and ``std::string_view``.
-
-- New :doc:`modernize-use-std-numbers
- <clang-tidy/checks/modernize/use-std-numbers>` check.
-
- Finds constants and function calls to math functions that can be replaced
- with C++20's mathematical constants from the ``numbers`` header and
- offers fix-it hints.
-
-- New :doc:`performance-enum-size
- <clang-tidy/checks/performance/enum-size>` check.
-
- Recommends the smallest possible underlying type for an ``enum`` or ``enum``
- class based on the range of its enumerators.
-
-- New :doc:`performance-lost-std-move
- <clang-tidy/checks/performance/lost-std-move>` check.
-
- Searches for lost std::move().
-
-- New :doc:`readability-reference-to-constructed-temporary
- <clang-tidy/checks/readability/reference-to-constructed-temporary>` check.
-
- Detects C++ code where a reference variable is used to extend the lifetime
- of a temporary object that has just been constructed.
-=======
Finds potentially erroneous calls to ``reset`` method on smart pointers when
the pointee type also has a ``reset`` method.
->>>>>>> origin/main
New check aliases
^^^^^^^^^^^^^^^^^
>From fdf01b62a2b7619c5d3e12700c9d756296479e62 Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Sun, 1 Jun 2025 08:14:23 +0300
Subject: [PATCH 14/16] docs
---
.../clang-tidy/performance/LostStdMoveCheck.h | 3 ++-
clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++++
.../checks/performance/lost-std-move.rst | 20 ++++++++++++++++++-
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h
index d9bdc1eeffdd7..789f1233e7042 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h
@@ -13,7 +13,8 @@
namespace clang::tidy::performance {
-/// Search for lost std::move().
+/// Warns if copy constructor is used instead of std::move() and suggests a fix.
+/// It honours cycles, lambdas, and unspecified call order in compound expressions.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/performance/lost-std-move.html
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e0f81a032c38d..7f5e01b1fba1f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -136,6 +136,12 @@ New checks
Finds unintended character output from ``unsigned char`` and ``signed char``
to an ``ostream``.
+- New :doc:`performance-lost-std-move
+ <clang-tidy/checks/performance/lost-std-move>` check.
+
+ Warns if copy constructor is used instead of std::move() and suggests a fix.
+ It honours cycles, lambdas, and unspecified call order in compound expressions.
+
- New :doc:`portability-avoid-pragma-once
<clang-tidy/checks/portability/avoid-pragma-once>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst
index ded49de7b8126..13cf7b63440f7 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst
@@ -3,7 +3,8 @@
performance-lost-std-move
=========================
-The check warns if copy constructor is used instead of std::move().
+Warns if copy constructor is used instead of std::move() and suggests a fix.
+It honours cycles, lambdas, and unspecified call order in compound expressions.
.. code-block:: c++
@@ -12,3 +13,20 @@ The check warns if copy constructor is used instead of std::move().
void g(X x) {
f(x); // warning: Could be std::move() [performance-lost-std-move]
}
+
+It finds the last local variable usage, and if it is a copy, emits a warning.
+The check is based on pure AST matching and doesn't take into account any data flow information.
+Thus, it doesn't catch assign-after-copy cases.
+Also it doesn't notice variable references "behind the scenes":
+
+.. code-block:: c++
+
+ void f(X);
+
+ void g(X x) {
+ auto &y = x;
+ f(x); // emits a warning...
+ y.f(); // ...but it is still used
+ }
+
+Such rare cases should be silenced using `// NOLINT`.
>From 24357a23a9fe6c022638e9655e0e587df67d972b Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Sun, 1 Jun 2025 08:24:11 +0300
Subject: [PATCH 15/16] format
---
.../performance/LostStdMoveCheck.cpp | 60 ++++++++++---------
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
index 9699e66023c48..00ca42148df77 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
@@ -16,13 +16,14 @@ namespace clang::tidy::performance {
template <typename Node>
void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
- llvm::SmallPtrSet<const Node*, 16>& Nodes) {
- for (const BoundNodes& Match : Matches)
+ llvm::SmallPtrSet<const Node *, 16> &Nodes) {
+ for (const BoundNodes &Match : Matches)
Nodes.insert(Match.getNodeAs<Node>(ID));
}
-static llvm::SmallPtrSet<const DeclRefExpr*, 16> allDeclRefExprsHonourLambda(
- const VarDecl& VarDecl, const Decl& Decl, ASTContext& Context) {
+static llvm::SmallPtrSet<const DeclRefExpr *, 16>
+allDeclRefExprsHonourLambda(const VarDecl &VarDecl, const Decl &Decl,
+ ASTContext &Context) {
auto Matches = match(
decl(forEachDescendant(
declRefExpr(to(varDecl(equalsNode(&VarDecl))),
@@ -33,20 +34,22 @@ static llvm::SmallPtrSet<const DeclRefExpr*, 16> allDeclRefExprsHonourLambda(
)
.bind("declRef"))),
Decl, Context);
- llvm::SmallPtrSet<const DeclRefExpr*, 16> DeclRefs;
+ llvm::SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
extractNodesByIdTo(Matches, "declRef", DeclRefs);
return DeclRefs;
}
-static const Expr* getLastVarUsage(const VarDecl& Var, const Decl& Func,
- ASTContext& Context) {
+static const Expr *getLastVarUsage(const VarDecl &Var, const Decl &Func,
+ ASTContext &Context) {
auto Exprs = allDeclRefExprsHonourLambda(Var, Func, Context);
- const Expr* LastExpr = nullptr;
- for (const clang::DeclRefExpr* Expr : Exprs) {
- if (!LastExpr) LastExpr = Expr;
+ const Expr *LastExpr = nullptr;
+ for (const clang::DeclRefExpr *Expr : Exprs) {
+ if (!LastExpr)
+ LastExpr = Expr;
- if (LastExpr->getBeginLoc() < Expr->getBeginLoc()) LastExpr = Expr;
+ if (LastExpr->getBeginLoc() < Expr->getBeginLoc())
+ LastExpr = Expr;
}
return LastExpr;
@@ -56,7 +59,7 @@ AST_MATCHER(CXXRecordDecl, hasTrivialMoveConstructor) {
return Node.hasDefinition() && Node.hasTrivialMoveConstructor();
}
-void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
+void LostStdMoveCheck::registerMatchers(MatchFinder *Finder) {
auto ReturnParent =
hasParent(expr(hasParent(cxxConstructExpr(hasParent(returnStmt())))));
@@ -84,7 +87,7 @@ void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
unless(hasAncestor(whileStmt())),
- // Not in a body of lambda
+ // Not in a body of lambda
unless(hasAncestor(compoundStmt(hasAncestor(lambdaExpr())))),
// only non-X&
@@ -112,21 +115,22 @@ void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
this);
}
-void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
- const auto* MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("decl");
- const auto* MatchedFunc = Result.Nodes.getNodeAs<FunctionDecl>("func");
- const auto* MatchedUse = Result.Nodes.getNodeAs<Expr>("use");
- const auto* MatchedUseCall = Result.Nodes.getNodeAs<CallExpr>("use_parent");
- const auto* MatchedLeafStatement =
+void LostStdMoveCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("decl");
+ const auto *MatchedFunc = Result.Nodes.getNodeAs<FunctionDecl>("func");
+ const auto *MatchedUse = Result.Nodes.getNodeAs<Expr>("use");
+ const auto *MatchedUseCall = Result.Nodes.getNodeAs<CallExpr>("use_parent");
+ const auto *MatchedLeafStatement =
Result.Nodes.getNodeAs<Stmt>("leaf_statement");
- if (!MatchedDecl->hasLocalStorage()) return;
+ if (!MatchedDecl->hasLocalStorage())
+ return;
if (MatchedUseCall) {
return;
}
- const Expr* LastUsage =
+ const Expr *LastUsage =
getLastVarUsage(*MatchedDecl, *MatchedFunc, *Result.Context);
if (LastUsage && LastUsage->getBeginLoc() > MatchedUse->getBeginLoc()) {
@@ -140,7 +144,7 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
}
// Calculate X usage count in the statement
- llvm::SmallPtrSet<const DeclRefExpr*, 16> DeclRefs;
+ llvm::SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
ArrayRef<BoundNodes> Matches = match(
findAll(declRefExpr(
@@ -158,7 +162,7 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
return;
}
- const SourceManager& Source = Result.Context->getSourceManager();
+ const SourceManager &Source = Result.Context->getSourceManager();
const auto Range =
CharSourceRange::getTokenRange(MatchedUse->getSourceRange());
const StringRef NeedleExprCode =
@@ -167,9 +171,11 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
if (NeedleExprCode == "=") {
diag(MatchedUse->getBeginLoc(), "could be std::move()")
- << FixItHint::CreateInsertion(
- MatchedUse->getBeginLoc(),
- (MatchedDecl->getName() + " = std::move(" + MatchedDecl->getName() + "),").str());
+ << FixItHint::CreateInsertion(MatchedUse->getBeginLoc(),
+ (MatchedDecl->getName() +
+ " = std::move(" +
+ MatchedDecl->getName() + "),")
+ .str());
} else {
diag(MatchedUse->getBeginLoc(), "could be std::move()")
<< FixItHint::CreateReplacement(
@@ -177,4 +183,4 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
}
}
-} // namespace clang::tidy::performance
+} // namespace clang::tidy::performance
>From b48425b0ae57eb145e85d4ae834abf3d2450d112 Mon Sep 17 00:00:00 2001
From: Vasily Kulikov <segoon at yandex-team.ru>
Date: Sun, 1 Jun 2025 11:05:02 +0300
Subject: [PATCH 16/16] format
---
clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h
index 789f1233e7042..324a0f50b6d3a 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h
@@ -14,7 +14,8 @@
namespace clang::tidy::performance {
/// Warns if copy constructor is used instead of std::move() and suggests a fix.
-/// It honours cycles, lambdas, and unspecified call order in compound expressions.
+/// It honours cycles, lambdas, and unspecified call order in compound
+/// expressions.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/performance/lost-std-move.html
More information about the cfe-commits
mailing list