[libcxx] LWG4172 fix self-move-assignment in {unique|shared}_lock

[elhewaty]

Fixes: #127861

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: None (elhewaty)

Changes

Fixes: #127861

---

Full diff: https://github.com/llvm/llvm-project/pull/129542.diff

5 Files Affected:

• (modified) libcxx/docs/Status/Cxx2cIssues.csv (+1-1)
• (modified) libcxx/include/__mutex/unique_lock.h (+1-7)
• (modified) libcxx/include/shared_mutex (+1-8)
• (modified) libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/move_assign.pass.cpp (+10-7)
• (modified) libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.cons/move_assign.pass.cpp (+4)

diff --git a/libcxx/docs/Status/Cxx2cIssues.csv b/libcxx/docs/Status/Cxx2cIssues.csv
index 09aef3554fb6f..d7653cc3f6bd5 100644
--- a/libcxx/docs/Status/Cxx2cIssues.csv
+++ b/libcxx/docs/Status/Cxx2cIssues.csv
@@ -113,7 +113,7 @@
 "","","","","",""
 "`LWG3578 <https://wg21.link/3578>`__","Iterator SCARYness in the context of associative container merging","2025-02 (Hagenberg)","","",""
 "`LWG3956 <https://wg21.link/3956>`__","``chrono::parse`` uses ``from_stream`` as a customization point","2025-02 (Hagenberg)","","",""
-"`LWG4172 <https://wg21.link/4172>`__","``unique_lock`` self-move-assignment is broken","2025-02 (Hagenberg)","","",""
+"`LWG4172 <https://wg21.link/4172>`__","``unique_lock`` self-move-assignment is broken","2025-02 (Hagenberg)","|Complete|","21",""
 "`LWG4175 <https://wg21.link/4175>`__","``get_env()`` specified in terms of ``as_const()`` but this doesn't work with rvalue senders","2025-02 (Hagenberg)","","",""
 "`LWG4179 <https://wg21.link/4179>`__","Wrong range in ``[alg.search]``","2025-02 (Hagenberg)","","",""
 "`LWG4186 <https://wg21.link/4186>`__","``regex_traits::transform_primary`` mistakenly detects ``typeid`` of a function","2025-02 (Hagenberg)","","",""
diff --git a/libcxx/include/__mutex/unique_lock.h b/libcxx/include/__mutex/unique_lock.h
index 84073ef4b5114..f5f48ddac6d8e 100644
--- a/libcxx/include/__mutex/unique_lock.h
+++ b/libcxx/include/__mutex/unique_lock.h
@@ -74,13 +74,7 @@ class _LIBCPP_TEMPLATE_VIS 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 b1e2a5d434400..28449a186a918 100644
--- a/libcxx/include/shared_mutex
+++ b/libcxx/include/shared_mutex
@@ -356,14 +356,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..fb51093e575b8 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,10 +21,8 @@
 
 #include "test_macros.h"
 
-
-int main(int, char**)
-{
-    {
+int main(int, char**) {
+  {
     typedef std::shared_timed_mutex M;
     M m0;
     M m1;
@@ -35,8 +33,13 @@ int main(int, char**)
     assert(lk1.owns_lock() == true);
     assert(lk0.mutex() == nullptr);
     assert(lk0.owns_lock() == false);
-    }
-    {
+
+    // Test self move assignment.
+    lk1 = std::move(lk1);
+    assert(lk1.mutex() == std::addressof(m0));
+    assert(lk1.owns_lock());
+  }
+  {
     typedef nasty_mutex M;
     M m0;
     M m1;
@@ -47,7 +50,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..eb2c9a1b5637b 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
@@ -32,5 +32,9 @@ int main(int, char**) {
   assert(lk0.mutex() == nullptr);
   assert(!lk0.owns_lock());
 
+  // Test Test self move assignment.
+  lk1 = std::move(lk1);
+  assert(lk1.mutex() == std::addressof(m0));
+  assert(lk1.owns_lock());
   return 0;
 }

[mordante]

Thanks for picking this up, some comments.

[mordante]

For consistency with the test above.

Suggested change

	assert(lk1.owns_lock());
	assert(lk1.owns_lock() == true);

Can you also test move_assignment of lk0?

[mordante]

Typically for this LWG issue you need to update the synopsis due to the adding of noexcept, but that seems already done.

[mordante]

I noticed this test does not validate the noexcept status of the function, can you add that.
(The next test should add it since there it's a new requirement.)
The test for swap has an example how to test for noexcept.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[elhewaty]

@frederick-vs-ja, @mordante any hint about these failures?

[elhewaty]

@ldionne can you please have a look?

[frederick-vs-ja]

You should include <__utility/move.h> in <__mutex/unique_lock.h> and <shared_mutex>. And then you need to add

_LIBCPP_PUSH_MACROS
#include <__undef_macros>

immediately before

_LIBCPP_BEGIN_NAMESPACE_STD

, and

_LIBCPP_POP_MACROS

immediately after

_LIBCPP_END_NAMESPACE_STD