Fix a crash in cppcoreguidelines-pro-type-member-init when checking a class that initializes itself as a base

[michael_miller]

Summary

Fix a crash when a record type initializes itself in its own base class initializer list.

[aaron.ballman]

test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

342

Is it required to be a templated class to trigger the crash?

[hokein]

LGTM. Thanks!

test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

342

Yes, it's required.

[aaron.ballman]

LGTM with a commenting request.

test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

342

Then some comments in this test would be good, I would assume otherwise from just looking at the test.

[michael_miller]

test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

342

It's a little bit of a pathological example. Without the template, the code won't compile at all due to an error. The check doesn't crash in that case (error: constructor for 'PositiveSelfInitialization' creates a delegation cycle [clang-diagnostic-delegating-ctor-cycles]). With the template in the mix, apparently PositiveSelfInitialization doesn't have a valid RecordDecl. If you instantiate the template, though, the code again fails to compile.

[michael_miller]

Added a comment explaining the new test added.

[michael_miller]

test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

342

Added a comment explaining why the check is there.

[aaron.ballman]

test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

342

Thank you, the comment is excellent.

clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

if (Init->isWritten()) {
       if (Insertions.size() == 1)
         Insertions.emplace_back(InitializerPlacement::Before, Init);
 
       // Gets either the field or base class being initialized by the provided
       // initializer.
       const auto *InitDecl =
           Init->isMemberInitializer()
               ? static_cast<const NamedDecl *>(Init->getMember())
-              : Init->getBaseClass()->getAs<RecordType>()->getDecl();
+              : Init->getBaseClass()->getAsCXXRecordDecl();
 
       // Add all fields between current field up until the next intializer.
       for (; Decl != std::end(OrderedDecls) && *Decl != InitDecl; ++Decl) {
         if (const T *D = dyn_cast<T>(*Decl)) {
           if (DeclsToInit.count(D) > 0)
             Insertions.back().Initializers.emplace_back(getName(D));
         }
       }
void ProTypeMemberInitCheck::checkMissingBaseClassInitializer(
   }
 
   if (BasesToInit.empty())
     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()->getAs<RecordType>()->getDecl());
+      BasesToInit.erase(Init->getBaseClass()->getAsCXXRecordDecl());
   }
 
   if (BasesToInit.empty())
     return;
 
   DiagnosticBuilder Diag =
       diag(Ctor->getLocStart(),
            "constructor does not initialize these bases: %0")

test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

 };
 
 // This check results in a CXXConstructorDecl with no body.
 struct NegativeDeletedConstructor : NegativeAggregateType {
   NegativeDeletedConstructor() = delete;
 
   Template<int> F;
 };
+
+template <typename T>
+class PositiveSelfInitialization : NegativeAggregateType
+{
+  PositiveSelfInitialization() : PositiveSelfInitialization() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NegativeAggregateType
+  // CHECK-FIXES: PositiveSelfInitialization() : NegativeAggregateType(), PositiveSelfInitialization() {}
+};