[clang-tidy] Fix bugprone-tagged-union-member-count false-positive #135831
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang-tools-extra Author: None (tigbr) ChangesThis patch implements a fix for the false-positive case described in issue #134840 for the bugprone-tagged-union-member-count clang-tidy check. The example given in the linked issue was the following: #include <pthread.h>
typedef enum {
MYENUM_ONE,
MYENUM_TWO,
} myEnumT;
typedef struct {
pthread_mutex_t mtx;
myEnumT my_enum;
} myTypeT;The bugprone-tagged-union-member-count check emits the following a warning for this struct: The issue is that typedef union
{
struct __pthread_mutex_s __data;
char __size[__SIZEOF_PTHREAD_MUTEX_T];
long int __align;
} pthread_mutex_t;From the checker's point of view therefore The proposed solution is that the types from system headers and the std namespace are no longer candidates to be the enum part or the union part of a user-defined tagged union. This filtering for enums and the std namespace may not be strictly necessary in this example, however I added it preemptively out of (perhaps unnecessary) caution. Full diff: https://github.com/llvm/llvm-project/pull/135831.diff 5 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp
index db99ef3786e5f..b91da7db39463 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp
@@ -106,11 +106,15 @@ void TaggedUnionMemberCountCheck::storeOptions(
void TaggedUnionMemberCountCheck::registerMatchers(MatchFinder *Finder) {
- auto UnionField = fieldDecl(hasType(qualType(
- hasCanonicalType(recordType(hasDeclaration(recordDecl(isUnion())))))));
+ auto NotFromSystemHeaderOrStdNamespace =
+ unless(anyOf(isExpansionInSystemHeader(), isInStdNamespace()));
- auto EnumField = fieldDecl(hasType(
- qualType(hasCanonicalType(enumType(hasDeclaration(enumDecl()))))));
+ auto UnionField =
+ fieldDecl(hasType(qualType(hasCanonicalType(recordType(hasDeclaration(
+ recordDecl(isUnion(), NotFromSystemHeaderOrStdNamespace)))))));
+
+ auto EnumField = fieldDecl(hasType(qualType(hasCanonicalType(
+ enumType(hasDeclaration(enumDecl(NotFromSystemHeaderOrStdNamespace)))))));
auto hasOneUnionField = fieldCountOfKindIsOne(UnionField, UnionMatchBindName);
auto hasOneEnumField = fieldCountOfKindIsOne(EnumField, TagMatchBindName);
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.c
index 60c93c553baca..96255c7fdd4fe 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.c
@@ -147,3 +147,16 @@ struct Name {\
// CHECK-MESSAGES: :[[@LINE+1]]:44: warning: tagged union has more data members (4) than tags (3)
DECLARE_TAGGED_UNION_STRUCT(Tags3, Union4, TaggedUnionStructFromMacro);
+
+// Typedefed unions from system header files should be ignored when
+// we are trying to pinpoint the union part in a user-defined tagged union.
+#include "pthread.h"
+
+// This should not be analyzed as a user-defined tagged union,
+// even though pthread_mutex_t may be declared as a typedefed union.
+struct SystemTypedefedUnionDataMemberShouldBeIgnored {
+ pthread_mutex_t Mutex;
+ enum {
+ MyEnum
+ } EnumField;
+};
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.cpp
index 25827e8c8de0c..f21c23b87ae44 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.cpp
@@ -308,3 +308,16 @@ void DoNotMatchLambdas() {
} u;
auto L = [e, u] () {};
}
+
+// Typedefed unions from system header files should be ignored when
+// we are trying to pinpoint the union part in a user-defined tagged union.
+#include "pthread.h"
+
+// This should not be analyzed as a user-defined tagged union,
+// even though pthread_mutex_t may be declared as a typedefed union.
+struct SystemTypedefedUnionDataMemberShouldBeIgnored {
+ pthread_mutex_t Mutex;
+ enum {
+ MyEnum
+ } EnumField;
+};
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.m b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.m
index 60c93c553baca..96255c7fdd4fe 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.m
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.m
@@ -147,3 +147,16 @@
// CHECK-MESSAGES: :[[@LINE+1]]:44: warning: tagged union has more data members (4) than tags (3)
DECLARE_TAGGED_UNION_STRUCT(Tags3, Union4, TaggedUnionStructFromMacro);
+
+// Typedefed unions from system header files should be ignored when
+// we are trying to pinpoint the union part in a user-defined tagged union.
+#include "pthread.h"
+
+// This should not be analyzed as a user-defined tagged union,
+// even though pthread_mutex_t may be declared as a typedefed union.
+struct SystemTypedefedUnionDataMemberShouldBeIgnored {
+ pthread_mutex_t Mutex;
+ enum {
+ MyEnum
+ } EnumField;
+};
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.mm b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.mm
index 8b308555281c5..b169b5cd480b5 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.mm
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.mm
@@ -307,3 +307,16 @@ void DoNotMatchLambdas() {
} u;
auto L = [e, u] () {};
}
+
+// Typedefed unions from system header files should be ignored when
+// we are trying to pinpoint the union part in a user-defined tagged union.
+#include "pthread.h"
+
+// This should not be analyzed as a user-defined tagged union,
+// even though pthread_mutex_t may be declared as a typedefed union.
+struct SystemTypedefedUnionDataMemberShouldBeIgnored {
+ pthread_mutex_t Mutex;
+ enum {
+ MyEnum
+ } EnumField;
+};
|
|
@llvm/pr-subscribers-clang-tidy Author: None (tigbr) ChangesThis patch implements a fix for the false-positive case described in issue #134840 for the bugprone-tagged-union-member-count clang-tidy check. The example given in the linked issue was the following: #include <pthread.h>
typedef enum {
MYENUM_ONE,
MYENUM_TWO,
} myEnumT;
typedef struct {
pthread_mutex_t mtx;
myEnumT my_enum;
} myTypeT;The bugprone-tagged-union-member-count check emits the following a warning for this struct: The issue is that typedef union
{
struct __pthread_mutex_s __data;
char __size[__SIZEOF_PTHREAD_MUTEX_T];
long int __align;
} pthread_mutex_t;From the checker's point of view therefore The proposed solution is that the types from system headers and the std namespace are no longer candidates to be the enum part or the union part of a user-defined tagged union. This filtering for enums and the std namespace may not be strictly necessary in this example, however I added it preemptively out of (perhaps unnecessary) caution. Full diff: https://github.com/llvm/llvm-project/pull/135831.diff 5 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp
index db99ef3786e5f..b91da7db39463 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp
@@ -106,11 +106,15 @@ void TaggedUnionMemberCountCheck::storeOptions(
void TaggedUnionMemberCountCheck::registerMatchers(MatchFinder *Finder) {
- auto UnionField = fieldDecl(hasType(qualType(
- hasCanonicalType(recordType(hasDeclaration(recordDecl(isUnion())))))));
+ auto NotFromSystemHeaderOrStdNamespace =
+ unless(anyOf(isExpansionInSystemHeader(), isInStdNamespace()));
- auto EnumField = fieldDecl(hasType(
- qualType(hasCanonicalType(enumType(hasDeclaration(enumDecl()))))));
+ auto UnionField =
+ fieldDecl(hasType(qualType(hasCanonicalType(recordType(hasDeclaration(
+ recordDecl(isUnion(), NotFromSystemHeaderOrStdNamespace)))))));
+
+ auto EnumField = fieldDecl(hasType(qualType(hasCanonicalType(
+ enumType(hasDeclaration(enumDecl(NotFromSystemHeaderOrStdNamespace)))))));
auto hasOneUnionField = fieldCountOfKindIsOne(UnionField, UnionMatchBindName);
auto hasOneEnumField = fieldCountOfKindIsOne(EnumField, TagMatchBindName);
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.c
index 60c93c553baca..96255c7fdd4fe 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.c
@@ -147,3 +147,16 @@ struct Name {\
// CHECK-MESSAGES: :[[@LINE+1]]:44: warning: tagged union has more data members (4) than tags (3)
DECLARE_TAGGED_UNION_STRUCT(Tags3, Union4, TaggedUnionStructFromMacro);
+
+// Typedefed unions from system header files should be ignored when
+// we are trying to pinpoint the union part in a user-defined tagged union.
+#include "pthread.h"
+
+// This should not be analyzed as a user-defined tagged union,
+// even though pthread_mutex_t may be declared as a typedefed union.
+struct SystemTypedefedUnionDataMemberShouldBeIgnored {
+ pthread_mutex_t Mutex;
+ enum {
+ MyEnum
+ } EnumField;
+};
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.cpp
index 25827e8c8de0c..f21c23b87ae44 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.cpp
@@ -308,3 +308,16 @@ void DoNotMatchLambdas() {
} u;
auto L = [e, u] () {};
}
+
+// Typedefed unions from system header files should be ignored when
+// we are trying to pinpoint the union part in a user-defined tagged union.
+#include "pthread.h"
+
+// This should not be analyzed as a user-defined tagged union,
+// even though pthread_mutex_t may be declared as a typedefed union.
+struct SystemTypedefedUnionDataMemberShouldBeIgnored {
+ pthread_mutex_t Mutex;
+ enum {
+ MyEnum
+ } EnumField;
+};
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.m b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.m
index 60c93c553baca..96255c7fdd4fe 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.m
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.m
@@ -147,3 +147,16 @@
// CHECK-MESSAGES: :[[@LINE+1]]:44: warning: tagged union has more data members (4) than tags (3)
DECLARE_TAGGED_UNION_STRUCT(Tags3, Union4, TaggedUnionStructFromMacro);
+
+// Typedefed unions from system header files should be ignored when
+// we are trying to pinpoint the union part in a user-defined tagged union.
+#include "pthread.h"
+
+// This should not be analyzed as a user-defined tagged union,
+// even though pthread_mutex_t may be declared as a typedefed union.
+struct SystemTypedefedUnionDataMemberShouldBeIgnored {
+ pthread_mutex_t Mutex;
+ enum {
+ MyEnum
+ } EnumField;
+};
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.mm b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.mm
index 8b308555281c5..b169b5cd480b5 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.mm
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.mm
@@ -307,3 +307,16 @@ void DoNotMatchLambdas() {
} u;
auto L = [e, u] () {};
}
+
+// Typedefed unions from system header files should be ignored when
+// we are trying to pinpoint the union part in a user-defined tagged union.
+#include "pthread.h"
+
+// This should not be analyzed as a user-defined tagged union,
+// even though pthread_mutex_t may be declared as a typedefed union.
+struct SystemTypedefedUnionDataMemberShouldBeIgnored {
+ pthread_mutex_t Mutex;
+ enum {
+ MyEnum
+ } EnumField;
+};
|
Types from system headers and the std namespace are no longer considered as the enum part or the union part of a user-defined tagged union. Fixes llvm#134840
tigbr
force-pushed
the
tagged-union-member-count-fix
branch
from
d69cd61 to
525459a
Compare
vbvictor
reviewed
Apr 16, 2025
clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.c
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst
Outdated
Show resolved
Hide resolved
|
Please mention changes in Release Notes. |
|
I'm not sure if we need to check explicitly if some |
…mespace but not a system header.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch implements a fix for the false-positive case described in issue #134840 for the bugprone-tagged-union-member-count clang-tidy check.
The example given in the linked issue was the following:
The bugprone-tagged-union-member-count check emits the following a warning for this struct:
The issue is that
pthread_mutex_tcan be a union behind a typedef like the following:From the checker's point of view therefore
myTypeTcontains a data member with an enum type and another data member with a union type and starts analyzing it like a user-defined tagged union.The proposed solution is that the types from system headers and the std namespace are no longer candidates to be the enum part or the union part of a user-defined tagged union. This filtering for enums and the std namespace may not be strictly necessary in this example, however I added it preemptively out of (perhaps unnecessary) caution.