diff options
3 files changed, 51 insertions, 32 deletions
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp index ea6ebbfb691..4cf7b1600c1 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp @@ -27,16 +27,23 @@ namespace cppcoreguidelines { namespace { -void fieldsRequiringInit(const RecordDecl::field_range &Fields, - ASTContext &Context, - SmallPtrSetImpl<const FieldDecl *> &FieldsToInit) { +// Iterate over all the fields in a record type, both direct and indirect (e.g. +// if the record contains an anonmyous struct). If OneFieldPerUnion is true and +// 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, + bool OneFieldPerUnion, Func &&Fn) { for (const FieldDecl *F : Fields) { - if (F->hasInClassInitializer()) - continue; - QualType Type = F->getType(); - if (!F->hasInClassInitializer() && - utils::type_traits::isTriviallyDefaultConstructible(Type, Context)) - FieldsToInit.insert(F); + if (F->isAnonymousStructOrUnion()) { + if (const CXXRecordDecl *R = F->getType()->getAsCXXRecordDecl()) + forEachField(R, R->fields(), OneFieldPerUnion, Fn); + } else { + Fn(F); + } + + if (OneFieldPerUnion && Record->isUnion()) + break; } } @@ -179,8 +186,8 @@ computeInsertions(const CXXConstructorDecl::init_const_range &Inits, // 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->isAnyMemberInitializer() + ? static_cast<const NamedDecl *>(Init->getAnyMember()) : Init->getBaseClass()->getAsCXXRecordDecl(); // Add all fields between current field up until the next intializer. @@ -216,7 +223,8 @@ void getInitializationsInOrder(const CXXRecordDecl *ClassDecl, Decls.emplace_back(Decl); } } - Decls.append(ClassDecl->fields().begin(), ClassDecl->fields().end()); + forEachField(ClassDecl, ClassDecl->fields(), false, + [&](const FieldDecl *F) { Decls.push_back(F); }); } template <typename T> @@ -238,22 +246,6 @@ void fixInitializerList(const ASTContext &Context, DiagnosticBuilder &Diag, } } -template <typename T, typename Func> -void forEachField(const RecordDecl *Record, const T &Fields, - bool OneFieldPerUnion, Func &&Fn) { - for (const FieldDecl *F : Fields) { - if (F->isAnonymousStructOrUnion()) { - if (const RecordDecl *R = getCanonicalRecordDecl(F->getType())) - forEachField(R, R->fields(), OneFieldPerUnion, Fn); - } else { - Fn(F); - } - - if (OneFieldPerUnion && Record->isUnion()) - break; - } -} - } // anonymous namespace ProTypeMemberInitCheck::ProTypeMemberInitCheck(StringRef Name, @@ -314,8 +306,14 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer( if (IsUnion && ClassDecl->hasInClassInitializer()) return; + // Gather all fields (direct and indirect) that need to be initialized. SmallPtrSet<const FieldDecl *, 16> FieldsToInit; - fieldsRequiringInit(ClassDecl->fields(), Context, FieldsToInit); + forEachField(ClassDecl, ClassDecl->fields(), false, [&](const FieldDecl *F) { + if (!F->hasInClassInitializer() && + utils::type_traits::isTriviallyDefaultConstructible(F->getType(), + Context)) + FieldsToInit.insert(F); + }); if (FieldsToInit.empty()) return; @@ -325,7 +323,7 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer( if (Init->isAnyMemberInitializer() && Init->isWritten()) { if (IsUnion) return; // We can only initialize one member of a union. - FieldsToInit.erase(Init->getMember()); + FieldsToInit.erase(Init->getAnyMember()); } } removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit); @@ -389,8 +387,8 @@ void ProTypeMemberInitCheck::checkMissingBaseClassInitializer( if (const auto *BaseClassDecl = getCanonicalRecordDecl(Base.getType())) { AllBases.emplace_back(BaseClassDecl); if (!BaseClassDecl->field_empty() && - utils::type_traits::isTriviallyDefaultConstructible( - Base.getType(), Context)) + utils::type_traits::isTriviallyDefaultConstructible(Base.getType(), + Context)) BasesToInit.insert(BaseClassDecl); } } diff --git a/clang-tools-extra/test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp b/clang-tools-extra/test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp index 5428f01f085..db97bb45686 100644 --- a/clang-tools-extra/test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp +++ b/clang-tools-extra/test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp @@ -103,3 +103,14 @@ public: int X; }; + +class PositiveIndirectMember { + struct { + int *A; + }; + + PositiveIndirectMember() : A() {} + PositiveIndirectMember(int) {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A + // CHECK-FIXES: PositiveIndirectMember(int) : A() {} +}; 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 26db5f0d912..5596c43d654 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 @@ -357,3 +357,13 @@ class PositiveSelfInitialization : NegativeAggregateType // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NegativeAggregateType // CHECK-FIXES: PositiveSelfInitialization() : NegativeAggregateType(), PositiveSelfInitialization() {} }; + +class PositiveIndirectMember { + struct { + int *A; + // CHECK-FIXES: int *A{}; + }; + + PositiveIndirectMember() {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A +}; |
