[clang-tidy] Add performance-explicit-move-constructor check

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

@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
 add_clang_library(clangTidyPerformanceModule STATIC
   AvoidEndlCheck.cpp
   EnumSizeCheck.cpp
+  ExplicitMoveConstructorCheck.cpp
   FasterStringFindCheck.cpp
   ForRangeCopyCheck.cpp
   ImplicitConversionInLoopCheck.cpp

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

@@ -0,0 +1,76 @@
+//===--- ExplicitMoveConstructorCheck.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 "ExplicitMoveConstructorCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::performance {
+
+static SourceRange findExplicitToken(const CXXConstructorDecl *Ctor,
+                                     const SourceManager &Source,
+                                     const LangOptions &LangOpts) {
+  SourceLocation CurrentLoc = Ctor->getBeginLoc();
+  const SourceLocation EndLoc = Ctor->getEndLoc();
+  Token Tok;
+
+  do {
+    const bool failed = Lexer::getRawToken(CurrentLoc, Tok, Source, LangOpts);
+
+    if (failed)
+      return {};
+
+    if (Tok.is(tok::raw_identifier) && Tok.getRawIdentifier() == "explicit")
+      return {Tok.getLocation(), Tok.getEndLoc()};
+
+    CurrentLoc = Tok.getEndLoc();
+  } while (Tok.isNot(tok::eof) && CurrentLoc < EndLoc);
+
+  return {};
+}
+
+void ExplicitMoveConstructorCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      traverse(
+          TK_IgnoreUnlessSpelledInSource,
+          cxxRecordDecl(
+              has(cxxConstructorDecl(isMoveConstructor(), isExplicit(),
+                                     unless(isDeleted()))
+                      .bind("move-ctor")),
+              has(cxxConstructorDecl(isCopyConstructor(), unless(isDeleted()))
+                      .bind("copy-ctor")),
+              unless(isExpansionInSystemHeader()))),
+      this);
+}
+
+void ExplicitMoveConstructorCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MoveCtor =
+      Result.Nodes.getNodeAs<CXXConstructorDecl>("move-ctor");
+  const auto *CopyCtor =
+      Result.Nodes.getNodeAs<CXXConstructorDecl>("copy-ctor");
+
+  if (!MoveCtor || !CopyCtor)
+    return;
+
+  auto Diag =
+      diag(MoveCtor->getLocation(),
+           "copy constructor may be called instead of move constructor");
+  const SourceRange ExplicitTokenRange =
+      findExplicitToken(MoveCtor, *Result.SourceManager, getLangOpts());
+
+  if (ExplicitTokenRange.isInvalid())
+    return;
+
+  Diag << FixItHint::CreateRemoval(
+      CharSourceRange::getCharRange(ExplicitTokenRange));
+}
+
+} // namespace clang::tidy::performance

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

