Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -10,6 +10,7 @@ MacroParenthesesCheck.cpp MacroRepeatedSideEffectsCheck.cpp MiscTidyModule.cpp + MoveConstructorInitCheck.cpp NoexceptMoveConstructorCheck.cpp StaticAssertCheck.cpp SwappedArgumentsCheck.cpp Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ 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 @@ "misc-macro-parentheses"); CheckFactories.registerCheck( "misc-macro-repeated-side-effects"); + CheckFactories.registerCheck( + "misc-move-constructor-init"); CheckFactories.registerCheck( "misc-noexcept-move-constructor"); CheckFactories.registerCheck( Index: clang-tidy/misc/MoveConstructorInitCheck.h =================================================================== --- clang-tidy/misc/MoveConstructorInitCheck.h +++ 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 Index: clang-tidy/misc/MoveConstructorInitCheck.cpp =================================================================== --- clang-tidy/misc/MoveConstructorInitCheck.cpp +++ 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("ctor"); + const auto *Initializer = Result.Nodes.getNodeAs("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 + Index: test/clang-tidy/misc-move-constructor-init.cpp =================================================================== --- test/clang-tidy/misc-move-constructor-init.cpp +++ 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 struct remove_reference {typedef T type;}; +template struct remove_reference {typedef T type;}; +template struct remove_reference {typedef T type;}; + +template +typename remove_reference::type&& move(T&& arg) { + return static_cast::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)) {} +};