Author: hokein Date: Tue May 10 02:42:19 2016 New Revision: 269024 URL: http://llvm.org/viewvc/llvm-project?rev=269024&view=rev Log: Fixed cppcoreguidelines-pro-type-member-init when checking records with indirect fields
Summary: Fixed a crash in cppcoreguidelines-pro-type-member-init when checking record types with indirect fields pre-C++11. Fixed handling of indirect fields so they are properly checked and suggested fixes are proposed. Patch by Michael Miller! Reviewers: aaron.ballman, alexfh, hokein Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D19993 Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp?rev=269024&r1=269023&r2=269024&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp Tue May 10 02:42:19 2016 @@ -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 CXXConstructorDe // 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 CXX 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 } } -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::checkMissin 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::checkMissin 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::checkMissin 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); } } Modified: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp?rev=269024&r1=269023&r2=269024&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp Tue May 10 02:42:19 2016 @@ -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() {} +}; Modified: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp?rev=269024&r1=269023&r2=269024&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp Tue May 10 02:42:19 2016 @@ -357,3 +357,13 @@ class PositiveSelfInitialization : Negat // 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 +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
