[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

83 ↗

(On Diff #100892)

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

22 ↗

(On Diff #103385)

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

21 ↗

(On Diff #103385)

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 ↗

(On Diff #109564)

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

37 ↗

(On Diff #109564)

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

59 ↗

(On Diff #109564)

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

80 ↗

(On Diff #109564)

Please do not use auto here.

95 ↗

(On Diff #109564)

Space before the colon?

104 ↗

(On Diff #109564)

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 ↗

(On Diff #109564)

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 ↗

(On Diff #109564)

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 ↗

(On Diff #109564)

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 ↗

(On Diff #109564)

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 ↗

(On Diff #109564)

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

27 ↗

(On Diff #109564)

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

9 ↗

(On Diff #118617)

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

19 ↗

(On Diff #118617)

Spurious semicolon.

25 ↗

(On Diff #118617)

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

[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

38–40 ↗

(On Diff #121698)

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

69 ↗

(On Diff #121698)

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

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

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

109 ↗

(On Diff #121698)

ctros -> ctors

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

4 ↗

(On Diff #121698)

The markdown here is wrong.

6 ↗

(On Diff #121698)

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

69 ↗

(On Diff #121698)

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

69 ↗

(On Diff #121698)

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

61 ↗

(On Diff #121886)

Please don't use auto here.

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

29 ↗

(On Diff #121886)

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

clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp

 //===--- BugproneTidyModule.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 "CopyConstructorInitCheck.h"
 #include "IntegerDivisionCheck.h"
 #include "SuspiciousMemsetUsageCheck.h"
 #include "UndefinedMemoryManipulationCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace bugprone {
 
 class BugproneModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<CopyConstructorInitCheck>(
+        "misc-copy-constructor-init");
     CheckFactories.registerCheck<IntegerDivisionCheck>(
         "bugprone-integer-division");
     CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>(
         "bugprone-suspicious-memset-usage");
     CheckFactories.registerCheck<UndefinedMemoryManipulationCheck>(
         "bugprone-undefined-memory-manipulation");
   }
 };

clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyBugproneModule
   BugproneTidyModule.cpp
+  CopyConstructorInitCheck.cpp
   IntegerDivisionCheck.cpp
   SuspiciousMemsetUsageCheck.cpp
   UndefinedMemoryManipulationCheck.cpp
 
   LINK_LIBS
   clangAnalysis
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyUtils
   clangTooling
   )

clang-tools-extra/trunk/clang-tidy/bugprone/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_BUGPRONE_COPY_CONSTRUCTOR_INIT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_COPY_CONSTRUCTOR_INIT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds copy constructors where the ctor don't call the copy 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_BUGPRONE_COPY_CONSTRUCTOR_INIT_H

clang-tools-extra/trunk/clang-tidy/bugprone/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 bugprone {
+
+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 CXXRecordDecl *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

clang-tools-extra/trunk/docs/ReleaseNotes.rst

- New `android-cloexec-epoll-create
   Detects usage of ``epoll_create()``.
 
 - New `android-cloexec-memfd_create
   <http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-memfd_create.html>`_ check
 
   Checks if the required file flag ``MFD_CLOEXEC`` is present in the argument
   of ``memfd_create()``.
 
+- New `bugprone-copy-constructor-init
+  <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-copy-constructor-init.html>`_ check
+
+  Finds copy constructors which don't call the copy constructor of the base class.
+
 - New `bugprone-integer-division
   <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-integer-division.html>`_ check
 
   Finds cases where integer division in a floating point context is likely to
   cause unintended loss of precision.
 
 - New `cppcoreguidelines-owning-memory <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-owning-memory.html>`_ check 
 

clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-copy-constructor-init.rst

+.. title:: clang-tidy - bugprone-copy-constructor-init
+
+bugprone-copy-constructor-init
+==============================
+
+Finds copy constructors where the constructor doesn't call
+the copy 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 also suggests a fix-its in some cases.

clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

.. toctree::
    android-cloexec-epoll-create1
    android-cloexec-fopen
    android-cloexec-inotify-init
    android-cloexec-inotify-init1
    android-cloexec-memfd-create
    android-cloexec-open
    android-cloexec-socket
    boost-use-to-string
+   bugprone-copy-constructor-init
    bugprone-integer-division
    bugprone-suspicious-memset-usage
    bugprone-undefined-memory-manipulation
    cert-dcl03-c (redirects to misc-static-assert) <cert-dcl03-c>
    cert-dcl21-cpp
    cert-dcl50-cpp
    cert-dcl54-cpp (redirects to misc-new-delete-overloads) <cert-dcl54-cpp>
    cert-dcl58-cpp

clang-tools-extra/trunk/test/clang-tidy/bugprone-copy-constructor-init.cpp

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