Add a new clang-tidy check (misc-move-constructor-init) that diagnoses move constructor initializations that call copy constructors instead of move constructors.

llvm-svn: 245571

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

@@ -10,6 +10,7 @@ add_clang_library(clangTidyMiscModule
   MacroParenthesesCheck.cpp
   MacroRepeatedSideEffectsCheck.cpp
   MiscTidyModule.cpp
+  MoveConstructorInitCheck.cpp
   NoexceptMoveConstructorCheck.cpp
   StaticAssertCheck.cpp
   SwappedArgumentsCheck.cpp

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

@@ -18,6 +18,7 @@
 #include "InefficientAlgorithmCheck.h"
 #include "MacroParenthesesCheck.h"
 #include "MacroRepeatedSideEffectsCheck.h"
+#include "MoveConstructorInitCheck.h"
 #include "NoexceptMoveConstructorCheck.h"
 #include "StaticAssertCheck.h"
 #include "SwappedArgumentsCheck.h"
@@ -50,6 +51,8 @@ public:
         "misc-macro-parentheses");
     CheckFactories.registerCheck<MacroRepeatedSideEffectsCheck>(
         "misc-macro-repeated-side-effects");
+    CheckFactories.registerCheck<MoveConstructorInitCheck>(
+        "misc-move-constructor-init");
     CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
         "misc-noexcept-move-constructor");
     CheckFactories.registerCheck<StaticAssertCheck>(

clang-tools-extra/clang-tidy/misc/MoveConstructorInitCheck.cpp

@@ -0,0 +1,77 @@
+//===--- MoveConstructorInitCheck.cpp - clang-tidy-------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "MoveConstructorInitCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+void MoveConstructorInitCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+    constructorDecl(unless(isImplicit()), allOf(
+      isMoveConstructor(),
+      hasAnyConstructorInitializer(
+        ctorInitializer(withInitializer(constructExpr(hasDeclaration(
+          constructorDecl(isCopyConstructor()).bind("ctor")
+        )))).bind("init")
+      )
+    )), this);
+}
+
+void MoveConstructorInitCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *CopyCtor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
+  const auto *Initializer = Result.Nodes.getNodeAs<CXXCtorInitializer>("init");
+
+  // Do not diagnose if the expression used to perform the initialization is a
+  // trivially-copyable type.
+  QualType QT = Initializer->getInit()->getType();
+  if (QT.isTriviallyCopyableType(*Result.Context))
+    return;
+  
+  const auto *RD = QT->getAsCXXRecordDecl();
+  if (RD && RD->isTriviallyCopyable())
+    return;
+
+  // Diagnose when the class type has a move constructor available, but the
+  // ctor-initializer uses the copy constructor instead.
+  const CXXConstructorDecl *Candidate = nullptr;
+  for (const auto *Ctor : CopyCtor->getParent()->ctors()) {
+    if (Ctor->isMoveConstructor() && Ctor->getAccess() <= AS_protected &&
+        !Ctor->isDeleted()) {
+      // The type has a move constructor that is at least accessible to the
+      // initializer.
+      //
+      // FIXME: Determine whether the move constructor is a viable candidate
+      // for the ctor-initializer, perhaps provide a fixit that suggests
+      // using std::move().
+      Candidate = Ctor;
+      break;
+    }
+  }
+
+  if (Candidate) {
+    // There's a move constructor candidate that the caller probably intended
+    // to call instead.
+    diag(Initializer->getSourceLocation(),
+         "move constructor initializes %0 by calling a copy constructor")
+        << (Initializer->isBaseInitializer() ? "base class" : "class member");
+    diag(CopyCtor->getLocation(), "copy constructor being called",
+         DiagnosticIDs::Note);
+    diag(Candidate->getLocation(), "candidate move constructor here",
+         DiagnosticIDs::Note);
+  }
+}
+
+} // namespace tidy
+} // namespace clang
+

clang-tools-extra/clang-tidy/misc/MoveConstructorInitCheck.h

@@ -0,0 +1,32 @@
+//===--- MoveConstructorInitCheck.h - clang-tidy-----------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTRUCTORINITCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTRUCTORINITCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief The check flags user-defined move constructors that have a
+/// ctor-initializer initializing a member or base class through a copy
+/// constructor instead of a move constructor.
+class MoveConstructorInitCheck : public ClangTidyCheck {
+public:
+  MoveConstructorInitCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTRUCTORINITCHECK_H

clang-tools-extra/test/clang-tidy/misc-move-constructor-init.cpp

@@ -0,0 +1,78 @@
+// RUN: clang-tidy %s -checks=-*,misc-move-constructor-init -- -std=c++14 | FileCheck %s -implicit-check-not="{{warning|error}}:"
+
+template <class T> struct remove_reference      {typedef T type;};
+template <class T> struct remove_reference<T&>  {typedef T type;};
+template <class T> struct remove_reference<T&&> {typedef T type;};
+
+template <typename T>
+typename remove_reference<T>::type&& move(T&& arg) {
+  return static_cast<typename remove_reference<T>::type&&>(arg);
+}
+
+struct C {
+  C() = default;
+  C(const C&) = default;
+};
+
+struct B {
+  B() {}
+  B(const B&) {}
+  B(B &&) {}
+};
+
+struct D : B {
+  D() : B() {}
+  D(const D &RHS) : B(RHS) {}
+  // CHECK: :[[@LINE+3]]:16: warning: move constructor initializes base class by calling a copy constructor [misc-move-constructor-init]
+  // CHECK: 19:3: note: copy constructor being called
+  // CHECK: 20:3: note: candidate move constructor here
+  D(D &&RHS) : B(RHS) {}
+};
+
+struct E : B {
+  E() : B() {}
+  E(const E &RHS) : B(RHS) {}
+  E(E &&RHS) : B(move(RHS)) {} // ok
+};
+
+struct F {
+  C M;
+
+  F(F &&) : M(C()) {} // ok
+};
+
+struct G {
+  G() = default;
+  G(const G&) = default;
+  G(G&&) = delete;
+};
+
+struct H : G {
+  H() = default;
+  H(const H&) = default;
+  H(H &&RHS) : G(RHS) {} // ok
+};
+
+struct I {
+  I(const I &) = default; // suppresses move constructor creation
+};
+
+struct J : I {
+  J(J &&RHS) : I(RHS) {} // ok
+};
+
+struct K {}; // Has implicit copy and move constructors, is trivially copyable
+struct L : K {
+  L(L &&RHS) : K(RHS) {} // ok
+};
+
+struct M {
+  B Mem;
+  // CHECK: :[[@LINE+1]]:16: warning: move constructor initializes class member by calling a copy constructor [misc-move-constructor-init]
+  M(M &&RHS) : Mem(RHS.Mem) {}
+};
+
+struct N {
+  B Mem;
+  N(N &&RHS) : Mem(move(RHS.Mem)) {}
+};