Sockke updated this revision to Diff 374430. Sockke added a comment. Update!
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108370/new/ https://reviews.llvm.org/D108370 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp +++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp @@ -516,3 +516,39 @@ PositiveDefaultConstructorOutOfDecl::PositiveDefaultConstructorOutOfDecl() = default; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these fields: F + +union U1 { + U1() {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: union constructor should initialize one of these fields: X, K, Z, Y + int X; + // CHECK-FIXES: int X{}; + union { + int K; + // CHECK-FIXES-NOT: int K{}; + }; + union { + int Z; + // CHECK-FIXES-NOT: int Z{}; + int Y; + // CHECK-FIXES-NOT: int Y{}; + }; +}; + +union U2 { + U2() {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: union constructor should initialize one of these fields: B, C, A + struct { + int B; + // CHECK-FIXES: int B{}; + union { + struct { + PositiveMultipleConstructors Value; + // CHECK-FIXES-NOT: PositiveMultipleConstructors Value{}; + }; + int C; + // CHECK-FIXES: int C{}; + }; + }; + int A; + // CHECK-FIXES-NOT: int A{}; +}; Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp +++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp @@ -44,6 +44,23 @@ } } +template <typename T, typename Func> +void forEachFieldWithFilter(const RecordDecl &Record, const T &Fields, + bool &AnyMemberHasInitPerUnion, Func &&Fn) { + for (const FieldDecl *F : Fields) { + if (F->isAnonymousStructOrUnion()) { + if (const CXXRecordDecl *R = F->getType()->getAsCXXRecordDecl()) { + AnyMemberHasInitPerUnion = false; + forEachFieldWithFilter(*R, R->fields(), AnyMemberHasInitPerUnion, Fn); + } + } else { + Fn(F); + } + if (Record.isUnion() && AnyMemberHasInitPerUnion) + break; + } +} + void removeFieldsInitializedInBody( const Stmt &Stmt, ASTContext &Context, SmallPtrSetImpl<const FieldDecl *> &FieldDecls) { @@ -461,8 +478,9 @@ // Collect all fields but only suggest a fix for the first member of unions, // as initializing more than one union member is an error. SmallPtrSet<const FieldDecl *, 16> FieldsToFix; - SmallPtrSet<const RecordDecl *, 4> UnionsSeen; - forEachField(ClassDecl, OrderedFields, [&](const FieldDecl *F) { + bool AnyMemberHasInitPerUnion = false; + forEachFieldWithFilter(ClassDecl, ClassDecl.fields(), + AnyMemberHasInitPerUnion, [&](const FieldDecl *F) { if (!FieldsToInit.count(F)) return; // Don't suggest fixes for enums because we don't know a good default. @@ -471,8 +489,8 @@ if (F->getType()->isEnumeralType() || (!getLangOpts().CPlusPlus20 && F->isBitField())) return; - if (!F->getParent()->isUnion() || UnionsSeen.insert(F->getParent()).second) - FieldsToFix.insert(F); + FieldsToFix.insert(F); + AnyMemberHasInitPerUnion = true; }); if (FieldsToFix.empty()) return;
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp +++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp @@ -516,3 +516,39 @@ PositiveDefaultConstructorOutOfDecl::PositiveDefaultConstructorOutOfDecl() = default; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these fields: F + +union U1 { + U1() {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: union constructor should initialize one of these fields: X, K, Z, Y + int X; + // CHECK-FIXES: int X{}; + union { + int K; + // CHECK-FIXES-NOT: int K{}; + }; + union { + int Z; + // CHECK-FIXES-NOT: int Z{}; + int Y; + // CHECK-FIXES-NOT: int Y{}; + }; +}; + +union U2 { + U2() {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: union constructor should initialize one of these fields: B, C, A + struct { + int B; + // CHECK-FIXES: int B{}; + union { + struct { + PositiveMultipleConstructors Value; + // CHECK-FIXES-NOT: PositiveMultipleConstructors Value{}; + }; + int C; + // CHECK-FIXES: int C{}; + }; + }; + int A; + // CHECK-FIXES-NOT: int A{}; +}; Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp +++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp @@ -44,6 +44,23 @@ } } +template <typename T, typename Func> +void forEachFieldWithFilter(const RecordDecl &Record, const T &Fields, + bool &AnyMemberHasInitPerUnion, Func &&Fn) { + for (const FieldDecl *F : Fields) { + if (F->isAnonymousStructOrUnion()) { + if (const CXXRecordDecl *R = F->getType()->getAsCXXRecordDecl()) { + AnyMemberHasInitPerUnion = false; + forEachFieldWithFilter(*R, R->fields(), AnyMemberHasInitPerUnion, Fn); + } + } else { + Fn(F); + } + if (Record.isUnion() && AnyMemberHasInitPerUnion) + break; + } +} + void removeFieldsInitializedInBody( const Stmt &Stmt, ASTContext &Context, SmallPtrSetImpl<const FieldDecl *> &FieldDecls) { @@ -461,8 +478,9 @@ // Collect all fields but only suggest a fix for the first member of unions, // as initializing more than one union member is an error. SmallPtrSet<const FieldDecl *, 16> FieldsToFix; - SmallPtrSet<const RecordDecl *, 4> UnionsSeen; - forEachField(ClassDecl, OrderedFields, [&](const FieldDecl *F) { + bool AnyMemberHasInitPerUnion = false; + forEachFieldWithFilter(ClassDecl, ClassDecl.fields(), + AnyMemberHasInitPerUnion, [&](const FieldDecl *F) { if (!FieldsToInit.count(F)) return; // Don't suggest fixes for enums because we don't know a good default. @@ -471,8 +489,8 @@ if (F->getType()->isEnumeralType() || (!getLangOpts().CPlusPlus20 && F->isBitField())) return; - if (!F->getParent()->isUnion() || UnionsSeen.insert(F->getParent()).second) - FieldsToFix.insert(F); + FieldsToFix.insert(F); + AnyMemberHasInitPerUnion = true; }); if (FieldsToFix.empty()) return;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits