Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h =================================================================== --- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h +++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h @@ -46,11 +46,13 @@ // 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: Index: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp =================================================================== --- clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp +++ clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp @@ -32,17 +32,17 @@ // the record type (or indirect field) is a union, forEachField will stop after // the first field. template -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 @@ // 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 &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 @@ return; SmallVector OrderedDecls; - getInitializationsInOrder(Ctor->getParent(), OrderedDecls); + getInitializationsInOrder(*Ctor->getParent(), OrderedDecls); for (const auto &Insertion : computeInsertions(Ctor->inits(), OrderedDecls, DeclsToInit)) { @@ -269,6 +269,19 @@ 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 @@ // 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("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("var")) { checkUninitializedTrivialType(*Result.Context, Var); } @@ -299,16 +318,16 @@ } 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 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 @@ 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 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 @@ 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 @@ 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 AllBases; SmallPtrSet 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 @@ 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( Index: clang-tidy/utils/TypeTraits.cpp =================================================================== --- clang-tidy/utils/TypeTraits.cpp +++ clang-tidy/utils/TypeTraits.cpp @@ -58,6 +58,9 @@ // constructible. if (ClassDecl->hasUserProvidedDefaultConstructor()) return false; + // A polymorphic class is not trivially constructible + if (ClassDecl->isPolymorphic()) + return false; // A class is trivially constructible if it has a trivial default constructor. if (ClassDecl->hasTrivialDefaultConstructor()) return true; @@ -73,6 +76,8 @@ for (const CXXBaseSpecifier &Base : ClassDecl->bases()) { if (!isTriviallyDefaultConstructible(Base.getType(), Context)) return false; + if (Base.isVirtual()) + return false; } return true; Index: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp =================================================================== --- test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp +++ test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp @@ -377,3 +377,49 @@ { NegativeInClassInitializedDefaulted s; } + +struct PositiveVirtualMethod { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: constructor does not initialize these fields: F + int F; + // CHECK-FIXES: int F{}; + virtual int f() = 0; +}; + +struct PositiveVirtualDestructor { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: constructor does not initialize these fields: F + PositiveVirtualDestructor() = default; + int F; + // CHECK-FIXES: int F{}; + virtual ~PositiveVirtualDestructor() {} +}; + +struct PositiveVirtualBase : public virtual NegativeAggregateType { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: constructor does not initialize these bases: NegativeAggregateType + // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: constructor does not initialize these fields: F + int F; + // CHECK-FIXES: int F{}; +}; + +template +struct PositiveTemplateVirtualDestructor { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: constructor does not initialize these fields: F + T Val; + int F; + // CHECK-FIXES: int F{}; + virtual ~PositiveTemplateVirtualDestructor() = default; +}; + +template struct PositiveTemplateVirtualDestructor; + +#define UNINITIALIZED_FIELD_IN_MACRO_BODY_VIRTUAL(FIELD) \ + struct UninitializedFieldVirtual##FIELD { \ + int FIELD; \ + virtual ~UninitializedFieldVirtual##FIELD() {} \ + }; \ +// Ensure FIELD is not initialized since fixes inside of macros are disabled. +// CHECK-FIXES: int FIELD; + +UNINITIALIZED_FIELD_IN_MACRO_BODY_VIRTUAL(F); +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these fields: F +UNINITIALIZED_FIELD_IN_MACRO_BODY_VIRTUAL(G); +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these fields: G