[clang-tidy] Fix bugprone-tagged-union-member-count false-positive

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

@@ -110,6 +110,19 @@ 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-unchecked-optional-access
+  <clang-tidy/checks/bugprone/unchecked-optional-access>` fixing false
+  positives from smart pointer accessors repeated in checking ``has_value``
+  and accessing ``value``. The option `IgnoreSmartPointerDereference` should
+  no longer be needed and will be removed. Also fixing false positive from
+  const reference accessors to objects containing optional member.
+
 - 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 a system header files or 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;
+};