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 + MoveConstructorBaseCheck.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 "MoveConstructorBaseCheck.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-base"); CheckFactories.registerCheck( "misc-noexcept-move-constructor"); CheckFactories.registerCheck( Index: clang-tidy/misc/MoveConstructorBaseCheck.h =================================================================== --- clang-tidy/misc/MoveConstructorBaseCheck.h +++ clang-tidy/misc/MoveConstructorBaseCheck.h @@ -0,0 +1,32 @@ +//===--- MoveConstructorBaseCheck.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_MOVECONSTRUCTORBASECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTRUCTORBASECHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// \brief The check flags user-defined move constructors that have a +/// ctor-initializer initializing a base class through a copy constructor +/// instead of a move constructor. +class MoveConstructorBaseCheck : public ClangTidyCheck { +public: + MoveConstructorBaseCheck(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_MOVECONSTRUCTORBASECHECK_H Index: clang-tidy/misc/MoveConstructorBaseCheck.cpp =================================================================== --- clang-tidy/misc/MoveConstructorBaseCheck.cpp +++ clang-tidy/misc/MoveConstructorBaseCheck.cpp @@ -0,0 +1,69 @@ +//===--- MoveConstructorBaseCheck.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 "MoveConstructorBaseCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { + +void MoveConstructorBaseCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + constructorDecl(allOf( + isMoveConstructor(), + hasAnyConstructorInitializer(allOf( + isBaseInitializer(), + ctorInitializer(withInitializer(constructExpr(hasDeclaration( + constructorDecl(isCopyConstructor()).bind("base_ctor") + )))).bind("init") + )) + )), this); +} + +void MoveConstructorBaseCheck::check(const MatchFinder::MatchResult &Result) { + const auto *BaseCopyCtor = + Result.Nodes.getNodeAs("base_ctor"); + const auto *Initializer = Result.Nodes.getNodeAs("init"); + + // Diagnose when the base class has a move constructor available, but the + // derived class uses the copy constructor as its ctor-initializer instead. + const CXXConstructorDecl *Candidate = nullptr; + for (const auto *BCtor : BaseCopyCtor->getParent()->ctors()) { + if (BCtor->isMoveConstructor() && + BCtor->getAccess() <= AS_protected && + !BCtor->isDeleted()) { + // The base class has a move constructor that is at least accessible to + // the derived class. + // + // FIXME: Determine whether the move constructor is a viable candidate + // for the derived constructor's parameter, perhaps provide a fixit that + // suggests std::move(). + Candidate = BCtor; + break; + } + } + + if (Candidate) { + // There's a base class move constructor candidate that the caller probably + // intended to call instead. + diag(Initializer->getSourceLocation(), + "move constructor initializes base class by calling a copy constructor"); + diag(BaseCopyCtor->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-base.cpp =================================================================== --- test/clang-tidy/misc-move-constructor-base.cpp +++ test/clang-tidy/misc-move-constructor-base.cpp @@ -0,0 +1,70 @@ +// RUN: clang-tidy %s -checks=-*,misc-move-constructor-base -- -std=c++14 | FileCheck %s + +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+1]]:16: warning: move constructor initializes base class by calling a copy constructor [misc-move-constructor-base] + D(D &&RHS) : B(RHS) {} +}; + +struct E : B { + E() : B() {} + E(const E &RHS) : B(RHS) {} + // CHECK-NOT: warning: + E(E &&RHS) : B(move(RHS)) {} // ok +}; + +struct F { + C M; + + // CHECK-NOT: warning: + F(F &&) : M(C()) {} +}; + +struct G { + G() = default; + G(const G&) = default; + G(G&&) = delete; +}; + +struct H : G { + H() = default; + H(const H&) = default; + // CHECK-NOT: warning: + H(H &&RHS) : G(RHS) {} // ok +}; + +struct I { + I(const I &) = default; // suppresses move constructor creation +}; + +struct J : I { + // CHECK-NOT: warning: + J(J &&RHS) : I(RHS) {} // ok +}; + +struct K {}; // Has implicit copy and move constructors +struct L : K { + // CHECK: :[[@LINE+1]]:16: warning: move constructor initializes base class by calling a copy constructor [misc-move-constructor-base] + L(L &&RHS) : K(RHS) {} +};