Author: liuke Date: 2021-09-24T13:15:21-04:00 New Revision: e4902480f1e2f12f73c2b504e3d717536653dd7b
URL: https://github.com/llvm/llvm-project/commit/e4902480f1e2f12f73c2b504e3d717536653dd7b DIFF: https://github.com/llvm/llvm-project/commit/e4902480f1e2f12f73c2b504e3d717536653dd7b.diff LOG: Fix wrong FixIt about union in cppcoreguidelines-pro-type-member-init At most one variant member of a union may have a default member initializer. The case of anonymous records with multiple levels of nesting like the following also needs to meet this rule. The original logic is to horizontally obtain all the member variables in a record that need to be initialized and then filter to the variables that need to be fixed. Obviously, it is impossible to correctly initialize the desired variables according to the nesting relationship. See Example 3 in class.union union U { U() {} int x; // int x{}; union { int k; // int k{}; <== wrong fix }; union { int z; // int z{}; <== wrong fix int y; }; }; Added: Modified: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp index a191598415217..0c220e865ff48 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp @@ -44,6 +44,23 @@ void forEachField(const RecordDecl &Record, const T &Fields, Func &&Fn) { } } +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 @@ 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<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 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer( 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; 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 8cab4fd755752..677f3a100a16f 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 @@ -516,3 +516,39 @@ struct PositiveDefaultConstructorOutOfDecl { 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{}; +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
