szdominik updated this revision to Diff 109564.
szdominik marked 2 inline comments as done.
szdominik added a comment.
Herald added a subscriber: JDevlieghere.
Fixed check-fixes lines in test cases.
Updated matcher definition.
https://reviews.llvm.org/D33722
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/CopyConstructorInitCheck.cpp
clang-tidy/misc/CopyConstructorInitCheck.h
clang-tidy/misc/MiscTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-copy-constructor-init.rst
test/clang-tidy/misc-copy-constructor-init.cpp
Index: test/clang-tidy/misc-copy-constructor-init.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-copy-constructor-init.cpp
@@ -0,0 +1,95 @@
+// RUN: %check_clang_tidy %s misc-copy-constructor-init %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: calling an inherited constructor other than the copy constructor [misc-copy-constructor-init]
+ // CHECK-FIXES: X2(const X2& other) : Copyable2(other) {};
+};
+
+class X3 : public Copyable, public Copyable2 {
+ X3(const X3& other): Copyable(other) {};
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: calling an inherited constructor other than the copy constructor [misc-copy-constructor-init]
+ // CHECK-FIXES: X3(const X3& other): Copyable2(other), Copyable(other) {};
+};
+
+class X4 : public Copyable {
+ X4(const X4& other): Copyable() {};
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: calling an inherited constructor other than the copy constructor [misc-copy-constructor-init]
+ // CHECK-FIXES: X4(const X4& other): Copyable(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: calling an inherited constructor other than the copy constructor [misc-copy-constructor-init]
+ // CHECK-FIXES: X5(const X5& other) : Copyable3(other) {};
+};
+
+class X6 : public Copyable2, public Copyable3 {
+ X6(const X6& other) {};
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: calling an inherited constructor other than the copy constructor [misc-copy-constructor-init]
+ // CHECK-FIXES: X6(const X6& other) : Copyable2(other), Copyable3(other) {};
+};
+
+class X7 : public Copyable, public Copyable2 {
+ X7(const X7& other): Copyable() {};
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: calling an inherited constructor other than the copy constructor [misc-copy-constructor-init]
+ // CHECK-MESSAGES: :[[@LINE-2]]:22: warning: calling an inherited constructor other than the copy constructor [misc-copy-constructor-init]
+ // CHECK-FIXES: X7(const X7& other): Copyable2(other), Copyable(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: calling an inherited constructor other than the copy constructor [misc-copy-constructor-init]
+ // CHECK-FIXES: X9(const X9& other): Copyable4<int>(other) {};
+};
+
+class X10 : public Copyable4<int> {
+ X10(const X10& other) {};
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: calling an inherited constructor other than the copy constructor [misc-copy-constructor-init]
+ // CHECK-FIXES: X10(const X10& other) : 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: calling an inherited constructor other than the copy constructor [misc-copy-constructor-init]
+ // CHECK-FIXES: X11(const X11& other) : Copyable5<int, float>(other) {};
+};
Index: docs/clang-tidy/checks/misc-copy-constructor-init.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-copy-constructor-init.rst
@@ -0,0 +1,30 @@
+.. title:: clang-tidy - misc-copy-constructor-init
+
+misc-copy-constructor-init
+=====================================
+
+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: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -74,6 +74,7 @@
misc-argument-comment
misc-assert-side-effect
misc-bool-pointer-implicit-conversion
+ misc-copy-constructor-init
misc-dangling-handle
misc-definitions-in-headers
misc-fold-init-type
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -71,7 +71,12 @@
<http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-no-malloc.html>`_ check
Allow custom memory management functions to be considered as well.
-
+
+- New `misc-copy-constructor-init
+ <http://clang.llvm.org/extra/clang-tidy/checks/misc-copy-constructor-init.html>`_ check
+
+ Finds copy constructors which don't call the constructor of the base class.
+
- New `misc-forwarding-reference-overload
<http://clang.llvm.org/extra/clang-tidy/checks/misc-forwarding-reference-overload.html>`_ check
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -13,6 +13,7 @@
#include "ArgumentCommentCheck.h"
#include "AssertSideEffectCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
+#include "CopyConstructorInitCheck.h"
#include "DanglingHandleCheck.h"
#include "DefinitionsInHeadersCheck.h"
#include "FoldInitTypeCheck.h"
@@ -73,6 +74,8 @@
"misc-unconventional-assign-operator");
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
"misc-bool-pointer-implicit-conversion");
+ CheckFactories.registerCheck<CopyConstructorInitCheck>(
+ "misc-copy-constructor-init");
CheckFactories.registerCheck<DanglingHandleCheck>("misc-dangling-handle");
CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
"misc-definitions-in-headers");
Index: clang-tidy/misc/CopyConstructorInitCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/CopyConstructorInitCheck.h
@@ -0,0 +1,36 @@
+//===--- CopyConstructorInitCheck.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_COPY_CONSTRUCTOR_INIT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_COPY_CONSTRUCTOR_INIT_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-copy-constructor-init.html
+class CopyConstructorInitCheck : public ClangTidyCheck {
+public:
+ CopyConstructorInitCheck(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_COPY_CONSTRUCTOR_INIT_H
Index: clang-tidy/misc/CopyConstructorInitCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/CopyConstructorInitCheck.cpp
@@ -0,0 +1,111 @@
+//===--- CopyConstructorInitCheck.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 "CopyConstructorInitCheck.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 {
+
+static auto ctorInit = cxxCtorInitializer(
+ isBaseInitializer(),
+ withInitializer(cxxConstructExpr(unless(hasDescendant(implicitCastExpr())))
+ .bind("cruct-expr")));
+
+void CopyConstructorInitCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ cxxConstructorDecl(isCopyConstructor(),
+ hasAnyConstructorInitializer(ctorInit.bind("init")))
+ .bind("ctor"),
+ this);
+}
+
+void CopyConstructorInitCheck::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(ctorInit.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(),
+ "calling an inherited constructor other than the copy constructor")
+ << 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 (const auto *Decl = dyn_cast<ClassTemplateSpecializationDecl>(
+ Init->getBaseClass()->getAsCXXRecordDecl())) {
+ FixItInitList += "<";
+
+ const ArrayRef<TemplateArgument> 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 (if exists).
+ while (!Tok.is(tok::colon) && !Tok.is(tok::l_brace))
+ Lex.LexFromRawLexer(Tok);
+
+ std::string FixItMsg;
+ // There is not initialization list in this constructor.
+ if (Tok.is(tok::l_brace))
+ 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(),
+ "calling an inherited constructor other than the copy constructor")
+ << FixItHint::CreateInsertion(Tok.getLocation(), FixItMsg);
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -7,6 +7,7 @@
MisplacedConstCheck.cpp
UnconventionalAssignOperatorCheck.cpp
BoolPointerImplicitConversionCheck.cpp
+ CopyConstructorInitCheck.cpp
DanglingHandleCheck.cpp
DefinitionsInHeadersCheck.cpp
FoldInitTypeCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
