xazax.hun updated this revision to Diff 121886.
xazax.hun marked 2 inline comments as done.
xazax.hun added a comment.
- Fix doc comments that I overlooked earlier
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,188 @@
+// RUN: %check_clang_tidy %s misc-copy-constructor-init %t
+
+class NonCopyable {
+public:
+ NonCopyable() = default;
+ NonCopyable(const NonCopyable &) = delete;
+
+private:
+ int a;
+};
+
+class NonCopyable2 {
+public:
+ NonCopyable2() = default;
+
+private:
+ NonCopyable2(const NonCopyable2 &);
+ int a;
+};
+
+class Copyable {
+public:
+ Copyable() = default;
+ Copyable(const Copyable &) = default;
+
+private:
+ int a;
+};
+
+class Copyable2 {
+public:
+ Copyable2() = default;
+ Copyable2(const Copyable2 &) = default;
+
+private:
+ int a;
+};
+
+class Copyable3 : public Copyable {
+public:
+ Copyable3() = default;
+ Copyable3(const Copyable3 &) = default;
+};
+
+template <class C>
+class Copyable4 {
+public:
+ Copyable4() = default;
+ Copyable4(const Copyable4 &) = default;
+
+private:
+ int a;
+};
+
+template <class T, class S>
+class Copyable5 {
+public:
+ Copyable5() = default;
+ Copyable5(const Copyable5 &) = default;
+
+private:
+ int a;
+};
+
+class EmptyCopyable {
+public:
+ EmptyCopyable() = default;
+ EmptyCopyable(const EmptyCopyable &) = default;
+};
+
+template <typename T>
+using CopyableAlias = Copyable5<T, int>;
+
+typedef Copyable5<int, int> CopyableAlias2;
+
+class X : public Copyable, public EmptyCopyable {
+ X(const X &other) : Copyable(other) {}
+};
+
+class X2 : public Copyable2 {
+ X2(const X2 &other) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor other than the copy constructor [misc-copy-constructor-init]
+ // CHECK-FIXES: X2(const X2 &other) : Copyable2(other) {}
+};
+
+class X2_A : public Copyable2 {
+ X2_A(const X2_A &) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+ // CHECK-FIXES: X2_A(const X2_A &) {}
+};
+
+class X3 : public Copyable, public Copyable2 {
+ X3(const X3 &other) : Copyable(other) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+ // CHECK-FIXES: X3(const X3 &other) : Copyable(other) {}
+};
+
+class X4 : public Copyable {
+ X4(const X4 &other) : Copyable() {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+ // CHECK-FIXES: X4(const X4 &other) : Copyable(other) {}
+};
+
+class X5 : public Copyable3 {
+ X5(const X5 &other) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+ // CHECK-FIXES: X5(const X5 &other) : Copyable3(other) {}
+};
+
+class X6 : public Copyable2, public Copyable3 {
+ X6(const X6 &other) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+ // 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]]:3: warning: calling a base constructor
+ // CHECK-FIXES: X7(const X7 &other) : Copyable(other) {}
+};
+
+class X8 : public Copyable4<int> {
+ X8(const X8 &other) : Copyable4(other) {}
+};
+
+class X9 : public Copyable4<int> {
+ X9(const X9 &other) : Copyable4() {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+ // CHECK-FIXES: X9(const X9 &other) : Copyable4(other) {}
+};
+
+class X10 : public Copyable4<int> {
+ X10(const X10 &other) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+ // CHECK-FIXES: X10(const X10 &other) : Copyable4(other) {}
+};
+
+class X11 : public Copyable5<int, float> {
+ X11(const X11 &other) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+ // CHECK-FIXES: X11(const X11 &other) : Copyable5(other) {}
+};
+
+class X12 : public CopyableAlias<float> {
+ X12(const X12 &other) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+ // CHECK-FIXES: X12(const X12 &other) {}
+};
+
+template <typename T>
+class X13 : T {
+ X13(const X13 &other) {}
+};
+
+template class X13<EmptyCopyable>;
+template class X13<Copyable>;
+
+#define FROMMACRO \
+ class X14 : public Copyable2 { \
+ X14(const X14 &other) {} \
+ };
+
+FROMMACRO
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: calling a base constructor
+
+class X15 : public CopyableAlias2 {
+ X15(const X15 &other) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+ // CHECK-FIXES: X15(const X15 &other) {}
+};
+
+class X16 : public NonCopyable {
+ X16(const X16 &other) {}
+};
+
+class X17 : public NonCopyable2 {
+ X17(const X17 &other) {}
+};
+
+class X18 : private Copyable {
+ X18(const X18 &other) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+ // CHECK-FIXES: X18(const X18 &other) : Copyable(other) {}
+};
+
+class X19 : private Copyable {
+ X19(const X19 &other) : Copyable(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 doesn'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
@@ -103,6 +103,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
@@ -152,6 +152,12 @@
Finds member expressions that access static members through instances and
replaces them with uses of the appropriate qualified-id.
+- 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.
+
+
- Added `modernize-use-emplace.IgnoreImplicitConstructors
<http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html#cmdoption-arg-IgnoreImplicitConstructors>`_
option.
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"
@@ -76,6 +77,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,120 @@
+//===--- 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 {
+
+void CopyConstructorInitCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ // In the future this might be extended to move constructors?
+ Finder->addMatcher(
+ cxxConstructorDecl(
+ isCopyConstructor(),
+ hasAnyConstructorInitializer(cxxCtorInitializer(
+ isBaseInitializer(),
+ withInitializer(cxxConstructExpr(hasDeclaration(
+ cxxConstructorDecl(isDefaultConstructor())))))),
+ unless(isInstantiated()))
+ .bind("ctor"),
+ this);
+}
+
+void CopyConstructorInitCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
+ std::string ParamName = Ctor->getParamDecl(0)->getNameAsString();
+
+ // We want only one warning (and FixIt) for each ctor.
+ std::string FixItInitList;
+ bool HasRelevantBaseInit = false;
+ bool ShouldNotDoFixit = false;
+ bool HasWrittenInitializer = false;
+ SmallVector<FixItHint, 2> SafeFixIts;
+ for (const auto *Init : Ctor->inits()) {
+ bool CtorInitIsWritten = Init->isWritten();
+ HasWrittenInitializer = HasWrittenInitializer || CtorInitIsWritten;
+ if (!Init->isBaseInitializer())
+ continue;
+ const Type *BaseType = Init->getBaseClass();
+ // Do not do fixits if there is a type alias involved or one of the bases
+ // are explicitly initialized. In the latter case we not do fixits to avoid
+ // -Wreorder warnings.
+ if (const auto *TempSpecTy = dyn_cast<TemplateSpecializationType>(BaseType))
+ ShouldNotDoFixit = ShouldNotDoFixit || TempSpecTy->isTypeAlias();
+ ShouldNotDoFixit = ShouldNotDoFixit || isa<TypedefType>(BaseType);
+ ShouldNotDoFixit = ShouldNotDoFixit || CtorInitIsWritten;
+ const auto *BaseClass = BaseType->getAsCXXRecordDecl()->getDefinition();
+ if (BaseClass->field_empty() &&
+ BaseClass->forallBases(
+ [](const CXXRecordDecl *Class) { return Class->field_empty(); }))
+ continue;
+ bool NonCopyableBase = false;
+ for (const auto *Ctor : BaseClass->ctors()) {
+ if (Ctor->isCopyConstructor() &&
+ (Ctor->getAccess() == AS_private || Ctor->isDeleted())) {
+ NonCopyableBase = true;
+ break;
+ }
+ }
+ if (NonCopyableBase)
+ continue;
+ const auto *CExpr = dyn_cast<CXXConstructExpr>(Init->getInit());
+ if (!CExpr || !CExpr->getConstructor()->isDefaultConstructor())
+ continue;
+ HasRelevantBaseInit = true;
+ if (CtorInitIsWritten) {
+ if (!ParamName.empty())
+ SafeFixIts.push_back(
+ FixItHint::CreateInsertion(CExpr->getLocEnd(), ParamName));
+ } else {
+ if (Init->getSourceLocation().isMacroID() ||
+ Ctor->getLocation().isMacroID() || ShouldNotDoFixit)
+ break;
+ FixItInitList += BaseClass->getNameAsString();
+ FixItInitList += "(" + ParamName + "), ";
+ }
+ }
+ if (!HasRelevantBaseInit)
+ return;
+
+ auto Diag = diag(Ctor->getLocation(),
+ "calling a base constructor other than the copy constructor")
+ << SafeFixIts;
+
+ if (FixItInitList.empty() || ParamName.empty() || ShouldNotDoFixit)
+ return;
+
+ std::string FixItMsg{FixItInitList.substr(0, FixItInitList.size() - 2)};
+ SourceLocation FixItLoc;
+ // There is no initialization list in this constructor.
+ if (!HasWrittenInitializer) {
+ FixItLoc = Ctor->getBody()->getLocStart();
+ FixItMsg = " : " + FixItMsg;
+ } else {
+ // We apply the missing ctors at the beginning of the initialization list.
+ FixItLoc = (*Ctor->init_begin())->getSourceLocation();
+ FixItMsg += ',';
+ }
+ FixItMsg += ' ';
+
+ Diag << FixItHint::CreateInsertion(FixItLoc, FixItMsg);
+} // namespace misc
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -8,6 +8,7 @@
MisplacedConstCheck.cpp
UnconventionalAssignOperatorCheck.cpp
BoolPointerImplicitConversionCheck.cpp
+ CopyConstructorInitCheck.cpp
DanglingHandleCheck.cpp
DefinitionsInHeadersCheck.cpp
FoldInitTypeCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
