From 4e1aafdefe135ccea555d7fef19470a14d6f27ba Mon Sep 17 00:00:00 2001 From: wangliushuai Date: Wed, 8 Jun 2022 20:25:08 +0800 Subject: [PATCH] [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init If a union member is initialized, other members are still recorded in the container to be initialized. This patch fixes this behavior. Reference issue: https://github.com/llvm/llvm-project/issues/54748 Differential Revision: https://reviews.llvm.org/D127293 --- .../ProTypeMemberInitCheck.cpp | 26 +++++++++++++++---- ...cppcoreguidelines-pro-type-member-init.cpp | 18 +++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp index 1540a451b46df..c325ce2cddd99 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp @@ -61,6 +61,17 @@ void forEachFieldWithFilter(const RecordDecl &Record, const T &Fields, } } +void removeFieldInitialized(const FieldDecl *M, + SmallPtrSetImpl &FieldDecls) { + const RecordDecl *R = M->getParent(); + if (R && R->isUnion()) { + // Erase all members in a union if any member of it is initialized. + for (const auto *F : R->fields()) + FieldDecls.erase(F); + } else + FieldDecls.erase(M); +} + void removeFieldsInitializedInBody( const Stmt &Stmt, ASTContext &Context, SmallPtrSetImpl &FieldDecls) { @@ -70,7 +81,7 @@ void removeFieldsInitializedInBody( hasLHS(memberExpr(member(fieldDecl().bind("fieldDecl")))))), Stmt, Context); for (const auto &Match : Matches) - FieldDecls.erase(Match.getNodeAs("fieldDecl")); + removeFieldInitialized(Match.getNodeAs("fieldDecl"), FieldDecls); } StringRef getName(const FieldDecl *Field) { return Field->getName(); } @@ -418,13 +429,18 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer( // Gather all fields (direct and indirect) that need to be initialized. SmallPtrSet FieldsToInit; - forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) { + bool AnyMemberHasInitPerUnion = false; + forEachFieldWithFilter(ClassDecl, ClassDecl.fields(), + AnyMemberHasInitPerUnion, [&](const FieldDecl *F) { if (IgnoreArrays && F->getType()->isArrayType()) return; + if (F->hasInClassInitializer() && F->getParent()->isUnion()) + AnyMemberHasInitPerUnion = true; if (!F->hasInClassInitializer() && utils::type_traits::isTriviallyDefaultConstructible(F->getType(), Context) && - !isEmpty(Context, F->getType()) && !F->isUnnamedBitfield()) + !isEmpty(Context, F->getType()) && !F->isUnnamedBitfield() && + !AnyMemberHasInitPerUnion) FieldsToInit.insert(F); }); if (FieldsToInit.empty()) @@ -437,7 +453,7 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer( if (Init->isAnyMemberInitializer() && Init->isWritten()) { if (IsUnion) return; // We can only initialize one member of a union. - FieldsToInit.erase(Init->getAnyMember()); + removeFieldInitialized(Init->getAnyMember(), FieldsToInit); } } removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit); @@ -478,7 +494,7 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer( // 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 FieldsToFix; - bool AnyMemberHasInitPerUnion = false; + AnyMemberHasInitPerUnion = false; forEachFieldWithFilter(ClassDecl, ClassDecl.fields(), AnyMemberHasInitPerUnion, [&](const FieldDecl *F) { if (!FieldsToInit.count(F)) diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp index 677f3a100a16f..5b8698b49a5c8 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp @@ -552,3 +552,21 @@ union U2 { int A; // CHECK-FIXES-NOT: int A{}; }; + +struct S1 { + S1() {} + // CHECK-MESSAGES-NOT: warning: + union { + int a = 0; + int b; + }; +}; + +struct S2 { + S2() : a{} {} + // CHECK-MESSAGES-NOT: warning: + union { + int a; + int b; + }; +};