=?utf-8?b?R8OhYm9yIFTDs3RodsOhcmk=?=,=?utf-8?b?R8OhYm9yIFTDs3RodsOhcmk=?Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/135...@github.com>
https://github.com/tigbr updated https://github.com/llvm/llvm-project/pull/135831 >From 525459a04dd6e7d0079095ac531c7cd712ac91d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20T=C3=B3thv=C3=A1ri?= <gabor.tothv...@ericsson.com> Date: Mon, 14 Apr 2025 17:09:07 +0200 Subject: [PATCH 1/3] [clang-tidy] Fix bugprone-tagged-union-member-count false-positive 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 #134840 --- .../bugprone/TaggedUnionMemberCountCheck.cpp | 12 ++++++---- .../bugprone/tagged-union-member-count.rst | 22 +++++++++++++++++++ .../bugprone/tagged-union-member-count.c | 13 +++++++++++ .../bugprone/tagged-union-member-count.cpp | 13 +++++++++++ .../bugprone/tagged-union-member-count.m | 13 +++++++++++ .../bugprone/tagged-union-member-count.mm | 13 +++++++++++ 6 files changed, 82 insertions(+), 4 deletions(-) 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/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst index 2f1036c10345e..b47a49543143b 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst @@ -9,6 +9,9 @@ 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. +The union and enum data members that are from system header files or +the std namespace are not considered to make up the tagged union part +of a user-defined tagged union type. Example: @@ -28,6 +31,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 ------------------------------ 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; +}; >From 5981e2aba462be93c8b78e25814e904a0168132f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20T=C3=B3thv=C3=A1ri?= <gabor.tothv...@ericsson.com> Date: Thu, 17 Apr 2025 10:54:23 +0200 Subject: [PATCH 2/3] Update release notes and improve formatting in documentation. --- clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++++ .../checks/bugprone/tagged-union-member-count.rst | 8 ++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 761c1d3a80359..db5fe8c1af751 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -140,6 +140,12 @@ Changes in existing checks calls of ``std::string`` constructor with char pointer, start position and length parameters. +- Improved :doc:`bugprone-tagged-union-member-count + <clang-tidy/checks/bugprone/tagged-union-member-count>` by fixing a false + positive when typedefed 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`` diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst index b47a49543143b..cdfe341f42dba 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/tagged-union-member-count.rst @@ -10,7 +10,7 @@ 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. The union and enum data members that are from system header files or -the std namespace are not considered to make up the tagged union part +the ``std`` namespace are not considered to make up the tagged union part of a user-defined tagged union type. Example: @@ -32,7 +32,7 @@ Example: }; The following example illustrates the exception for unions and enums from -system header files and the std namespace. +system header files and the ``std`` namespace. .. code-block:: c++ @@ -43,11 +43,11 @@ system header files and the std namespace. pthread_mutex_t Mutex; }; -The pthread_mutex_t type may be defined as a union behind a typedef, +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 +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 >From e915153f1f2ed01e467fcf1463016d5482776ac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20T=C3=B3thv=C3=A1ri?= <gabor.tothv...@ericsson.com> Date: Thu, 17 Apr 2025 13:37:07 +0200 Subject: [PATCH 3/3] Properly stub the pthread.h header in the tests. --- .../Inputs/tagged-union-member-count/system/pthread.h | 10 ++++++++++ .../checkers/bugprone/tagged-union-member-count.c | 5 +++-- .../checkers/bugprone/tagged-union-member-count.cpp | 5 +++-- .../checkers/bugprone/tagged-union-member-count.m | 5 +++-- .../checkers/bugprone/tagged-union-member-count.mm | 5 +++-- 5 files changed, 22 insertions(+), 8 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/tagged-union-member-count/system/pthread.h diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/tagged-union-member-count/system/pthread.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/tagged-union-member-count/system/pthread.h new file mode 100644 index 0000000000000..43aa224ebf5eb --- /dev/null +++ b/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; 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 96255c7fdd4fe..79a86cd675cea 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 @@ -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, @@ -150,7 +151,7 @@ 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" +#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. 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 f21c23b87ae44..465667b896e63 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 @@ -1,4 +1,5 @@ -// 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: -isystem %S/Inputs/tagged-union-member-count/system // Test check with C++ features typedef enum Tags3 { @@ -311,7 +312,7 @@ void DoNotMatchLambdas() { // 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" +#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. 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 96255c7fdd4fe..79a86cd675cea 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 @@ -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, @@ -150,7 +151,7 @@ // 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" +#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. 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 b169b5cd480b5..531b10becb2e3 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 @@ -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, @@ -310,7 +311,7 @@ void DoNotMatchLambdas() { // 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" +#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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