@@ -0,0 +1,34 @@
+//===--- ExplicitMoveConstructorCheck.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_EXPLICITMOVECONSTRUCTORCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_EXPLICITMOVECONSTRUCTORCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::performance {
+
+/// Find classes that define an explicit move constructor and a (non-deleted)
+/// copy constructor.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance/explicit-move-constructor.html
+class ExplicitMoveConstructorCheck : public ClangTidyCheck {
+public:
+  ExplicitMoveConstructorCheck(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_EXPLICITMOVECONSTRUCTORCHECK_H

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

@@ -11,6 +11,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidEndlCheck.h"
 #include "EnumSizeCheck.h"
+#include "ExplicitMoveConstructorCheck.h"
 #include "FasterStringFindCheck.h"
 #include "ForRangeCopyCheck.h"
 #include "ImplicitConversionInLoopCheck.h"
@@ -37,6 +38,8 @@ class PerformanceModule : public ClangTidyModule {
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<AvoidEndlCheck>("performance-avoid-endl");
     CheckFactories.registerCheck<EnumSizeCheck>("performance-enum-size");
+    CheckFactories.registerCheck<ExplicitMoveConstructorCheck>(
+        "performance-explicit-move-constructor");
     CheckFactories.registerCheck<FasterStringFindCheck>(
         "performance-faster-string-find");
     CheckFactories.registerCheck<ForRangeCopyCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

@@ -163,6 +163,12 @@ New checks
   Replace comparisons between signed and unsigned integers with their safe
   C++20 ``std::cmp_*`` alternative, if available.
 
+- New :doc:`performance-explicit-move-constructor
+  <clang-tidy/checks/performance/explicit-move-constructor>` check.
+
+  Warns when a class defines an explicit move constructor, which may cause
+  the copy constructor to get called instead.
+
 - New :doc:`portability-template-virtual-member-function
   <clang-tidy/checks/portability/template-virtual-member-function>` check.
 

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

@@ -330,6 +330,7 @@ Clang-Tidy Checks
    :doc:`openmp-use-default-none <openmp/use-default-none>`,
    :doc:`performance-avoid-endl <performance/avoid-endl>`, "Yes"
    :doc:`performance-enum-size <performance/enum-size>`,
+   :doc:`performance-explicit-move-constructor <performance/explicit-move-constructor>`, "Yes"
    :doc:`performance-faster-string-find <performance/faster-string-find>`, "Yes"
    :doc:`performance-for-range-copy <performance/for-range-copy>`, "Yes"
    :doc:`performance-implicit-conversion-in-loop <performance/implicit-conversion-in-loop>`,

clang-tools-extra/docs/clang-tidy/checks/performance/explicit-move-constructor.rst

@@ -0,0 +1,32 @@
+.. title:: clang-tidy - performance-explicit-move-constructor
+
+performance-explicit-move-constructor
+=====================================
+
+Warns when a class defines an explicit move constructor, which may cause
+the copy constructor to get called instead.
+
+Example:
+
+.. code-block:: c++
+
+    class Expensive {
+    public:
+        // ...
+        Expensive(const Expensive&) { /* ... */ }
+        explicit Expensive(Expensive&&) { /* ... */ }
+    };
+
+    void process(Expensive);
+
+    int main() {
+        Expensive exp{};
+        process(std::move(exp));
+
+        return 0;
+    }
+
+Here, the call to ``process`` is actually going to copy ``exp`` instead of
+moving it, potentially incurring a performance penalty if copying is expensive.
+No warning will be emitted if the copy constructor is deleted, as any call to
+it would make the program fail to compile.

clang-tools-extra/test/clang-tidy/checkers/performance/explicit-move-constructor.cpp

@@ -0,0 +1,65 @@
+// RUN: %check_clang_tidy %s performance-explicit-move-constructor %t
+
+struct Empty {};
+
+class NotReported1 {};
+
+class NotReported2 {
+public:
+  NotReported2(NotReported2&&) = default;
+  NotReported2(const NotReported2&) = default;
+};
+
+class NotReported3 {
+public:
+  explicit NotReported3(NotReported3&&) = default;
+};
+
+class NotReported4 {
+public:
+  explicit NotReported4(NotReported4&&) = default;
+  NotReported4(const NotReported4&) = delete;
+};
+
+class NotReported5 {
+public:
+  explicit NotReported5(NotReported5&&) = delete;
+  NotReported5(const NotReported5&) = default;
+};
+
+class Reported1 {
+public:
+  explicit Reported1(Reported1&&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: copy constructor may be called instead of move constructor [performance-explicit-move-constructor]
+  // CHECK-FIXES: {{^  }}Reported1(Reported1&&) = default;{{$}}
+  Reported1(const Reported1&) = default;
+};
+
+template <typename>
+class Reported2 {
+public:
+  explicit Reported2(Reported2&&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: copy constructor may be called instead of move constructor [performance-explicit-move-constructor]
+  // CHECK-FIXES: {{^  }}Reported2(Reported2&&) = default;{{$}}
+  Reported2(const Reported2&) = default;
+};
+
+template <typename T>
+class Reported3 : public T {
+public:
+  explicit Reported3(Reported3&&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: copy constructor may be called instead of move constructor [performance-explicit-move-constructor]
+  // CHECK-FIXES: {{^  }}Reported3(Reported3&&) = default;{{$}}
+  Reported3(const Reported3&) = default;
+};
+
+template <typename T>
+class Reported4 {
+public:
+  explicit Reported4(Reported4&&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: copy constructor may be called instead of move constructor [performance-explicit-move-constructor]
+  // CHECK-FIXES: {{^  }}Reported4(Reported4&&) = default;{{$}}
+  Reported4(const Reported4&) = default;
+
+  T member;
+};