diff options
4 files changed, 113 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: diff --git a/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp b/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp index a0e40fe7954..03b05d19632 100644 --- a/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp +++ b/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp @@ -58,6 +58,9 @@ bool recordIsTriviallyDefaultConstructible(const RecordDecl &RecordDecl, // 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 @@ bool recordIsTriviallyDefaultConstructible(const RecordDecl &RecordDecl, for (const CXXBaseSpecifier &Base : ClassDecl->bases()) { if (!isTriviallyDefaultConstructible(Base.getType(), Context)) return false; + if (Base.isVirtual()) + return false; } return true; diff --git a/clang-tools-extra/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp b/clang-tools-extra/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp index a5024d030f1..97146dc29e4 100644 --- a/clang-tools-extra/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp +++ b/clang-tools-extra/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp @@ -377,3 +377,49 @@ void Bug30487() { 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 <typename T> +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<int>; + +#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 |
