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

clang-tools-extra/clang-tidy/performance/CMakeLists.txt

@@ -12,6 +12,7 @@ add_clang_library(clangTidyPerformanceModule STATIC
   InefficientAlgorithmCheck.cpp
   InefficientStringConcatenationCheck.cpp
   InefficientVectorOperationCheck.cpp
+  LostStdMoveCheck.cpp
   MoveConstArgCheck.cpp
   MoveConstructorInitCheck.cpp
   NoAutomaticMoveCheck.cpp

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

@@ -0,0 +1,180 @@
+//===--- 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 "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+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)
+    Nodes.insert(Match.getNodeAs<Node>(ID));
+}
+
+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))),
+
+                      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 = allDeclRefExprsHonourLambda(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) {
+  auto ReturnParent =
+      hasParent(expr(hasParent(cxxConstructExpr(hasParent(returnStmt())))));
+
+  auto OutermostExpr = expr(unless(hasParent(expr())));
+  auto LeafStatement = stmt(OutermostExpr);
+
+  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())),
+
+	  // Not in a body of lambda
+          unless(hasAncestor(compoundStmt(hasAncestor(lambdaExpr())))),
+
+          // only non-X&
+          unless(hasDeclaration(
+              varDecl(hasType(qualType(lValueReferenceType()))))),
+
+          hasAncestor(LeafStatement.bind("leaf_statement")),
+
+          hasDeclaration(
+              varDecl(hasAncestor(functionDecl().bind("func"))).bind("decl")),
+
+          anyOf(
+
+              // f(x)
+              hasParent(expr(hasParent(cxxConstructExpr())).bind("use_parent")),
+
+              // f((x))
+              hasParent(parenExpr(hasParent(
+                  expr(hasParent(cxxConstructExpr())).bind("use_parent"))))
+
+                  )
+
+              )
+          .bind("use"),
+      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 =
+      Result.Nodes.getNodeAs<Stmt>("leaf_statement");
+
+  if (!MatchedDecl->hasLocalStorage()) return;
+
+  if (MatchedUseCall) {
+    return;
+  }
+
+  const Expr* LastUsage =
+      getLastVarUsage(*MatchedDecl, *MatchedFunc, *Result.Context);
+
+  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))),
+
+                  unless(hasAncestor(lambdaExpr(hasAnyCapture(lambdaCapture(
+                      capturesVar(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;
+  }
+
+  const SourceManager& Source = Result.Context->getSourceManager();
+  const auto Range =
+      CharSourceRange::getTokenRange(MatchedUse->getSourceRange());
+  const StringRef NeedleExprCode =
+      Lexer::getSourceText(Range, Source, Result.Context->getLangOpts());
+
+  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

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

@@ -0,0 +1,33 @@
+//===--- 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.CPlusPlus11;
+  }
+};
+
+} // namespace clang::tidy::performance
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_LOSTSTDMOVECHECK_H

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>(

clang-tools-extra/docs/ReleaseNotes.rst

@@ -145,8 +145,75 @@ 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
 ^^^^^^^^^^^^^^^^^

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -340,6 +340,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>`,

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]
+   }