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 <dein.nega...@gmail.com>
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 <dein.nega...@gmail.com>
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 T>
class auto_ptr {
};
+namespace pmr {
+ template <typename TYPE = void>
+ 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 {
+ return allocator_type();
+ }
+};
+
+// Support "extended" copy/move constructors + ADL swap
+class AllocatorAwareClassAdlSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+ using allocator_type = std::pmr::allocator<>;
+
+ AllocatorAwareClassAdlSwap(const AllocatorAwareClassAdlSwap& other) {
+ }
+
+ AllocatorAwareClassAdlSwap(const AllocatorAwareClassAdlSwap& other, const
allocator_type& alloc) {
+ }
+
+ AllocatorAwareClassAdlSwap& operator=(const AllocatorAwareClassAdlSwap&
other) {
+ AllocatorAwareClassAdlSwap tmp(other, get_allocator());
+ swap(*this, tmp);
+ return *this;
+ }
+
+ allocator_type get_allocator() const {
+ return allocator_type();
+ }
+
+ friend void swap(AllocatorAwareClassAdlSwap&, AllocatorAwareClassAdlSwap&)
{
+ }
+};
+
///////////////////////////////////////////////////////////////////
/// Test cases which should be caught by the check.
>From 3771a9c519ace58827508ac422cadbbf17e2699a Mon Sep 17 00:00:00 2001
From: Andrey Karlov <dein.nega...@gmail.com>
Date: Mon, 7 Jul 2025 10:43:13 +0300
Subject: [PATCH 3/8] clang-format
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 21 +++++++++----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 8307e744a434c..69f02ee37bba5 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(
>From 4f576fe13dad850149de43c9a28ac8dcb2d1e0ff Mon Sep 17 00:00:00 2001
From: Andrey Karlov <dein.nega...@gmail.com>
Date: Mon, 7 Jul 2025 11:03:43 +0300
Subject: [PATCH 4/8] Update ReleaseNotes
---
clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index f8f183e9de1cc..d27ff43f3af52 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -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
+ <clang-tidy/checks/bugprone/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.
+
- Improved :doc:`bugprone-unchecked-optional-access
<clang-tidy/checks/bugprone/unchecked-optional-access>` fixing false
positives from smart pointer accessors repeated in checking ``has_value``
>From 4b84a6ae3ab2df97bc7bdc80f24d2ba9cd5791a6 Mon Sep 17 00:00:00 2001
From: Andrey Karlov <dein.nega...@gmail.com>
Date: Mon, 7 Jul 2025 11:30:31 +0300
Subject: [PATCH 5/8] Update ReleaseNotes.rst
---
clang-tools-extra/docs/ReleaseNotes.rst | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 9ee23710d8f4b..33e17946eb5c8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -190,10 +190,10 @@ Changes in existing checks
length parameters.
- Improved :doc:`bugprone-unhandled-self-assignment
- <clang-tidy/checks/bugprone/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.
+ <clang-tidy/checks/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.
- Improved :doc:`bugprone-unchecked-optional-access
<clang-tidy/checks/bugprone/unchecked-optional-access>` fixing false
>From 6f4ec4ad355cb64c327c2748fa3aee082a23c294 Mon Sep 17 00:00:00 2001
From: Andrey Karlov <dein.nega...@gmail.com>
Date: Mon, 7 Jul 2025 12:03:45 +0300
Subject: [PATCH 6/8] Update ReleaseNotes.rst (typo fix)
---
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 33e17946eb5c8..aab36269413d6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -190,7 +190,7 @@ Changes in existing checks
length parameters.
- Improved :doc:`bugprone-unhandled-self-assignment
- <clang-tidy/checks/bugprone/unhandled-self-assignment` check by adding
+ <clang-tidy/checks/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.
>From 7d5e9f5e3ca23ce4a525bd83bbbe8a41bb6a99cd Mon Sep 17 00:00:00 2001
From: Andrey Karlov <dein.nega...@gmail.com>
Date: Mon, 14 Jul 2025 14:03:48 +0300
Subject: [PATCH 7/8] Adressing PR questions
---
.../bugprone/UnhandledSelfAssignmentCheck.cpp | 22 ++++++++++++-------
clang-tools-extra/docs/ReleaseNotes.rst | 11 +++++-----
2 files changed, 19 insertions(+), 14 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
index 69f02ee37bba5..c4c4267545b59 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -76,14 +76,20 @@ void
UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
// 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"))))))))));
+ 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("*"))))))))));
DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
if (WarnOnlyIfThisHasSuspiciousField) {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 2162146923b2b..433febebc3388 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -205,12 +205,6 @@ Changes in existing checks
calls of ``std::string`` constructor with char pointer, start position and
length parameters.
-- Improved :doc:`bugprone-unhandled-self-assignment
- <clang-tidy/checks/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.
-
- Improved :doc:`bugprone-unchecked-optional-access
<clang-tidy/checks/bugprone/unchecked-optional-access>` fixing false
positives from smart pointer accessors repeated in checking ``has_value``
@@ -218,6 +212,11 @@ Changes in existing checks
no longer be needed and will be removed. Also fixing false positive from
const reference accessors to objects containing optional member.
+- Improved :doc:`bugprone-unhandled-self-assignment
+ <clang-tidy/checks/bugprone/unhandled-self-assignment>` check by adding
+ an additional matcher that generalizes the copy-and-swap idiom pattern
+ detection.
+
- Improved :doc:`bugprone-unsafe-functions
<clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
additional C++ member functions to match.
>From b2d98ef47b3ac8a5874b1b009dfb6736f3347ff1 Mon Sep 17 00:00:00 2001
From: Andrey Karlov <dein.nega...@gmail.com>
Date: Mon, 21 Jul 2025 16:49:45 +0300
Subject: [PATCH 8/8] More tests
---
.../bugprone/unhandled-self-assignment.cpp | 116 ++++++++++++++++++
1 file changed, 116 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 f2f61062f883f..b55233c0fa58d 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
@@ -236,6 +236,122 @@ bool operator!=(Foo2 &, Foo2 &) {
};
}
+// Missing call to member `swap` function
+class AllocatorAwareClassNoSwap {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+ using allocator_type = std::pmr::allocator<>;
+
+ AllocatorAwareClassNoSwap(const AllocatorAwareClassNoSwap& other) {
+ }
+
+ AllocatorAwareClassNoSwap(const AllocatorAwareClassNoSwap& other, const
allocator_type& alloc) {
+ }
+
+ AllocatorAwareClassNoSwap& operator=(const AllocatorAwareClassNoSwap&
other) {
+ // CHECK-MESSAGES: [[@LINE-1]]:32: warning: operator=() does not
handle self-assignment properly [bugprone-unhandled-self-assignment]
+ AllocatorAwareClassNoSwap tmp(other, get_allocator());
+ return *this;
+ }
+
+ allocator_type get_allocator() const {
+ return allocator_type();
+ }
+};
+
+// "Wrong" type is passed to member `swap` function
+class AllocatorAwareClassSwapWrongArgType {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+ using allocator_type = std::pmr::allocator<>;
+
+ AllocatorAwareClassSwapWrongArgType(const
AllocatorAwareClassSwapWrongArgType& other) {
+ }
+
+ AllocatorAwareClassSwapWrongArgType(const
AllocatorAwareClassSwapWrongArgType& other, const allocator_type& alloc) {
+ }
+
+ AllocatorAwareClassSwapWrongArgType& operator=(const
AllocatorAwareClassSwapWrongArgType& other) {
+ // CHECK-MESSAGES: [[@LINE-1]]:42: warning: operator=() does not
handle self-assignment properly [bugprone-unhandled-self-assignment]
+ int tmp = 0;
+ swap(tmp);
+
+ return *this;
+ }
+
+ void swap(int&) noexcept {
+ }
+
+ allocator_type get_allocator() const {
+ return allocator_type();
+ }
+};
+
+
+// Making ADL `swap` call but with wrong argument type
+class AllocatorAwareClassAdlSwapWrongArgType {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+ using allocator_type = std::pmr::allocator<>;
+
+ AllocatorAwareClassAdlSwapWrongArgType(const
AllocatorAwareClassAdlSwapWrongArgType& other) {
+ }
+
+ AllocatorAwareClassAdlSwapWrongArgType(const
AllocatorAwareClassAdlSwapWrongArgType& other, const allocator_type& alloc) {
+ }
+
+ AllocatorAwareClassAdlSwapWrongArgType& operator=(const
AllocatorAwareClassAdlSwapWrongArgType& other) {
+ // CHECK-MESSAGES: [[@LINE-1]]:45: warning: operator=() does not
handle self-assignment properly [bugprone-unhandled-self-assignment]
+ int tmp = 0;
+ swap(*this, tmp);
+
+ return *this;
+ }
+
+ allocator_type get_allocator() const {
+ return allocator_type();
+ }
+
+ friend void swap(AllocatorAwareClassAdlSwapWrongArgType&, int&) {
+ }
+};
+
+// `this` isn't passed to `swap`
+class AllocatorAwareClassAdlSwapWrongArgs {
+ // pointer member to trigger bugprone-unhandled-self-assignment
+ void *foo = nullptr;
+
+ public:
+ using allocator_type = std::pmr::allocator<>;
+
+ AllocatorAwareClassAdlSwapWrongArgs(const
AllocatorAwareClassAdlSwapWrongArgs& other) {
+ }
+
+ AllocatorAwareClassAdlSwapWrongArgs(const
AllocatorAwareClassAdlSwapWrongArgs& other, const allocator_type& alloc) {
+ }
+
+ AllocatorAwareClassAdlSwapWrongArgs& operator=(const
AllocatorAwareClassAdlSwapWrongArgs& other) {
+ // CHECK-MESSAGES: [[@LINE-1]]:42: warning: operator=() does not
handle self-assignment properly [bugprone-unhandled-self-assignment]
+ AllocatorAwareClassAdlSwapWrongArgs tmp1(other, get_allocator());
+ AllocatorAwareClassAdlSwapWrongArgs tmp2(*this, get_allocator());
+ swap(tmp1, tmp2);
+ return *this;
+ }
+
+ allocator_type get_allocator() const {
+ return allocator_type();
+ }
+
+ friend void swap(AllocatorAwareClassAdlSwapWrongArgs&,
AllocatorAwareClassAdlSwapWrongArgs&) {
+ }
+};
+
///////////////////////////////////////////////////////////////////
/// Test cases correctly ignored by the check.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
