Sockke created this revision.
Sockke added reviewers: aaron.ballman, bkramer, alexfh, malcolm.parsons, MTC,
steven.zhang.
Herald added subscribers: shchenz, kbarton, xazax.hun, nemanjai.
Sockke requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
I can only initialize one member variable of the same level in the union. 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 <http://eel.is/c++draft/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;
};
};
Repository:
rG LLVM Github Monorepo
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,34 @@
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;
+ };
+ union {
+ int Z;
+ 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;
+ };
+ int C;
+ // CHECK-FIXES: int C{};
+ };
+ };
+ 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,34 @@
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;
+ };
+ union {
+ int Z;
+ 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;
+ };
+ int C;
+ // CHECK-FIXES: int C{};
+ };
+ };
+ 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
