Fix some false-positives with cppcoreguidelines-pro-type-member-init. Handle classes with default constructors that are defaulted or are not present in the AST.

Classes with virtual methods or virtual bases are not trivially default constructible, so their members and bases need to be initialized.

Patch by Malcolm Parsons.

llvm-svn: 283224

2 files changed, 62 insertions, 35 deletions

diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
index 4cf7b1600c1..4ae258bdeb1 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -32,17 +32,17 @@ namespace {
 // the record type (or indirect field) is a union, forEachField will stop after
 // the first field.
 template <typename T, typename Func>
-void forEachField(const RecordDecl *Record, const T &Fields,
+void forEachField(const RecordDecl &Record, const T &Fields,
                   bool OneFieldPerUnion, Func &&Fn) {
   for (const FieldDecl *F : Fields) {
     if (F->isAnonymousStructOrUnion()) {
       if (const CXXRecordDecl *R = F->getType()->getAsCXXRecordDecl())
-        forEachField(R, R->fields(), OneFieldPerUnion, Fn);
+        forEachField(*R, R->fields(), OneFieldPerUnion, Fn);
     } else {
       Fn(F);
     }
 
-    if (OneFieldPerUnion && Record->isUnion())
+    if (OneFieldPerUnion && Record.isUnion())
       break;
   }
 }
@@ -214,16 +214,16 @@ computeInsertions(const CXXConstructorDecl::init_const_range &Inits,
 
 // Gets the list of bases and members that could possibly be initialized, in
 // order as they appear in the class declaration.
-void getInitializationsInOrder(const CXXRecordDecl *ClassDecl,
+void getInitializationsInOrder(const CXXRecordDecl &ClassDecl,
                                SmallVectorImpl<const NamedDecl *> &Decls) {
   Decls.clear();
-  for (const auto &Base : ClassDecl->bases()) {
+  for (const auto &Base : ClassDecl.bases()) {
     // Decl may be null if the base class is a template parameter.
     if (const NamedDecl *Decl = getCanonicalRecordDecl(Base.getType())) {
       Decls.emplace_back(Decl);
     }
   }
-  forEachField(ClassDecl, ClassDecl->fields(), false,
+  forEachField(ClassDecl, ClassDecl.fields(), false,
                [&](const FieldDecl *F) { Decls.push_back(F); });
 }
 
@@ -236,7 +236,7 @@ void fixInitializerList(const ASTContext &Context, DiagnosticBuilder &Diag,
     return;
 
   SmallVector<const NamedDecl *, 16> OrderedDecls;
-  getInitializationsInOrder(Ctor->getParent(), OrderedDecls);
+  getInitializationsInOrder(*Ctor->getParent(), OrderedDecls);
 
   for (const auto &Insertion :
        computeInsertions(Ctor->inits(), OrderedDecls, DeclsToInit)) {
@@ -269,6 +269,19 @@ void ProTypeMemberInitCheck::registerMatchers(MatchFinder *Finder) {
                                IsNonTrivialDefaultConstructor))
           .bind("ctor"),
       this);
+
+  // Match classes with a default constructor that is defaulted or is not in the
+  // AST.
+  Finder->addMatcher(
+      cxxRecordDecl(
+          isDefinition(), unless(isInstantiated()),
+          anyOf(has(cxxConstructorDecl(isDefaultConstructor(), isDefaulted(),
+                                       unless(isImplicit()))),
+                unless(has(cxxConstructorDecl()))),
+          unless(isTriviallyDefaultConstructible()))
+          .bind("record"),
+      this);
+
   auto HasDefaultConstructor = hasInitializer(
       cxxConstructExpr(unless(requiresZeroInitialization()),
                        hasDeclaration(cxxConstructorDecl(
@@ -287,8 +300,14 @@ void ProTypeMemberInitCheck::check(const MatchFinder::MatchResult &Result) {
     // Skip declarations delayed by late template parsing without a body.
     if (!Ctor->getBody())
       return;
-    checkMissingMemberInitializer(*Result.Context, Ctor);
-    checkMissingBaseClassInitializer(*Result.Context, Ctor);
+    checkMissingMemberInitializer(*Result.Context, *Ctor->getParent(), Ctor);
+    checkMissingBaseClassInitializer(*Result.Context, *Ctor->getParent(), Ctor);
+  } else if (const auto *Record =
+                 Result.Nodes.getNodeAs<CXXRecordDecl>("record")) {
+    assert(Record->hasDefaultConstructor() &&
+           "Matched record should have a default constructor");
+    checkMissingMemberInitializer(*Result.Context, *Record, nullptr);
+    checkMissingBaseClassInitializer(*Result.Context, *Record, nullptr);
   } else if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var")) {
     checkUninitializedTrivialType(*Result.Context, Var);
   }
@@ -299,16 +318,16 @@ void ProTypeMemberInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
 }
 
 void ProTypeMemberInitCheck::checkMissingMemberInitializer(
-    ASTContext &Context, const CXXConstructorDecl *Ctor) {
-  const CXXRecordDecl *ClassDecl = Ctor->getParent();
-  bool IsUnion = ClassDecl->isUnion();
+    ASTContext &Context, const CXXRecordDecl &ClassDecl,
+    const CXXConstructorDecl *Ctor) {
+  bool IsUnion = ClassDecl.isUnion();
 
-  if (IsUnion && ClassDecl->hasInClassInitializer())
+  if (IsUnion && ClassDecl.hasInClassInitializer())
     return;
 
   // Gather all fields (direct and indirect) that need to be initialized.
   SmallPtrSet<const FieldDecl *, 16> FieldsToInit;
-  forEachField(ClassDecl, ClassDecl->fields(), false, [&](const FieldDecl *F) {
+  forEachField(ClassDecl, ClassDecl.fields(), false, [&](const FieldDecl *F) {
     if (!F->hasInClassInitializer() &&
         utils::type_traits::isTriviallyDefaultConstructible(F->getType(),
                                                             Context))
@@ -317,21 +336,23 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer(
   if (FieldsToInit.empty())
     return;
 
-  for (const CXXCtorInitializer *Init : Ctor->inits()) {
-    // Remove any fields that were explicitly written in the initializer list
-    // or in-class.
-    if (Init->isAnyMemberInitializer() && Init->isWritten()) {
-      if (IsUnion)
-        return; // We can only initialize one member of a union.
-      FieldsToInit.erase(Init->getAnyMember());
+  if (Ctor) {
+    for (const CXXCtorInitializer *Init : Ctor->inits()) {
+      // Remove any fields that were explicitly written in the initializer list
+      // or in-class.
+      if (Init->isAnyMemberInitializer() && Init->isWritten()) {
+        if (IsUnion)
+          return; // We can only initialize one member of a union.
+        FieldsToInit.erase(Init->getAnyMember());
+      }
     }
+    removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit);
   }
-  removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit);
 
   // Collect all fields in order, both direct fields and indirect fields from
   // anonmyous record types.
   SmallVector<const FieldDecl *, 16> OrderedFields;
-  forEachField(ClassDecl, ClassDecl->fields(), false,
+  forEachField(ClassDecl, ClassDecl.fields(), false,
                [&](const FieldDecl *F) { OrderedFields.push_back(F); });
 
   // Collect all the fields we need to initialize, including indirect fields.
@@ -342,14 +363,15 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer(
     return;
 
   DiagnosticBuilder Diag =
-      diag(Ctor->getLocStart(),
+      diag(Ctor ? Ctor->getLocStart() : ClassDecl.getLocation(),
            IsUnion
                ? "union constructor should initialize one of these fields: %0"
                : "constructor does not initialize these fields: %0")
       << toCommaSeparatedString(OrderedFields, AllFieldsToInit);
 
-  // Do not propose fixes in macros since we cannot place them correctly.
-  if (Ctor->getLocStart().isMacroID())
+  // Do not propose fixes for constructors in macros since we cannot place them
+  // correctly.
+  if (Ctor && Ctor->getLocStart().isMacroID())
     return;
 
   // Collect all fields but only suggest a fix for the first member of unions,
@@ -370,20 +392,20 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer(
           getLocationForEndOfToken(Context, Field->getSourceRange().getEnd()),
           "{}");
     }
-  } else {
+  } else if (Ctor) {
     // Otherwise, rewrite the constructor's initializer list.
     fixInitializerList(Context, Diag, Ctor, FieldsToFix);
   }
 }
 
 void ProTypeMemberInitCheck::checkMissingBaseClassInitializer(
-    const ASTContext &Context, const CXXConstructorDecl *Ctor) {
-  const CXXRecordDecl *ClassDecl = Ctor->getParent();
+    const ASTContext &Context, const CXXRecordDecl &ClassDecl,
+    const CXXConstructorDecl *Ctor) {
 
   // Gather any base classes that need to be initialized.
   SmallVector<const RecordDecl *, 4> AllBases;
   SmallPtrSet<const RecordDecl *, 4> BasesToInit;
-  for (const CXXBaseSpecifier &Base : ClassDecl->bases()) {
+  for (const CXXBaseSpecifier &Base : ClassDecl.bases()) {
     if (const auto *BaseClassDecl = getCanonicalRecordDecl(Base.getType())) {
       AllBases.emplace_back(BaseClassDecl);
       if (!BaseClassDecl->field_empty() &&
@@ -397,20 +419,23 @@ void ProTypeMemberInitCheck::checkMissingBaseClassInitializer(
     return;
 
   // Remove any bases that were explicitly written in the initializer list.
-  for (const CXXCtorInitializer *Init : Ctor->inits()) {
-    if (Init->isBaseInitializer() && Init->isWritten())
-      BasesToInit.erase(Init->getBaseClass()->getAsCXXRecordDecl());
+  if (Ctor) {
+    for (const CXXCtorInitializer *Init : Ctor->inits()) {
+      if (Init->isBaseInitializer() && Init->isWritten())
+        BasesToInit.erase(Init->getBaseClass()->getAsCXXRecordDecl());
+    }
   }
 
   if (BasesToInit.empty())
     return;
 
   DiagnosticBuilder Diag =
-      diag(Ctor->getLocStart(),
+      diag(Ctor ? Ctor->getLocStart() : ClassDecl.getLocation(),
            "constructor does not initialize these bases: %0")
       << toCommaSeparatedString(AllBases, BasesToInit);
 
-  fixInitializerList(Context, Diag, Ctor, BasesToInit);
+  if (Ctor)
+      fixInitializerList(Context, Diag, Ctor, BasesToInit);
 }
 
 void ProTypeMemberInitCheck::checkUninitializedTrivialType(
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
index 401fe0378f2..20d3f60fd8d 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
@@ -46,11 +46,13 @@ private:
   // To fix: Write a data member initializer, or mention it in the member
   // initializer list.
   void checkMissingMemberInitializer(ASTContext &Context,
+                                     const CXXRecordDecl &ClassDecl,
                                      const CXXConstructorDecl *Ctor);
 
   // A subtle side effect of Type.6 part 2:
   // Make sure to initialize trivially constructible base classes.
   void checkMissingBaseClassInitializer(const ASTContext &Context,
+                                        const CXXRecordDecl &ClassDecl,
                                         const CXXConstructorDecl *Ctor);
 
   // Checks Type.6 part 2: