llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
@llvm/pr-subscribers-clang-tools-extra
Author: Andrey Karlov (negativ)
<details>
<summary>Changes</summary>
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 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.
``````````
</details>
https://github.com/llvm/llvm-project/pull/147066
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
