[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 01/10] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 02/10] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/9] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/9] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
@@ -113,7 +113,11 @@ Changes in existing checks - Improved :doc:`bugprone-infinite-loop ` check by adding detection for variables introduced by structured bindings. - +- Improved :doc:`bugprone-unhandled-self-assignment vbvictor wrote: Please place an empty line between entries ```suggestion - Improved :doc:`bugprone-unhandled-self-assignment ``` https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/vbvictor approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
negativ wrote:
@vbvictor hey! I've added test cases for when:
- Missing call to `swap` function entirely
- There is a call to the member `swap` function but the argument type is "wrong"
- There is a call to the `ADL` `swap` function but the type of one argument is
"wrong"
- There is a call to the `ADL` `swap` function but `*this` isn't passed to it
https://github.com/llvm/llvm-project/pull/147066
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/9] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/9] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/8] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/8] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/7] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/7] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
@@ -69,6 +69,28 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl()),
+ hasBody(compoundStmt(
+ hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(stmt(anyOf(
+ cxxMemberCallExpr(hasArgument(
+ 0, declRefExpr(to(varDecl(equalsBoundNode("tmp_var")),
+ callExpr(
+ callee(functionDecl(hasName("swap"))), argumentCountIs(2),
+ hasAnyArgument(
+ declRefExpr(to(varDecl(equalsBoundNode("tmp_var"),
+ hasAnyArgument(unaryOperator(has(cxxThisExpr()),
+ hasOperatorName("*"));
vbvictor wrote:
But still other test cases could be added.
https://github.com/llvm/llvm-project/pull/147066
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
@@ -69,6 +69,28 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl()),
+ hasBody(compoundStmt(
+ hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(stmt(anyOf(
+ cxxMemberCallExpr(hasArgument(
+ 0, declRefExpr(to(varDecl(equalsBoundNode("tmp_var")),
+ callExpr(
+ callee(functionDecl(hasName("swap"))), argumentCountIs(2),
+ hasAnyArgument(
+ declRefExpr(to(varDecl(equalsBoundNode("tmp_var"),
+ hasAnyArgument(unaryOperator(has(cxxThisExpr()),
+ hasOperatorName("*"));
vbvictor wrote:
Oh yes, I had in mind `std::swap` as a function that will place value of 2nd
argument inside 1st like `std::move`. Yes it isn't needed here, sorry for the
noise.
https://github.com/llvm/llvm-project/pull/147066
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/vbvictor edited https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/vbvictor edited https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
@@ -69,6 +69,28 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl()),
+ hasBody(compoundStmt(
+ hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(stmt(anyOf(
+ cxxMemberCallExpr(hasArgument(
+ 0, declRefExpr(to(varDecl(equalsBoundNode("tmp_var")),
+ callExpr(
+ callee(functionDecl(hasName("swap"))), argumentCountIs(2),
+ hasAnyArgument(
+ declRefExpr(to(varDecl(equalsBoundNode("tmp_var"),
+ hasAnyArgument(unaryOperator(has(cxxThisExpr()),
+ hasOperatorName("*"));
negativ wrote:
@vbvictor why should we fire a warning for the following code?
```cpp
Thing& operator=(const Thing& other) {
using std::swap;
Thing tmp{other};
swap(tmp, *this);
return *this;
}
```
I don't think hardcoding the argument order gives us any real technical
benefits (maybe just slightly faster matching) but could cause false-positive
warnings on perfectly good code.
https://github.com/llvm/llvm-project/pull/147066
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
vbvictor wrote:
Could you add cases when the check will fire (I suppose it at least):
- when the user did `swap(tmp, *this);`
- when user forgot to place `swap` at all.
- when user created `tmp` variable and did `swap`, but `tmp` variable had a
wrong type.
https://github.com/llvm/llvm-project/pull/147066
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
@@ -69,6 +69,28 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl()),
+ hasBody(compoundStmt(
+ hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(stmt(anyOf(
+ cxxMemberCallExpr(hasArgument(
+ 0, declRefExpr(to(varDecl(equalsBoundNode("tmp_var")),
+ callExpr(
+ callee(functionDecl(hasName("swap"))), argumentCountIs(2),
+ hasAnyArgument(
+ declRefExpr(to(varDecl(equalsBoundNode("tmp_var"),
+ hasAnyArgument(unaryOperator(has(cxxThisExpr()),
+ hasOperatorName("*"));
vbvictor wrote:
When we have `std::swap(*this, tmp);`, we expect the first argument to be
`*this` and second `tmp`. The matcher should be more specific about it: use
`hasArgument(0, ...cxxThisExpr())` and `hasArgument(1,
...equalsBoundNode("tmp_var"))`.
https://github.com/llvm/llvm-project/pull/147066
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/7] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/7] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/5chmidti approved this pull request. https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
@@ -69,6 +69,22 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
negativ wrote:
@5chmidti fixed as suggested. thank you!
https://github.com/llvm/llvm-project/pull/147066
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/7] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/7] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
@@ -69,6 +69,22 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
negativ wrote:
@5chmidti since I'm just getting started with AST-matchers, I went with the
safe approach :D Stripped out the extra restrictions - it's simply
`ofClass(cxxRecordDecl())` now
https://github.com/llvm/llvm-project/pull/147066
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
@@ -189,6 +189,12 @@ Changes in existing checks calls of ``std::string`` constructor with char pointer, start position and length parameters. +- Improved :doc:`bugprone-unhandled-self-assignment + ` check by adding + an additional matcher that generalizes the copy-and-swap idiom pattern + detection. The checker now properly recognizes copy-and-swap implementations + that use "extended" copy/move constructors. negativ wrote: Fixed https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
@@ -189,6 +189,12 @@ Changes in existing checks calls of ``std::string`` constructor with char pointer, start position and length parameters. +- Improved :doc:`bugprone-unhandled-self-assignment negativ wrote: @EugeneZelenko fixed https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/7] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/7] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/6] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/6] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
@@ -189,6 +189,12 @@ Changes in existing checks calls of ``std::string`` constructor with char pointer, start position and length parameters. +- Improved :doc:`bugprone-unhandled-self-assignment EugeneZelenko wrote: Should be after `bugprone-unchecked-optional-access` https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/6] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/6] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
MikeWeller wrote: Fixes https://github.com/llvm/llvm-project/issues/146324 https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/5] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/5] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
negativ wrote: > Please mention changes in Release Notes. @EugeneZelenko done. https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/4] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/4] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/4] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/4] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ updated
https://github.com/llvm/llvm-project/pull/147066
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/3] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/3] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other) {
+}
+
+AllocatorAwareClassStdSwap(const AllocatorAwareClassStdSwap& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClassStdSwap& operator=(const AllocatorAwareClassStdSwap&
other) {
+using std::swap;
+
+AllocatorAwareClassStdSwap tmp(other, get_allocator());
+swap(*this, tmp);
+return *this;
+}
+
+allocator_type get_allocator() const {
+re
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff HEAD~1 HEAD --extensions cpp --
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
``
View the diff from clang-format here.
``diff
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 8307e744a..69f02ee37 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
// Checking that some kind of constructor is called and followed by a `swap`:
// T& operator=(const T& other) {
//T tmp{this->internal_data(), some, other, args};
@@ -84,7 +84,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
hasAnyArgument(declRefExpr(to(varDecl(
equalsBoundNode("tmp_var"));
-
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -105,15 +105,14 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
hasType(arrayType(;
}
- Finder->addMatcher(cxxMethodDecl(ofClass(cxxRecordDecl().bind("class")),
- isCopyAssignmentOperator(), IsUserDefined,
- HasReferenceParam, HasNoSelfCheck,
- unless(HasNonTemplateSelfCopy),
- unless(HasTemplateSelfCopy),
- unless(HasCopyAndSwap),
- HasNoNestedSelfAssign, AdditionalMatcher)
- .bind("copyAssignmentOperator"),
- this);
+ Finder->addMatcher(
+ cxxMethodDecl(
+ ofClass(cxxRecordDecl().bind("class")), isCopyAssignmentOperator(),
+ IsUserDefined, HasReferenceParam, HasNoSelfCheck,
+ unless(HasNonTemplateSelfCopy), unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap), HasNoNestedSelfAssign, AdditionalMatcher)
+ .bind("copyAssignmentOperator"),
+ this);
}
void UnhandledSelfAssignmentCheck::check(
``
https://github.com/llvm/llvm-project/pull/147066
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/vbvictor edited https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] `clang-tidy`: Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/EugeneZelenko commented: Please mention changes in Release Notes. https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] `clang-tidy`: Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
@llvm/pr-subscribers-clang-tools-extra
Author: Andrey Karlov (negativ)
Changes
This change enhances the `bugprone-unhandled-self-assignment` checker by adding
an additional matcher that generalizes the copy-and-swap idiom pattern
detection.
# What Changed
Added a new matcher that checks for:
- An instance of the current class being created in operator= (regardless of
constructor arguments)
- That instance being passed to a `swap` function call
# Problem Solved
This fix reduces false positives in PMR-like scenarios where "extended"
constructors are used (typically taking an additional allocator argument). The
checker now properly recognizes copy-and-swap implementations that use extended
copy/move constructors instead of flagging them as unhandled self-assignment
cases.
For e.g:
```cpp
T& operator=(const T& other) {
T tmp{other.internal_data(), get_allocator()}; // Extended constructor
swap(tmp);
return *this;
}
```
---
Full diff: https://github.com/llvm/llvm-project/pull/147066.diff
2 Files Affected:
- (modified)
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp (+18-1)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
(+90)
``diff
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClass& other) noexcept {
+}
+
+allocator_type get_allocator() const {
+return allocator_type();
+}
+};
+
+// Support "extended" copy/move constructors + std::swap
+class AllocatorAwareClassStdSwap {
+ // pointer member to trigger bugprone-unhandled-self
[clang-tools-extra] `clang-tidy`: Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ ready_for_review https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] `clang-tidy`: Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/147066 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] `clang-tidy`: Make `bugprone-unhandled-self-assignment` check more general (PR #147066)
https://github.com/negativ created
https://github.com/llvm/llvm-project/pull/147066
This change enhances the `bugprone-unhandled-self-assignment` checker by adding
an additional matcher that generalizes the copy-and-swap idiom pattern
detection.
# What Changed
Added a new matcher that checks for:
- An instance of the current class being created in operator= (regardless of
constructor arguments)
- That instance being passed to a `swap` function call
# Problem Solved
This fix reduces false positives in PMR-like scenarios where "extended"
constructors are used (typically taking an additional allocator argument). The
checker now properly recognizes copy-and-swap implementations that use extended
copy/move constructors instead of flagging them as unhandled self-assignment
cases.
For e.g:
```cpp
T& operator=(const T& other) {
T tmp{other.internal_data(), get_allocator()}; // Extended constructor
swap(tmp);
return *this;
}
```
>From 950b6dce92eb2a831e7bd7e7ba44b3e8cc354dd4 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:13:20 +0300
Subject: [PATCH 1/2] Checking that some kind of constructor is called and
followed by a `swap`
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 1f432c4ccc5f0..8307e744a434c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -68,7 +68,23 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
const auto HasNoNestedSelfAssign =
cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
hasName("operator="), ofClass(equalsBoundNode("class";
-
+
+ // Checking that some kind of constructor is called and followed by a `swap`:
+ // T& operator=(const T& other) {
+ //T tmp{this->internal_data(), some, other, args};
+ //swap(tmp);
+ //return *this;
+ // }
+ const auto HasCopyAndSwap = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl(),
+ hasDescendant(
+ stmt(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"
+ .bind("tmp_var")),
+ hasDescendant(callExpr(callee(functionDecl(hasName("swap"))),
+ hasAnyArgument(declRefExpr(to(varDecl(
+ equalsBoundNode("tmp_var"));
+
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
// Matcher for standard smart pointers.
@@ -94,6 +110,7 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
HasReferenceParam, HasNoSelfCheck,
unless(HasNonTemplateSelfCopy),
unless(HasTemplateSelfCopy),
+ unless(HasCopyAndSwap),
HasNoNestedSelfAssign, AdditionalMatcher)
.bind("copyAssignmentOperator"),
this);
>From 0cd33ec49693cfc6f5a72b72a9c50ba868634221 Mon Sep 17 00:00:00 2001
From: Andrey Karlov
Date: Fri, 4 Jul 2025 17:19:02 +0300
Subject: [PATCH 2/2] Tests
---
.../bugprone/unhandled-self-assignment.cpp| 90 +++
1 file changed, 90 insertions(+)
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
index 8610393449f97..f2f61062f883f 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-self-assignment.cpp
@@ -28,6 +28,13 @@ template
class auto_ptr {
};
+namespace pmr {
+template
+class allocator {};
+}
+
+struct allocator_arg_t {} allocator_arg;
+
} // namespace std
void assert(int expression){};
@@ -540,6 +547,89 @@ class NotACopyAssignmentOperator {
Uy *getUy() const { return Ptr2; }
};
+// Support "extended" copy/move constructors
+class AllocatorAwareClass {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+using allocator_type = std::pmr::allocator<>;
+
+AllocatorAwareClass(const AllocatorAwareClass& other) {
+}
+
+AllocatorAwareClass(const AllocatorAwareClass& other, const
allocator_type& alloc) {
+}
+
+AllocatorAwareClass& operator=(const AllocatorAwareClass& other) {
+AllocatorAwareClass tmp(other, get_allocator());
+swap(tmp);
+return *this;
+}
+
+void swap(AllocatorAwareClas
