[clang-tidy] Fix bugprone-tagged-union-member-count false-positive (#135831)

This patch implements a fix for the false-positive case described in
[issue #134840](#134840) for
the
[bugprone-tagged-union-member-count](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/tagged-union-member-count.html)
clang-tidy check.

The example given in the linked issue was the following:

```C
#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](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/tagged-union-member-count.html)
check emits the following a warning for this struct:

```
<source>:8:9: warning: tagged union has more data members (3) than tags (2)! [bugprone-tagged-union-member-count]
    8 | typedef struct {
      |         ^
1 warning generated.
```

The issue is that `pthread_mutex_t` can be a union behind a typedef like
the following:

```C
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 `myTypeT` contains 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.

Fixes #134840.

Author: tigbr

clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp

@@ -105,11 +105,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);

clang-tools-extra/docs/ReleaseNotes.rst

@@ -142,6 +142,12 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/signed-char-misuse>` check by fixing
   false positives on C23 enums with the fixed underlying type of signed char.
 
+- Improved :doc:`bugprone-tagged-union-member-count
+  <clang-tidy/checks/bugprone/tagged-union-member-count>` by fixing a false
+  positive when enums or unions from system header files or the ``std``
+  namespace are treated as the tag or the data part of a user-defined
+  tagged union respectively.
+
 - Improved :doc:`bugprone-unhandled-self-assignment
   <clang-tidy/checks/bugprone/unhandled-self-assignment>` check by adding
   an additional matcher that generalizes the copy-and-swap idiom pattern

clang-tools-extra/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst

@@ -9,6 +9,8 @@ different from the number of data members inside the union.
 A struct or a class is considered to be a tagged union if it has
 exactly one union data member and exactly one enum data member and
 any number of other data members that are neither unions or enums.
+Furthermore, the types of the union and the enum members must 
+not come from system header files nor the ``std`` namespace.
 
 Example:
 
@@ -28,6 +30,25 @@ Example:
     } Data;
   };
 
+The following example illustrates the exception for unions and enums from
+system header files and the ``std`` namespace.
+
+.. code-block:: c++
+
+  #include <pthread.h>
+
+  struct NotTaggedUnion {
+    enum MyEnum { MyEnumConstant1, MyEnumConstant2 } En;
+    pthread_mutex_t Mutex;
+  };
+
+The ``pthread_mutex_t`` type may be defined as a union behind a ``typedef``,
+in which case the check could mistake this type as a user-defined tagged union.
+After all, it has exactly one enum data member and exactly one union data member.
+To avoid false-positive cases originating from this, unions and enums from
+system headers and the ``std`` namespace are ignored when pinpointing the
+union part and the enum part of a potential user-defined tagged union.
+
 How enum constants are counted
 ------------------------------
 

clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/tagged-union-member-count/stdnamespace.h

@@ -0,0 +1,12 @@
+#define __SIZEOF_PTHREAD_MUTEX_T 40
+
+namespace std {
+  typedef union {
+    struct __pthread_mutex_s {
+      int __lock;
+      unsigned int __count;
+    } __data;
+    char __size[__SIZEOF_PTHREAD_MUTEX_T];
+    long int __align;
+  } pthread_mutex_t;
+};

clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/tagged-union-member-count/system/pthread.h

@@ -0,0 +1,10 @@
+#define __SIZEOF_PTHREAD_MUTEX_T 40
+
+typedef union {
+  struct __pthread_mutex_s {
+    int __lock;
+    unsigned int __count;
+  } __data;
+  char __size[__SIZEOF_PTHREAD_MUTEX_T];
+  long int __align;
+} pthread_mutex_t;

clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.c

@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s bugprone-tagged-union-member-count %t
+// RUN: %check_clang_tidy %s bugprone-tagged-union-member-count %t -- -- \
+// RUN: -isystem %S/Inputs/tagged-union-member-count/system
 
 typedef enum Tags3 {
   tags3_1,
@@ -147,3 +148,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);
+
+// 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 could be a union.
+struct SystemTypedefedUnionDataMemberShouldBeIgnored {
+  pthread_mutex_t Mutex;
+  enum {
+    MyEnum
+  } EnumField;
+};

clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.cpp

@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-tagged-union-member-count %t
+// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-tagged-union-member-count %t -- -- \
+// RUN: -I%S/Inputs/tagged-union-member-count \
+// RUN: -isystem %S/Inputs/tagged-union-member-count/system
 // Test check with C++ features
 
 typedef enum Tags3 {
@@ -308,3 +310,26 @@ 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;
+};
+
+// Filter when union or enum comes from the std namespace but not a system header
+#include "stdnamespace.h"
+
+struct StdNameSpaceUnionDataMemberShouldBeIgnored {
+  std::pthread_mutex_t Mutex;
+  enum {
+    MyEnum
+  } EnumField;
+};

clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.m

@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s bugprone-tagged-union-member-count %t
+// RUN: %check_clang_tidy %s bugprone-tagged-union-member-count %t -- -- \
+// RUN: -isystem %S/Inputs/tagged-union-member-count/system
 
 typedef enum Tags3 {
   tags3_1,
@@ -147,3 +148,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;
+};

clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.mm

@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s bugprone-tagged-union-member-count %t
+// RUN: %check_clang_tidy %s bugprone-tagged-union-member-count %t -- -- \
+// RUN: -isystem %S/Inputs/tagged-union-member-count/system
 
 typedef enum Tags3 {
   tags3_1,
@@ -307,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;
+};