[clang-tidy] Add checker for undelegated copy of base classes

[xazax.hun]

Summary

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.

[Eugene.Zelenko]

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

docs/clang-tidy/checks/misc-undelegated-copy-of-base-classes.rst

29 ↗

(On Diff #100858)

checker -> check, FixItHint -> fix-it.

[Eugene.Zelenko]

docs/clang-tidy/checks/misc-undelegated-copy-of-base-classes.rst

31 ↗

(On Diff #100858)

Please add newline.

[szdominik]

Update with fixed docs.

[Eugene.Zelenko]

docs/ReleaseNotes.rst

88

I think will be good idea to replace ctor with constructor. Or re-phrase sentence to avoid repetition of word constructor.

[alexfh]

I would be interested in seeing the results of this check's run on LLVM+Clang code.

[szdominik]

Warnings of the check's run on llvm/clang codebase.

undelegated-copy-of-base-classes-clangrun.txt2 KBDownload

[aaron.ballman]

clang-tidy/misc/UndelegatedCopyOfBaseClassesCheck.cpp

41 ↗

(On Diff #100892)

It seems like you can hoist out from the cxxCtorInitializer() onward and only write this code once rather than twice.

48 ↗

(On Diff #100892)

No need for the initializer.

53 ↗

(On Diff #100892)

Spurious parens.

55 ↗

(On Diff #100892)

Diagnostics are not full sentences, should this should use a lowercase m and drop the exclamation mark. Also, it should be "argument" rather than parameter.

I think it might be more clearly rewritten as: "calling an inherited constructor other than the copy constructor". Also, it would be helpful to point to which constructor *is* being called as a note.

63 ↗

(On Diff #100892)

const auto *

67 ↗

(On Diff #100892)

Do not use auto here.

92 ↗

(On Diff #100892)

This doesn't balance tokens. What about:

struct S {
 /* ... */
};

struct T : S {
  T(const T &O) : S((1 + 2), O) {}
};

96 ↗

(On Diff #100892)

No initializer needed.

108–109 ↗

(On Diff #100892)

Similar comments about the diagnostic as above. I think the two diagnostics should really be the same string. At the end of the day, the check is to ensure that a derived copy constructor calls an inherited copy constructor. Whether the copy constructor calls an inherited default constructor, or some other kind of constructor, is largely immaterial because they're both wrong in the same way. I'd recommend using the diagnostic text from my comment above for both.

[malcolm.parsons]

This check is similar to misc-move-constructor-init; could it have a similar name please.

[szdominik]

Rename check.
Hoisted matcher, changed warning message & nits.

[szdominik]

clang-tidy/misc/UndelegatedCopyOfBaseClassesCheck.cpp

92 ↗

(On Diff #100892)

The loop search only the first right paren in the line, which is the end of parameter list ( "...&0)" ), so, as I see, it doesn't interesting what happens after the colon.

[aaron.ballman]

clang-tidy/misc/UndelegatedCopyOfBaseClassesCheck.cpp

92 ↗

(On Diff #100892)

Let me try again. :-)

struct S {
 /* ... */
};

struct T : S {
  T(const T &O, int = (1 + 2)) : S((1 + 2), O) {}
};

[szdominik]

clang-tidy/misc/UndelegatedCopyOfBaseClassesCheck.cpp

92 ↗

(On Diff #100892)

Ah, you're right. I'll figure out something... :)

[szdominik]

Updated loop for searching the beginning of the initlist.

[xazax.hun]

clang-tidy/misc/CopyConstructorInitCheck.cpp

23

Wouldn't something like:

static auto ctorInit = cxxCtorInitializer(....);

work?

I would prefer that rather than a new AST_MATCHER.

And later you could use ctorInit without parens.

test/clang-tidy/misc-copy-constructor-init.cpp

22

These check-fixes lines should contain the whole line after the fixit is applied.

[szdominik]

Fixed check-fixes lines in test cases.
Updated matcher definition.

[szdominik]

There wasn't any update on this check lately - can I help to make it better?

[aaron.ballman]

clang-tidy/misc/CopyConstructorInitCheck.cpp

24

You pick a more readable name than cruct-expr, like construct-expr?

37

Wouldn't registering this matcher achieve the same goal instead of needing to re-match?

59

Please pick a name other than Decl, since that's a type name.

80

Please do not use auto here.

95

Space before the colon?

104

Insteaad of having to re-lex the physical source, can the AST should be modified to carry the information you need if it doesn't already have it? For instance, you can tell there is not initializer list by looking at CXXConstructorDecl::getNumCtorInitializers().

test/clang-tidy/misc-copy-constructor-init.cpp

27

Don't we want the ctor-inits to be in the same order as the bases are specified?

[klimek]

The run on llvm indicates that we don't want this to trigger if the base class doesn't have anything to copy (that is, no fields & a defaulted copy-ctor, or an empty copy-ctor).

[szdominik]

Small changes after aaron.ballman's comments.

clang-tidy/misc/CopyConstructorInitCheck.cpp

37

I wanted to create only one FixIt to every ctor - if I move the forEachCtorInitializer to the register part, it would warn us twice.

104

The getNumCtorInitializers method counts the generated initializers as well, not just the manually written ones.
My basic problem was that the AST's methods can't really distinguish the generated and the manually written initializers, so it's quite complicated to find the locations what we need. I found easier & more understandable if I start reading the tokens.

test/clang-tidy/misc-copy-constructor-init.cpp

27

I think it's more readable if we put the FixIts to the beginning of the expression - it's easier to check that everyting is correct & it's more obvious what the changes are.

[aaron.ballman]

clang-tidy/misc/CopyConstructorInitCheck.cpp

104

This sounds like a deficiency with the AST that should be rectified rather than worked around. Going back to lexing the source can be very expensive (think about source files that live on a network drive, for instance) and is often tricky to get correct. For instance, it seems the lexing starts at the constructor declaration itself, so does a default argument to that copy constructor using ?: cause issues? e.g., S(const S&, int = 0 ? 1 : 2)

test/clang-tidy/misc-copy-constructor-init.cpp

10

Please clang-format this file so it meets our usual formatting requirements.

20

Spurious semicolon.

26

Spurious semicolon (check the remainder of the file, this seems to be a common issue).

27

However, that then produces additional warnings because the ctor-inits are not in the canonical order (-Wreorder). See http://coliru.stacked-crooked.com/a/a9d77afe87618c13

[xazax.hun]

• Dominic said he no longer have time to continue with this patch, so I commandeered this revision
• Skip template instantiations
• Do not attempt fix macro expansions
• Do not attempt fix type aliases and typedef types
• Do not attempt to fix cases where the parameter is unnamed
• Use a more conservative approach and only warn for default constructors invoked by copy ctors. This could be refined in a followup patch.
• Skip the check for empty base classes
• Do not use matchers in the check callback
• Add tests for relevant changes
• Fix style issues in tests and docs
• Rebased for latest ToT

[xazax.hun]

Two problems are not resolved. One is Aaron prefers to query some infor from the AST instead of relexing. The second is providing base initializers in the wrong order.
I think there are other checks that do relexing in some cases, this should not be a blocker. I am not sure that we should complicate the fixit logic with the order. If -Wreorder has fixit hints, user should be able to get rid of the warning by applying that.

[aaron.ballman]

In D33722#915134, @xazax.hun wrote:

Two problems are not resolved. One is Aaron prefers to query some infor from the AST instead of relexing. The second is providing base initializers in the wrong order.
I think there are other checks that do relexing in some cases, this should not be a blocker.

In the other cases, it is not clear that the re-lexed information should be carried in the AST. In this case, I think it's pretty clear that the AST should carry this information. Further, I don't know that the re-lexing is correct (I pointed out an example that I think will be problematic) and carrying the information in the AST would solve that more cleanly than trying to reimplement the logic here.

I am not sure that we should complicate the fixit logic with the order. If -Wreorder has fixit hints, user should be able to get rid of the warning by applying that.

I disagree. We should not be changing code to be incorrect, requiring the user to find that next incorrectness only through an additional compilation. That is a rather poor user experience.

[xazax.hun]

• Do not warn for NonCopyable bases
• Remove lexing

[xazax.hun]

In D33722#915440, @aaron.ballman wrote:

In the other cases, it is not clear that the re-lexed information should be carried in the AST. In this case, I think it's pretty clear that the AST should carry this information. Further, I don't know that the re-lexing is correct (I pointed out an example that I think will be problematic) and carrying the information in the AST would solve that more cleanly than trying to reimplement the logic here.

Oh, you are right! Sorry for my oversight, I did not notice your example. I got a bit lost in all of the comments. Fortunately, I was able to get rid of the relexing, so this should be ok now.

I am not sure that we should complicate the fixit logic with the order. If -Wreorder has fixit hints, user should be able to get rid of the warning by applying that.

I disagree. We should not be changing code to be incorrect, requiring the user to find that next incorrectness only through an additional compilation. That is a rather poor user experience.

The resulting code is not incorrect. We do not introduce undefined behavior. But if we do not want to introduce new warnings either, I could skip the fixits if there is an initializer already for any of the base classes. What do you think?

[xazax.hun]

Also, bugprone might be a better module to put this?

[aaron.ballman]

In D33722#916539, @xazax.hun wrote:

In D33722#915440, @aaron.ballman wrote:

I disagree. We should not be changing code to be incorrect, requiring the user to find that next incorrectness only through an additional compilation. That is a rather poor user experience.

The resulting code is not incorrect. We do not introduce undefined behavior.

Undefined behavior is not the only form of incorrectness. The resulting code is incorrect because the initalization order does not match the declaration order and relying on that behavior leads to bugs, which is why -Wreorder is on by default.

But if we do not want to introduce new warnings either, I could skip the fixits if there is an initializer already for any of the base classes. What do you think?

I think that's reasonable behavior.

clang-tidy/misc/CopyConstructorInitCheck.cpp

39–41

I'm not certain this function is all that useful -- it could be replaced with Class->field_empty() in the caller (even as a lambda if needed).

70

What if the base class is inherited privately? e.g.,

struct Base {
  Base(const Base&) {}
};

struct Derived : private Base {
  Derived(const Derived &) {}
};

110

ctros -> ctors

docs/clang-tidy/checks/misc-copy-constructor-init.rst

5

The markdown here is wrong.

7

don't -> doesn't

[aaron.ballman]

In D33722#916540, @xazax.hun wrote:

Also, bugprone might be a better module to put this?

I don't have strong opinions on misc vs bugprone (they're both effectively catch-alls for tidy checks, as best I can tell).

[xazax.hun]

• Fix review comments

[xazax.hun]

• Fix doc comments that I overlooked earlier

[xazax.hun]

In D33722#916990, @aaron.ballman wrote:

In D33722#916540, @xazax.hun wrote:

Also, bugprone might be a better module to put this?

I don't have strong opinions on misc vs bugprone (they're both effectively catch-alls for tidy checks, as best I can tell).

@alexfh, do you have an opinion here?

clang-tidy/misc/CopyConstructorInitCheck.cpp

70

We warn in that case too. I added a test to demonstrate this. I think we still want to copy private bases in copy ctors if they are not empty and copyable.

[aaron.ballman]

clang-tidy/misc/CopyConstructorInitCheck.cpp

70

Good, thank you for adding the test (and I agree, we want to warn in that case).

[aaron.ballman]

Aside from two minor nits, the check LGTM. Whether we put it in misc or bugprone can be answered by @alexfh or by your best judgement.

clang-tidy/misc/CopyConstructorInitCheck.cpp

62

Please don't use auto here.

docs/clang-tidy/checks/misc-copy-constructor-init.rst

30

This comment is no longer accurate (there are some times we don't supply a fix-it).

clang-tidy/misc/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyMiscModule
   ArgumentCommentCheck.cpp
   AssertSideEffectCheck.cpp
   ForwardingReferenceOverloadCheck.cpp
   MisplacedConstCheck.cpp
   UnconventionalAssignOperatorCheck.cpp
   BoolPointerImplicitConversionCheck.cpp
+  CopyConstructorInitCheck.cpp
   DanglingHandleCheck.cpp
   DefinitionsInHeadersCheck.cpp
   FoldInitTypeCheck.cpp
   ForwardDeclarationNamespaceCheck.cpp
   InaccurateEraseCheck.cpp
   IncorrectRoundings.cpp
   InefficientAlgorithmCheck.cpp
   MacroParenthesesCheck.cpp

clang-tidy/misc/CopyConstructorInitCheck.h

+//===--- 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

clang-tidy/misc/CopyConstructorInitCheck.cpp

+//===--- 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

clang-tidy/misc/MiscTidyModule.cpp

 //===--- MiscTidyModule.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 "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "ArgumentCommentCheck.h"
 #include "AssertSideEffectCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
+#include "CopyConstructorInitCheck.h"
 #include "DanglingHandleCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "FoldInitTypeCheck.h"
 #include "ForwardDeclarationNamespaceCheck.h"
 #include "ForwardingReferenceOverloadCheck.h"
 #include "InaccurateEraseCheck.h"
 #include "IncorrectRoundings.h"
 #include "InefficientAlgorithmCheck.h"
CheckFactories.registerCheck<AssertSideEffectCheck>(
         "misc-assert-side-effect");
     CheckFactories.registerCheck<ForwardingReferenceOverloadCheck>(
         "misc-forwarding-reference-overload");
     CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
     CheckFactories.registerCheck<UnconventionalAssignOperatorCheck>(
         "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");
     CheckFactories.registerCheck<FoldInitTypeCheck>("misc-fold-init-type");
     CheckFactories.registerCheck<ForwardDeclarationNamespaceCheck>(
         "misc-forward-declaration-namespace");
     CheckFactories.registerCheck<InaccurateEraseCheck>("misc-inaccurate-erase");
     CheckFactories.registerCheck<IncorrectRoundings>(

docs/ReleaseNotes.rst

- New `cert-dcl58-cpp
   <http://clang.llvm.org/extra/clang-tidy/checks/cert-dcl58-cpp.html>`_ check
 
   Finds modification of the ``std`` or ``posix`` namespace.
 
 - Improved `cppcoreguidelines-no-malloc
   <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
 
   Finds perfect forwarding constructors that can unintentionally hide copy or move constructors.
 
 - New `modernize-replace-random-shuffle
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-replace-random-shuffle.html>`_ check
 
   Finds and fixes usage of ``std::random_shuffle`` as the function has been removed from C++17.
 
 - New `modernize-return-braced-init-list
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-return-braced-init-list.html>`_ check
 
   Finds and replaces explicit calls to the constructor in a return statement by
   a braced initializer list so that the return type is not needlessly repeated.
   
 - Improved `modernize-use-emplace

docs/clang-tidy/checks/list.rst

.. toctree::
    hicpp-use-override
    llvm-header-guard
    llvm-include-order
    llvm-namespace-comment
    llvm-twine-local
    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
    misc-forward-declaration-namespace
    misc-forwarding-reference-overload
    misc-inaccurate-erase
    misc-incorrect-roundings
    misc-inefficient-algorithm

docs/clang-tidy/checks/misc-copy-constructor-init.rst

+.. 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.

test/clang-tidy/misc-copy-constructor-init.cpp

+// 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) {};
+};