[clang-tidy] Add check performance-lost-std-move #139525

segoon · 2025-05-12T10:08:36Z

No description provided.

llvmbot · 2025-05-12T10:09:10Z

@llvm/pr-subscribers-clang-tools-extra

Author: Vasiliy Kulikov (segoon)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/139525.diff

8 Files Affected:

(modified) clang-tools-extra/clang-tidy/performance/CMakeLists.txt (+1)
(added) clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp (+96)
(added) clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h (+37)
(modified) clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp (+2)
(modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
(modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
(added) clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst (+14)
(added) clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp (+108)

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);
+}

llvmbot · 2025-05-12T10:09:10Z

@llvm/pr-subscribers-clang-tidy

Author: Vasiliy Kulikov (segoon)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/139525.diff

8 Files Affected:

(modified) clang-tools-extra/clang-tidy/performance/CMakeLists.txt (+1)
(added) clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp (+96)
(added) clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h (+37)
(modified) clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp (+2)
(modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
(modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
(added) clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst (+14)
(added) clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp (+108)

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);
+}

firewave · 2025-05-12T10:34:46Z

See also #53489 and #57908.

CC @Febbe

github-actions · 2025-05-12T12:23:12Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions h,cpp -- clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h clang-tools-extra/test/clang-tidy/checkers/performance/lost-std-move.cpp clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp

View the diff from clang-format here.

