[libcxx-commits] [libcxx] [libcxx] LWG4172 fix self-move-assignment in {unique|shared}_lock (PR #129542)

[Mohamed Atef via libcxx-commits]

https://github.com/elhewaty updated https://github.com/llvm/llvm-project/pull/129542

>From 7b8417e47f602a3bc40287a5f31e8f2f5430f02b Mon Sep 17 00:00:00 2001
From: Mohamed Atef <mohamedatef1698 at gmail.com>
Date: Tue, 14 Oct 2025 11:13:58 +0300
Subject: [PATCH 1/2] [libcxx] LWG4172 fix self-move-assignment in
 {unique|shared}_lock

---
 libcxx/include/__mutex/unique_lock.h          |  8 +------
 libcxx/include/shared_mutex                   |  9 +-------
 .../move_assign.pass.cpp                      | 23 +++++++++++++------
 .../move_assign.pass.cpp                      | 11 ++++++++-
 4 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/libcxx/include/__mutex/unique_lock.h b/libcxx/include/__mutex/unique_lock.h
index aea93eb9b8c9b..5fd50282a3f2a 100644
--- a/libcxx/include/__mutex/unique_lock.h
+++ b/libcxx/include/__mutex/unique_lock.h
@@ -74,13 +74,7 @@ class unique_lock {
   }
 
   _LIBCPP_HIDE_FROM_ABI unique_lock& operator=(unique_lock&& __u) _NOEXCEPT {
-    if (__owns_)
-      __m_->unlock();
-
-    __m_        = __u.__m_;
-    __owns_     = __u.__owns_;
-    __u.__m_    = nullptr;
-    __u.__owns_ = false;
+    unique_lock{std::move(__u)}.swap(*this);
     return *this;
   }
 
diff --git a/libcxx/include/shared_mutex b/libcxx/include/shared_mutex
index 8c02e348e4de7..bbcdc6cf554f3 100644
--- a/libcxx/include/shared_mutex
+++ b/libcxx/include/shared_mutex
@@ -340,14 +340,7 @@ public:
   }
 
   _LIBCPP_HIDE_FROM_ABI shared_lock& operator=(shared_lock&& __u) _NOEXCEPT {
-    if (__owns_)
-      __m_->unlock_shared();
-    __m_        = nullptr;
-    __owns_     = false;
-    __m_        = __u.__m_;
-    __owns_     = __u.__owns_;
-    __u.__m_    = nullptr;
-    __u.__owns_ = false;
+    shared_lock{std::move(__u)}.swap(*this);
     return *this;
   }
 
diff --git a/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/move_assign.pass.cpp b/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/move_assign.pass.cpp
index 6d7838e8c6c95..b0647f06e4c43 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/move_assign.pass.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/move_assign.pass.cpp
@@ -21,22 +21,31 @@
 
 #include "test_macros.h"
 
-
-int main(int, char**)
-{
-    {
+int main(int, char**) {
+  {
     typedef std::shared_timed_mutex M;
     M m0;
     M m1;
     std::shared_lock<M> lk0(m0);
     std::shared_lock<M> lk1(m1);
+
+    // Test self move assignment for lk0.
+    lk0 = std::move(lk0);
+    assert(lk0.mutex() == std::addressof(m0));
+    assert(lk0.owns_lock() == true);
+
     lk1 = std::move(lk0);
     assert(lk1.mutex() == std::addressof(m0));
     assert(lk1.owns_lock() == true);
     assert(lk0.mutex() == nullptr);
     assert(lk0.owns_lock() == false);
-    }
-    {
+
+    // Test self move assignment for lk1.
+    lk1 = std::move(lk1);
+    assert(lk1.mutex() == std::addressof(m0));
+    assert(lk1.owns_lock() == true);
+  }
+  {
     typedef nasty_mutex M;
     M m0;
     M m1;
@@ -47,7 +56,7 @@ int main(int, char**)
     assert(lk1.owns_lock() == true);
     assert(lk0.mutex() == nullptr);
     assert(lk0.owns_lock() == false);
-    }
+  }
 
   return 0;
 }
diff --git a/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.cons/move_assign.pass.cpp b/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.cons/move_assign.pass.cpp
index 588d8332c4164..5b25cb6d5940d 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.cons/move_assign.pass.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.cons/move_assign.pass.cpp
@@ -24,13 +24,22 @@ int main(int, char**) {
   std::unique_lock<checking_mutex> lk0(m0);
   std::unique_lock<checking_mutex> lk1(m1);
 
+  // Test self move assignment for lk0.
+  lk0 = std::move(lk0);
+  assert(lk0.mutex() == std::addressof(m0));
+  assert(lk0.owns_lock() == true);
+
   auto& result = (lk1 = std::move(lk0));
 
   assert(&result == &lk1);
   assert(lk1.mutex() == std::addressof(m0));
   assert(lk1.owns_lock());
   assert(lk0.mutex() == nullptr);
-  assert(!lk0.owns_lock());
+  assert(lk0.owns_lock() == false);
 
+  // Test self move assignment for lk1
+  lk1 = std::move(lk1);
+  assert(lk1.mutex() == std::addressof(m0));
+  assert(lk1.owns_lock() == true);
   return 0;
 }

>From b42e8d70a07ce7dd008abbd5b399df04aafd119f Mon Sep 17 00:00:00 2001
From: Mohamed Atef <mohamedatef1698 at gmail.com>
Date: Sat, 8 Nov 2025 14:15:55 +0200
Subject: [PATCH 2/2] [libcxx] Address comments to fix build failures

---
 libcxx/include/__mutex/unique_lock.h | 1 +
 libcxx/include/shared_mutex          | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/libcxx/include/__mutex/unique_lock.h b/libcxx/include/__mutex/unique_lock.h
index 5fd50282a3f2a..8f023228e9465 100644
--- a/libcxx/include/__mutex/unique_lock.h
+++ b/libcxx/include/__mutex/unique_lock.h
@@ -16,6 +16,7 @@
 #include <__mutex/tag_types.h>
 #include <__system_error/throw_system_error.h>
 #include <__utility/swap.h>
+#include <__utility/move.h>
 #include <cerrno>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
diff --git a/libcxx/include/shared_mutex b/libcxx/include/shared_mutex
index bbcdc6cf554f3..f6d3ac0bc981e 100644
--- a/libcxx/include/shared_mutex
+++ b/libcxx/include/shared_mutex
@@ -139,6 +139,7 @@ template <class Mutex>
 #    include <__mutex/unique_lock.h>
 #    include <__system_error/throw_system_error.h>
 #    include <__utility/swap.h>
+#    include <__utility/move.h>
 #    include <cerrno>
 #    include <version>
 
@@ -151,6 +152,8 @@ _LIBCPP_PUSH_MACROS
 #        pragma GCC system_header
 #      endif
 
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 struct _LIBCPP_EXPORTED_FROM_ABI __shared_mutex_base {
@@ -430,6 +433,7 @@ inline _LIBCPP_HIDE_FROM_ABI void swap(shared_lock<_Mutex>& __x, shared_lock<_Mu
 }
 
 _LIBCPP_END_NAMESPACE_STD
+_LIBCPP_POP_MACROS
 
 #    endif // _LIBCPP_STD_VER >= 14