diff --git a/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp index c1ea63cda5003..ddbb14e3ac62b 100644 --- a/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp +++ b/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); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index edab04730cb25..d32bf11120c12 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -110,6 +110,12 @@ Changes in existing checks ` check by fixing false positives on C23 enums with the fixed underlying type of signed char. +- Improved :doc:`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 ` check by adding an additional matcher that generalizes the copy-and-swap idiom pattern 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..072b5a3eee20f 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,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 + + 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/Inputs/tagged-union-member-count/stdnamespace.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/tagged-union-member-count/stdnamespace.h new file mode 100644 index 0000000000000..4f6eafde8501e --- /dev/null +++ b/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; +}; 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 60c93c553baca..f78a05f54a15e 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, @@ -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 + +// 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; +}; 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..c8e36bc6e1a44 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,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 + +// 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; +}; 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..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, @@ -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 + +// 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..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, @@ -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 + +// 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; +};