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/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 <dein.nega...@gmail.com> 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 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/5] 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/5] 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/5] 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits