https://github.com/arthur3336 updated https://github.com/llvm/llvm-project/pull/174286
>From bf229ecefaa17f1fd0c41037f825f8bfd0e1f23f Mon Sep 17 00:00:00 2001 From: Arthur Courteaud <[email protected]> Date: Sat, 3 Jan 2026 19:18:54 +0100 Subject: [PATCH] [clang-tidy] Fix false positive in readability-make-member-function-const for unions with pointer/reference members Fixes #174269 The check incorrectly suggested adding const to member functions that return non-const references from dereferencing pointer members within unions. Union members share memory, making such cases inherently non-const-safe. Added special handling to detect pointer/reference members within unions and correctly mark them as non-const usage. --- .../MakeMemberFunctionConstCheck.cpp | 11 ++++ clang-tools-extra/docs/ReleaseNotes.rst | 4 ++ .../make-member-function-const.cpp | 50 +++++++++++++++++++ 3 files changed, 65 insertions(+) diff --git a/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp b/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp index 2e6edd706b131..018af0f84f40f 100644 --- a/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp @@ -154,6 +154,17 @@ class FindUsageOfThis : public RecursiveASTVisitor<FindUsageOfThis> { // 1) has builtin type (a 'const int' cannot be modified), // 2) or it's a public member (the pointee of a public 'int * const' can // can be modified by any user of the class). + + // Union members are never safe for pointer/reference types + // (all union members share memory). + if (const auto *Field = dyn_cast<FieldDecl>(Member->getMemberDecl())) { + if (Field->getParent()->isUnion()) { + const QualType MemberType = Field->getType(); + if (MemberType->isPointerType() || MemberType->isReferenceType()) + return false; + } + } + if (Member->getFoundDecl().getAccess() != AS_public && !Cast->getType()->isBuiltinType()) return false; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 391c3a6b3db79..e4e5e06404d3e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -657,6 +657,10 @@ Changes in existing checks by not enforcing parameter name consistency between a variadic parameter pack in the primary template and specific parameters in its specializations. +- Improved :doc:`readability-make-member-function-const + <clang-tidy/checks/readability/make-member-function-const>` check by fixing + false positives when accessing pointer or reference members inside unions. + - Improved :doc:`readability-math-missing-parentheses <clang-tidy/checks/readability/math-missing-parentheses>` check by correctly diagnosing operator precedence issues inside parenthesized expressions. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/make-member-function-const.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/make-member-function-const.cpp index 72a8e362b9c8a..7cdd9bfd7819a 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/make-member-function-const.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/make-member-function-const.cpp @@ -336,3 +336,53 @@ struct MemberFunctionPointer { }; } // namespace Keep + +namespace UnionMemberAccess { + // Test case from GitHub issue #174269 + // Union with pointer member returning non-const reference + struct UnionWithPointer { + union { int* resource; }; + int& get() { return *resource; } // Should NOT trigger warning + const int& get() const { return *resource; } + }; + + // Union with pointer - single method variant + struct UnionWithPointerSingle { + union { int* resource; }; + int& get() { return *resource; } // Should NOT trigger warning + }; + + // Union with value type - should still suggest const where appropriate + struct UnionWithValue { + union { int value; }; + int get() { return value; } + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: method 'get' can be made const + // CHECK-FIXES: int get() const { return value; } + }; + + // Union with multiple pointer members + struct UnionMultiplePointers { + union { + int* int_ptr; + double* double_ptr; + }; + int& getInt() { return *int_ptr; } // Should NOT trigger warning + double& getDouble() { return *double_ptr; } // Should NOT trigger warning + }; + + // Named union member access + struct NamedUnion { + union Inner { + int* resource; + } inner; + int& get() { return *inner.resource; } // Should NOT trigger warning + }; + + // Regular struct for comparison - this SHOULD work as before + struct RegularStruct { + private: + int* ptr; + public: + int& get() { return *ptr; } // Should NOT trigger warning (private non-const access) + }; +} // namespace UnionMemberAccess _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
