[clang-tools-extra] 2d8e91a - [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init
via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 25 03:49:03 PDT 2022
Author: Sockke
Date: 2022-08-25T18:47:38+08:00
New Revision: 2d8e91a5d3597461b5ae2a40d9362abde67b5e1f
URL: https://github.com/llvm/llvm-project/commit/2d8e91a5d3597461b5ae2a40d9362abde67b5e1f
DIFF: https://github.com/llvm/llvm-project/commit/2d8e91a5d3597461b5ae2a40d9362abde67b5e1f.diff
LOG: [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, the other members of the union are still suggested to be initialized in this check. This patch fixes this behavior.
Reference issue: https://github.com/llvm/llvm-project/issues/54748
Reviewed By: njames93
Differential Revision: https://reviews.llvm.org/D127293
Added:
Modified:
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
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 1540a451b46df..975b51cb3bfc5 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<const FieldDecl *> &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<const FieldDecl *> &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>("fieldDecl"));
+ removeFieldInitialized(Match.getNodeAs<FieldDecl>("fieldDecl"), FieldDecls);
}
StringRef getName(const FieldDecl *Field) { return Field->getName(); }
@@ -418,13 +429,20 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer(
// Gather all fields (direct and indirect) that need to be initialized.
SmallPtrSet<const FieldDecl *, 16> 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;
+ removeFieldInitialized(F, FieldsToInit);
+ }
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 +455,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 +496,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<const FieldDecl *, 16> FieldsToFix;
- bool AnyMemberHasInitPerUnion = false;
+ AnyMemberHasInitPerUnion = false;
forEachFieldWithFilter(ClassDecl, ClassDecl.fields(),
AnyMemberHasInitPerUnion, [&](const FieldDecl *F) {
if (!FieldsToInit.count(F))
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index fbe47316d23ae..1656debda1368 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,11 @@ Changes in existing checks
<clang-tidy/checks/bugprone/signal-handler>` check. Partial
support for C++14 signal handler rules was added. Bug report generation was
improved.
+
+- Fixed a false positive in :doc:`cppcoreguidelines-pro-type-member-init
+ <clang-tidy/checks/cppcoreguidelines/pro-type-member-init>` when warnings
+ would be emitted for uninitialized members of an anonymous union despite
+ there being an initializer for one of the other members.
- Improved `modernize-use-emplace <clang-tidy/checks/modernize/use-emplace.html>`_ check.
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..2e2097bae361a 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,30 @@ union U2 {
int A;
// CHECK-FIXES-NOT: int A{};
};
+
+struct S1 {
+ S1() {}
+ union {
+ int a = 0;
+ int b;
+ };
+};
+
+struct S2 {
+ S2() : a{} {}
+ union {
+ int a;
+ int b;
+ };
+};
+
+struct S3 {
+ S3() {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A [cppcoreguidelines-pro-type-member-init]
+ int A;
+ // CHECK-FIXES: int A{};
+ union {
+ int B;
+ int C = 0;
+ };
+};
More information about the cfe-commits
mailing list