Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -40,6 +40,7 @@ SwappedArgumentsCheck.cpp ThrowByValueCatchByReferenceCheck.cpp UndelegatedConstructor.cpp + UndelegatedCopyOfBaseClassesCheck.cpp UniqueptrResetReleaseCheck.cpp UnusedAliasDeclsCheck.cpp UnusedParametersCheck.cpp Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -48,6 +48,7 @@ #include "ThrowByValueCatchByReferenceCheck.h" #include "UnconventionalAssignOperatorCheck.h" #include "UndelegatedConstructor.h" +#include "UndelegatedCopyOfBaseClassesCheck.h" #include "UniqueptrResetReleaseCheck.h" #include "UnusedAliasDeclsCheck.h" #include "UnusedParametersCheck.h" @@ -131,6 +132,8 @@ "misc-throw-by-value-catch-by-reference"); CheckFactories.registerCheck( "misc-undelegated-constructor"); + CheckFactories.registerCheck( + "misc-undelegated-copy-of-base-classes"); CheckFactories.registerCheck( "misc-uniqueptr-reset-release"); CheckFactories.registerCheck( Index: clang-tidy/misc/UndelegatedCopyOfBaseClassesCheck.h =================================================================== --- /dev/null +++ clang-tidy/misc/UndelegatedCopyOfBaseClassesCheck.h @@ -0,0 +1,36 @@ +//===--- UndelegatedCopyOfBaseClassesCheck.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_UNDELEGATED_COPY_OF_BASE_CLASSES_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNDELEGATED_COPY_OF_BASE_CLASSES_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// Finds copy constructors where the ctor don't call the constructor of the +/// base class. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc-undelegated-copy-of-base-classes.html +class UndelegatedCopyOfBaseClassesCheck : public ClangTidyCheck { +public: + UndelegatedCopyOfBaseClassesCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNDELEGATED_COPY_OF_BASE_CLASSES_H Index: clang-tidy/misc/UndelegatedCopyOfBaseClassesCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/UndelegatedCopyOfBaseClassesCheck.cpp @@ -0,0 +1,115 @@ +//===--- UndelegatedCopyOfBaseClassesCheck.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 "UndelegatedCopyOfBaseClassesCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { + +void UndelegatedCopyOfBaseClassesCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + cxxConstructorDecl( + isCopyConstructor(), + hasAnyConstructorInitializer(cxxCtorInitializer( + isBaseInitializer(), withInitializer(cxxConstructExpr(unless( + hasDescendant(implicitCastExpr()))))))) + .bind("ctor"), + this); +} + +void UndelegatedCopyOfBaseClassesCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *Ctor = Result.Nodes.getStmtAs("ctor"); + + // We match here because we want one warning (and FixIt) for every ctor. + const auto Matches = match( + cxxConstructorDecl( + isCopyConstructor(), + forEachConstructorInitializer( + cxxCtorInitializer( + isBaseInitializer(), + withInitializer(cxxConstructExpr( + unless(hasDescendant(implicitCastExpr()))) + .bind("cruct-expr"))).bind("init"))), + *Ctor, *Result.Context); + + std::string FixItInitList = ""; + for (const auto &Match : Matches) { + const auto *Init = Match.getNodeAs("init"); + + // Valid when the initializer is written manually (not generated). + if ((Init->getSourceRange()).isValid()) { + const auto *CExpr = Match.getNodeAs("cruct-expr"); + diag(CExpr->getLocEnd(), "Missing parameter in the base initializer!") + << FixItHint::CreateInsertion( + CExpr->getLocEnd(), Ctor->getParamDecl(0)->getNameAsString()); + } else { + FixItInitList += + Init->getBaseClass()->getAsCXXRecordDecl()->getNameAsString(); + + // We want to write in the FixIt the template arguments too. + if (auto *Decl = dyn_cast( + Init->getBaseClass()->getAsCXXRecordDecl())) { + FixItInitList += "<"; + + const auto Args = Decl->getTemplateArgs().asArray(); + for (const auto &Arg : Args) + FixItInitList += Arg.getAsType().getAsString() + ", "; + + FixItInitList = FixItInitList.substr(0, FixItInitList.size() - 2); + FixItInitList += ">"; + } + + FixItInitList += "(" + Ctor->getParamDecl(0)->getNameAsString() + "), "; + } + } + // Early return if there were just missing parameters. + if (FixItInitList.empty()) + return; + FixItInitList = FixItInitList.substr(0, FixItInitList.size() - 2); + + auto &SM = Result.Context->getSourceManager(); + SourceLocation StartLoc = Ctor->getLocation(); + StringRef Buffer = SM.getBufferData(SM.getFileID(StartLoc)); + const char *StartChar = SM.getCharacterData(StartLoc); + + Lexer Lex(StartLoc, (Result.Context)->getLangOpts(), StartChar, StartChar, + Buffer.end()); + Token Tok; + // Loop until the beginning of the initialization list. + while (!Tok.is(tok::r_paren)) + Lex.LexFromRawLexer(Tok); + Lex.LexFromRawLexer(Tok); + + std::string FixItMsg = ""; + // There is not initialization list in this constructor. + if (!Tok.is(tok::colon)) + FixItMsg += ": "; + FixItMsg += FixItInitList; + // We want to apply the missing constructor at the beginning of the + // initialization list. + if (Tok.is(tok::colon)) { + Lex.LexFromRawLexer(Tok); + FixItMsg += ", "; + } + + diag(Tok.getLocation(), "Copy constructor should call the copy " + "constructor of all copyable base class!") + << FixItHint::CreateInsertion(Tok.getLocation(), FixItMsg); +} + +} // namespace misc +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -77,6 +77,11 @@ Finds perfect forwarding constructors that can unintentionally hide copy or move constructors. +- New `misc-undelegated-copy-of-base-classes + `_ check + + Finds copy constructors where the ctor don't call the constructor of the base class. + - New `modernize-replace-random-shuffle `_ check Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -109,6 +109,7 @@ misc-throw-by-value-catch-by-reference misc-unconventional-assign-operator misc-undelegated-constructor + misc-undelegated-copy-of-base-classes misc-uniqueptr-reset-release misc-unused-alias-decls misc-unused-parameters Index: docs/clang-tidy/checks/misc-undelegated-copy-of-base-classes.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/misc-undelegated-copy-of-base-classes.rst @@ -0,0 +1,30 @@ +.. title:: clang-tidy - misc-undelegated-copy-of-base-classes + +misc-undelegated-copy-of-base-classes +===================================== + +Finds copy constructors where the constructor don't call +the constructor of the base class. + +.. code-block:: c++ + + class Copyable { + public: + Copyable() = default; + Copyable(const Copyable&) = default; + }; + class X2 : public Copyable { + X2(const X2& other) {}; // Copyable(other) is missing + }; + +Also finds copy constructors where the constructor of +the base class don't have parameter. + +.. code-block:: c++ + + class X4 : public Copyable { + X4(const X4& other): Copyable() {}; // other is missing + }; + +The check suggests a fix-it in every scenario including multiple +missing initializers and constructors with template argument. Index: test/clang-tidy/misc-undelegated-copy-of-base-classes.cpp =================================================================== --- /dev/null +++ test/clang-tidy/misc-undelegated-copy-of-base-classes.cpp @@ -0,0 +1,96 @@ +// RUN: %check_clang_tidy %s misc-undelegated-copy-of-base-classes %t + +class Copyable { + public: + Copyable() = default; + Copyable(const Copyable&) = default; +}; +class X : public Copyable { + X(const X& other) : Copyable(other) {} + //Good code: the copy ctor call the ctor of the base class. +}; + +class Copyable2 { + public: + Copyable2() = default; + Copyable2(const Copyable2&) = default; +}; +class X2 : public Copyable2 { + X2(const X2& other) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: Copy constructor should call the copy constructor of all copyable base class! [misc-undelegated-copy-of-base-classes] + // CHECK-FIXES: : Copyable2(other) +}; + +class X3 : public Copyable, public Copyable2 { + X3(const X3& other): Copyable(other) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: Copy constructor should call the copy constructor of all copyable base class! [misc-undelegated-copy-of-base-classes] + // CHECK-FIXES: Copyable2(other), +}; + +class X4 : public Copyable { + X4(const X4& other): Copyable() {}; + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: Missing parameter in the base initializer! [misc-undelegated-copy-of-base-classes] + // CHECK-FIXES: other +}; + +class Copyable3 : public Copyable { + public: + Copyable3() = default; + Copyable3(const Copyable3&) = default; +}; +class X5 : public Copyable3 { + X5(const X5& other) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: Copy constructor should call the copy constructor of all copyable base class! [misc-undelegated-copy-of-base-classes] + // CHECK-FIXES: : Copyable3(other) +}; + +class X6 : public Copyable2, public Copyable3 { + X6(const X6& other) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: Copy constructor should call the copy constructor of all copyable base class! [misc-undelegated-copy-of-base-classes] + // CHECK-FIXES: : Copyable2(other), Copyable3(other) +}; + +class X7 : public Copyable, public Copyable2 { + X7(const X7& other): Copyable() {}; + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: Missing parameter in the base initializer! [misc-undelegated-copy-of-base-classes] + // CHECK-FIXES: other + // CHECK-MESSAGES: :[[@LINE-3]]:22: warning: Copy constructor should call the copy constructor of all copyable base class! [misc-undelegated-copy-of-base-classes] + // CHECK-FIXES: Copyable2(other), +}; + +template +class Copyable4 { + public: + Copyable4() = default; + Copyable4(const Copyable4&) = default; +}; + +class X8 : public Copyable4 { + X8(const X8& other): Copyable4(other) {}; + //Good code: the copy ctor call the ctor of the base class. +}; + +class X9 : public Copyable4 { + X9(const X9& other): Copyable4() {}; + // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: Missing parameter in the base initializer! [misc-undelegated-copy-of-base-classes] + // CHECK-FIXES: other +}; + +class X10 : public Copyable4 { + X10(const X10& other) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: Copy constructor should call the copy constructor of all copyable base class! [misc-undelegated-copy-of-base-classes] + // CHECK-FIXES: : Copyable4(other) +}; + +template +class Copyable5 { + public: + Copyable5() = default; + Copyable5(const Copyable5&) = default; +}; + +class X11 : public Copyable5 { + X11(const X11& other) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: Copy constructor should call the copy constructor of all copyable base class! [misc-undelegated-copy-of-base-classes] + // CHECK-FIXES: : Copyable5(other) +};