diff --git a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
index fca0f751b..14da9db88 100644
--- a/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp
@@ -63,7 +63,8 @@ void LostStdMoveCheck::registerMatchers(MatchFinder *Finder) {
 const Expr *LostStdMoveCheck::getLastVarUsage(const VarDecl &Var,
                                               const Decl &Func,
                                               ASTContext &Context) {
-  llvm::SmallPtrSet<const DeclRefExpr *, 16> Exprs = allDeclRefExprs(Var, Func, Context);
+  llvm::SmallPtrSet<const DeclRefExpr *, 16> Exprs =
+      allDeclRefExprs(Var, Func, Context);
 
   const Expr *LastExpr = nullptr;
   for (const auto &Expr : Exprs) {

EugeneZelenko · 2025-05-12T14:18:42Z

clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp

+const Expr *LostStdMoveCheck::getLastVarUsage(const VarDecl &Var,
+                                              const Decl &Func,
+                                              ASTContext &Context) {
+  auto Exprs = allDeclRefExprs(Var, Func, Context);


Please do not use auto unless type is explicitly states in same statement or iterator.

EugeneZelenko · 2025-05-12T14:19:04Z

clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp

+  if (MatchedUseCall)
+    return;
+
+  const auto *LastUsage =


EugeneZelenko · 2025-05-12T14:19:38Z

clang-tools-extra/docs/ReleaseNotes.rst

+- New :doc:`performance-lost-std-move
+  <clang-tidy/checks/performance/lost-std-move>` check.
+
+  Searches for lost std::move().


Suggested change

Searches for lost std::move().

Searches for lost ``std::move()``.

EugeneZelenko · 2025-05-12T14:20:01Z

clang-tools-extra/docs/ReleaseNotes.rst

 - New :doc:`readability-ambiguous-smartptr-reset-call
  <clang-tidy/checks/readability/ambiguous-smartptr-reset-call>` check.

+


Unintended change?

EugeneZelenko · 2025-05-12T14:20:47Z

clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst

+performance-lost-std-move
+=========================
+
+The check warns if copy constructor is used instead of std::move().


Please synchronize with statement in Release Notes. This check should be omitted.

vbvictor

Commented general suggestions/improvements for the check without deep dive into matchers/tests.

Please look at tests in previous attempt in creating such check https://reviews.llvm.org/D137205.
Check that all test cases from that PR are covered in your tests.
If some cases are not covered and you can't address them, please add them to Limitations in docs and leave a fixme.

vbvictor · 2025-05-21T15:05:05Z

clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h

+  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;


Suggested change

return LangOpts.CPlusPlus;

return LangOpts.CPlusPlus11;

vbvictor · 2025-05-21T15:06:27Z

clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h

+  }
+
+private:
+  const Expr *getLastVarUsage(const VarDecl &Var, const Decl &Func,


this should be converted to static function and move to .cpp file if no fields of check class are needed

vbvictor · 2025-05-21T15:07:45Z

clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp

+  llvm::SmallPtrSet<const DeclRefExpr *, 16> Exprs = allDeclRefExprs(Var, Func, Context);
+
+  const Expr *LastExpr = nullptr;
+  for (const auto &Expr : Exprs) {


Use explicit type instead of auto if it isn't written explicitly in code line.

vbvictor · 2025-05-21T15:07:51Z

clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp

+template <typename Node>
+void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
+                        llvm::SmallPtrSet<const Node *, 16> &Nodes) {
+  for (const auto &Match : Matches)


vbvictor · 2025-05-21T15:10:00Z

clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp

+
+  // Calculate X usage count in the statement
+  llvm::SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
+  auto Matches = match(


vbvictor · 2025-05-21T15:13:10Z

clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp

+    return;
+  }
+
+  diag(LastUsage->getBeginLoc(), "Could be std::move()");


Try to provide fixits with the insertion of std::move

vbvictor · 2025-05-21T15:13:38Z

clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h

+
+namespace clang::tidy::performance {
+
+/// Search for lost std::move().


This should be synced with release notes and check docs

vbvictor · 2025-05-21T15:19:15Z

clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst

+performance-lost-std-move
+=========================
+
+Warns if copy constructor is used instead of ``std::move()``.


This need a little more description when the check will warn, e.g when this will fire, with what types?
Look for reference at https://reviews.llvm.org/D137205

vbvictor · 2025-05-21T15:20:55Z

clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst

+performance-lost-std-move
+=========================
+
+Warns if copy constructor is used instead of ``std::move()``.


Nit suggestion: try make the description similar to already existing new checks in ReleaseNotes.rst.
Description should start with "Finds ..."

vbvictor · 2025-05-21T15:21:47Z

clang-tools-extra/docs/clang-tidy/checks/performance/lost-std-move.rst

+
+   void g(X x) {
+     f(x);  // warning: Could be std::move() [performance-lost-std-move]
+   }


Give examples when check will not warn

[clang-tidy] Add check performance-lost-std-move

3abbce9

llvmbot added clang-tools-extra clang-tidy labels May 12, 2025

Merge branch 'main' into feature/lost-std-move

35ac17a

f(x, x)

b48a1f7

format

32ebfed

EugeneZelenko requested review from PiotrZSL, HerrCai0907 and carlosgalvezp May 12, 2025 14:17

EugeneZelenko reviewed May 12, 2025

View reviewed changes

segoon added 3 commits May 12, 2025 18:05

fixes

f526417

cleanup

13c1433

sync

d3b8c17

vbvictor reviewed May 21, 2025

View reviewed changes

	Searches for lost std::move().
	Searches for lost ``std::move()``.

		- New :doc:`readability-ambiguous-smartptr-reset-call
		<clang-tidy/checks/readability/ambiguous-smartptr-reset-call>` check.


		namespace clang::tidy::performance {

		/// Search for lost std::move().

[clang-tidy] Add check performance-lost-std-move #139525

Are you sure you want to change the base?

[clang-tidy] Add check performance-lost-std-move #139525

Uh oh!

Conversation

segoon commented May 12, 2025

Uh oh!

llvmbot commented May 12, 2025

Uh oh!

llvmbot commented May 12, 2025

Uh oh!

firewave commented May 12, 2025

Uh oh!

github-actions bot commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbvictor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbvictor May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbvictor May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented May 12, 2025 •

edited

Loading

vbvictor May 21, 2025 •

edited

Loading

vbvictor May 21, 2025 •

edited

Loading