szdominik created this revision.
Herald added a subscriber: mgorny.
Finds copy constructors where the constructor don't call
the constructor of the base class.
class X : public Copyable {
X(const X& other) {}; // Copyable(other) is missing
};
Also finds copy constructors where the base initializer
don't have parameter.
https://reviews.llvm.org/D33722
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/UndelegatedCopyOfBaseClassesCheck.cpp
clang-tidy/misc/UndelegatedCopyOfBaseClassesCheck.h
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-undelegated-copy-of-base-classes.rst
test/clang-tidy/misc-undelegated-copy-of-base-classes.cpp
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 C>
+class Copyable4 {
+ public:
+ Copyable4() = default;
+ Copyable4(const Copyable4&) = default;
+};
+
+class X8 : public Copyable4<int> {
+ X8(const X8& other): Copyable4<int>(other) {};
+ //Good code: the copy ctor call the ctor of the base class.
+};
+
+class X9 : public Copyable4<int> {
+ X9(const X9& other): Copyable4<int>() {};
+ // 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<int> {
+ 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<int>(other)
+};
+
+template <class T, class S>
+class Copyable5 {
+ public:
+ Copyable5() = default;
+ Copyable5(const Copyable5&) = default;
+};
+
+class X11 : public Copyable5<int, float> {
+ 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<int, float>(other)
+};
\ No newline at end of file
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 checker suggests a FixItHint in every scenario including multiple
+missing initializers and constructors with template argument.
\ No newline at end of file
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: 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<CXXConstructorDecl>("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<CXXCtorInitializer>("init");
+
+ // Valid when the initializer is written manually (not generated).
+ if ((Init->getSourceRange()).isValid()) {
+ const auto *CExpr = Match.getNodeAs<CXXConstructExpr>("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<ClassTemplateSpecializationDecl>(
+ 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: 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<UndelegatedConstructorCheck>(
"misc-undelegated-constructor");
+ CheckFactories.registerCheck<UndelegatedCopyOfBaseClassesCheck>(
+ "misc-undelegated-copy-of-base-classes");
CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(
"misc-uniqueptr-reset-release");
CheckFactories.registerCheck<UnusedAliasDeclsCheck>(
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
