[clang-tools-extra] [clang-tidy] Fix false positive in readability-make-member-function-c… (PR #174286)
Arthur Courteaud via cfe-commits
cfe-commits at lists.llvm.org
Sun Jan 18 09:32:21 PST 2026
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 <arthurcourteaud at gmail.com>
Date: Sat, 3 Jan 2026 19:18:54 +0100
Subject: [PATCH 1/4] [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
>From af98d311245574b1db3e0156505510aef5b1c66e Mon Sep 17 00:00:00 2001
From: Arthur Courteaud <150355163+arthur3336 at users.noreply.github.com>
Date: Wed, 7 Jan 2026 16:49:35 +0100
Subject: [PATCH 2/4] Update
clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
Co-authored-by: Baranov Victor <bar.victor.2002 at gmail.com>
---
.../clang-tidy/readability/MakeMemberFunctionConstCheck.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp b/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
index 018af0f84f40f..addbb711b0ffb 100644
--- a/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
@@ -157,12 +157,10 @@ class FindUsageOfThis : public RecursiveASTVisitor<FindUsageOfThis> {
// 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()) {
+ if (const auto *Field = dyn_cast<FieldDecl>(Member->getMemberDecl()); Field->getParent()->isUnion()) {
const QualType MemberType = Field->getType();
if (MemberType->isPointerType() || MemberType->isReferenceType())
return false;
- }
}
if (Member->getFoundDecl().getAccess() != AS_public &&
>From 773192bda4c64bf8bf2fd61b8f8fbcac646170d8 Mon Sep 17 00:00:00 2001
From: Arthur Courteaud <150355163+arthur3336 at users.noreply.github.com>
Date: Wed, 7 Jan 2026 16:57:59 +0100
Subject: [PATCH 3/4] Fix pointer/reference type check for union members
---
.../readability/MakeMemberFunctionConstCheck.cpp | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp b/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
index addbb711b0ffb..d7d0045ddc700 100644
--- a/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
@@ -157,10 +157,11 @@ class FindUsageOfThis : public RecursiveASTVisitor<FindUsageOfThis> {
// Union members are never safe for pointer/reference types
// (all union members share memory).
- if (const auto *Field = dyn_cast<FieldDecl>(Member->getMemberDecl()); Field->getParent()->isUnion()) {
- const QualType MemberType = Field->getType();
- if (MemberType->isPointerType() || MemberType->isReferenceType())
- return false;
+ if (const auto *Field = dyn_cast<FieldDecl>(Member->getMemberDecl());
+ Field->getParent()->isUnion()) {
+ const QualType MemberType = Field->getType();
+ if (MemberType.isPointerType() || MemberType.isReferenceType())
+ return false;
}
if (Member->getFoundDecl().getAccess() != AS_public &&
>From 716ee1c7d954188128f57ed7506847f0e0ed1eb3 Mon Sep 17 00:00:00 2001
From: Arthur Courteaud <150355163+arthur3336 at users.noreply.github.com>
Date: Sun, 18 Jan 2026 18:29:53 +0100
Subject: [PATCH 4/4] Update
clang-tools-extra/test/clang-tidy/checkers/readability/make-member-function-const.cpp
Co-authored-by: Baranov Victor <bar.victor.2002 at gmail.com>
---
.../checkers/readability/make-member-function-const.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 7cdd9bfd7819a..901abdbd5aad0 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
@@ -367,7 +367,7 @@ namespace UnionMemberAccess {
double* double_ptr;
};
int& getInt() { return *int_ptr; } // Should NOT trigger warning
- double& getDouble() { return *double_ptr; } // Should NOT trigger warning
+ double& get_double() { return *double_ptr; } // Should NOT trigger warning
};
// Named union member access
More information about the cfe-commits
mailing list