llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: None (tigbr) <details> <summary>Changes</summary> This patch implements a fix for the false-positive case described in [issue #<!-- -->134840](https://github.com/llvm/llvm-project/issues/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. --- Full diff: https://github.com/llvm/llvm-project/pull/135831.diff 5 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp (+8-4) - (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.c (+13) - (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.cpp (+13) - (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.m (+13) - (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/tagged-union-member-count.mm (+13) ``````````diff 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; +}; `````````` </details> https://github.com/llvm/llvm-project/pull/135831 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
