From eeccc10a5fd8541ee9a34b5cbeb62f8da4986851 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Wed, 12 Mar 2025 19:42:22 +0200 Subject: [PATCH 1/5] LWG-4172 --- stl/inc/mutex | 13 +-- stl/inc/shared_mutex | 9 +- tests/std/test.lst | 1 + .../env.lst | 4 + .../test.cpp | 101 ++++++++++++++++++ 5 files changed, 109 insertions(+), 19 deletions(-) create mode 100644 tests/std/tests/LWG4172_unique_lock_self_move_assignment/env.lst create mode 100644 tests/std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp diff --git a/stl/inc/mutex b/stl/inc/mutex index 25cb1aed89..82ec2f30ec 100644 --- a/stl/inc/mutex +++ b/stl/inc/mutex @@ -170,17 +170,8 @@ public: _Other._Owns = false; } - unique_lock& operator=(unique_lock&& _Other) noexcept /* strengthened */ { - if (this != _STD addressof(_Other)) { - if (_Owns) { - _Pmtx->unlock(); - } - - _Pmtx = _Other._Pmtx; - _Owns = _Other._Owns; - _Other._Pmtx = nullptr; - _Other._Owns = false; - } + unique_lock& operator=(unique_lock&& _Other) noexcept { + unique_lock(_STD move(_Other)).swap(*this); return *this; } diff --git a/stl/inc/shared_mutex b/stl/inc/shared_mutex index 3c67475e64..ac758fa3c3 100644 --- a/stl/inc/shared_mutex +++ b/stl/inc/shared_mutex @@ -251,14 +251,7 @@ public: } shared_lock& operator=(shared_lock&& _Right) noexcept { - if (_Owns) { - _Pmtx->unlock_shared(); - } - - _Pmtx = _Right._Pmtx; - _Owns = _Right._Owns; - _Right._Pmtx = nullptr; - _Right._Owns = false; + shared_lock(_STD move(_Right)).swap(*this); return *this; } diff --git a/tests/std/test.lst b/tests/std/test.lst index 1c40a3294d..484a11f3a1 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -269,6 +269,7 @@ tests\LWG3561_discard_block_engine_counter tests\LWG3610_iota_view_size_and_integer_class tests\LWG4084_iostream_uppercase_inf_nan tests\LWG4105_ranges_ends_with_and_integer_class +tests\LWG4172_unique_lock_self_move_assignment tests\P0009R18_mdspan_default_accessor tests\P0009R18_mdspan_extents tests\P0009R18_mdspan_extents_death diff --git a/tests/std/tests/LWG4172_unique_lock_self_move_assignment/env.lst b/tests/std/tests/LWG4172_unique_lock_self_move_assignment/env.lst new file mode 100644 index 0000000000..19f025bd0e --- /dev/null +++ b/tests/std/tests/LWG4172_unique_lock_self_move_assignment/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\usual_matrix.lst diff --git a/tests/std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp b/tests/std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp new file mode 100644 index 0000000000..e81d2f27ce --- /dev/null +++ b/tests/std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include + +using namespace std; + +struct lockable_with_counters { + void lock() { + ++lock_count; + } + + void unlock() { + ++unlock_count; + } + + void lock_shared() { + ++shared_lock_count; + } + + void unlock_shared() { + ++shared_unlock_count; + } + + int lock_count = 0; + int unlock_count = 0; + int shared_lock_count = 0; + int shared_unlock_count = 0; +}; + + +int main() { + lockable_with_counters lockable1; + lockable_with_counters lockable2; + + { + unique_lock lock_a(lockable1); + assert(lockable1.lock_count == 1); + assert(lockable1.unlock_count == 0); + + lock_a = move(lock_a); + assert(lockable1.lock_count == 1); + assert(lockable1.unlock_count == 0); + { + unique_lock lock_b(lockable2); + assert(lockable2.lock_count == 1); + assert(lockable2.unlock_count == 0); + + lock_a = move(lock_b); + + assert(lockable1.lock_count == 1); + assert(lockable1.unlock_count == 1); + assert(lockable2.lock_count == 1); + assert(lockable2.unlock_count == 0); + } + + assert(lockable1.lock_count == 1); + assert(lockable1.unlock_count == 1); + assert(lockable2.lock_count == 1); + assert(lockable2.unlock_count == 0); + } + + assert(lockable1.lock_count == 1); + assert(lockable1.unlock_count == 1); + assert(lockable2.lock_count == 1); + assert(lockable2.unlock_count == 1); + + { + shared_lock lock_a(lockable1); + assert(lockable1.shared_lock_count == 1); + assert(lockable1.shared_unlock_count == 0); + + lock_a = move(lock_a); + assert(lockable1.shared_lock_count == 1); + assert(lockable1.shared_unlock_count == 0); + { + shared_lock lock_b(lockable2); + assert(lockable2.shared_lock_count == 1); + assert(lockable2.shared_unlock_count == 0); + + lock_a = move(lock_b); + + assert(lockable1.shared_lock_count == 1); + assert(lockable1.shared_unlock_count == 1); + assert(lockable2.shared_lock_count == 1); + assert(lockable2.shared_unlock_count == 0); + } + + assert(lockable1.shared_lock_count == 1); + assert(lockable1.shared_unlock_count == 1); + assert(lockable2.shared_lock_count == 1); + assert(lockable2.shared_unlock_count == 0); + } + + assert(lockable1.shared_lock_count == 1); + assert(lockable1.shared_unlock_count == 1); + assert(lockable2.shared_lock_count == 1); + assert(lockable2.shared_unlock_count == 1); +} From 60a7a54e38d00ceee61a0d3a9f67d3cf874ef01c Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Wed, 12 Mar 2025 20:22:48 +0200 Subject: [PATCH 2/5] fix clang warnings --- .../LWG4172_unique_lock_self_move_assignment/test.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp b/tests/std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp index e81d2f27ce..e961a0b671 100644 --- a/tests/std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp +++ b/tests/std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp @@ -30,7 +30,12 @@ struct lockable_with_counters { int shared_unlock_count = 0; }; - +#ifdef __clang__ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wself-move" +#endif // __clang__ +#pragma warning(push) +#pragma warning(disable : 26800) // use a moved-from object int main() { lockable_with_counters lockable1; lockable_with_counters lockable2; @@ -99,3 +104,7 @@ int main() { assert(lockable2.shared_lock_count == 1); assert(lockable2.shared_unlock_count == 1); } +#pragma warning(pop) +#ifdef __clang__ +#pragma clang diagnostic pop +#endif // __clang__ From 6971d5516e5239b67fad92ac4f5c1c738c42a9b5 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 12 Mar 2025 12:37:04 -0700 Subject: [PATCH 3/5] Use braces to construct temporaries. --- stl/inc/mutex | 2 +- stl/inc/shared_mutex | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/mutex b/stl/inc/mutex index 82ec2f30ec..8cb450c59d 100644 --- a/stl/inc/mutex +++ b/stl/inc/mutex @@ -171,7 +171,7 @@ public: } unique_lock& operator=(unique_lock&& _Other) noexcept { - unique_lock(_STD move(_Other)).swap(*this); + unique_lock{_STD move(_Other)}.swap(*this); return *this; } diff --git a/stl/inc/shared_mutex b/stl/inc/shared_mutex index ac758fa3c3..06ebb758e3 100644 --- a/stl/inc/shared_mutex +++ b/stl/inc/shared_mutex @@ -251,7 +251,7 @@ public: } shared_lock& operator=(shared_lock&& _Right) noexcept { - shared_lock(_STD move(_Right)).swap(*this); + shared_lock{_STD move(_Right)}.swap(*this); return *this; } From 93ea073a4d6caf852a448cf3b633bc4b832842ab Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 12 Mar 2025 12:42:43 -0700 Subject: [PATCH 4/5] Include `` for `move()`. --- .../std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp b/tests/std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp index e961a0b671..bf2aca8644 100644 --- a/tests/std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp +++ b/tests/std/tests/LWG4172_unique_lock_self_move_assignment/test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include using namespace std; From 11b37b688e5222e3db06b55a2946c27804c0e4c0 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 12 Mar 2025 16:29:04 -0700 Subject: [PATCH 5/5] usual_matrix => impure_matrix, as /clr:pure rejects mutex --- .../std/tests/LWG4172_unique_lock_self_move_assignment/env.lst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/LWG4172_unique_lock_self_move_assignment/env.lst b/tests/std/tests/LWG4172_unique_lock_self_move_assignment/env.lst index 19f025bd0e..f141421b29 100644 --- a/tests/std/tests/LWG4172_unique_lock_self_move_assignment/env.lst +++ b/tests/std/tests/LWG4172_unique_lock_self_move_assignment/env.lst @@ -1,4 +1,4 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -RUNALL_INCLUDE ..\usual_matrix.lst +RUNALL_INCLUDE ..\impure_matrix.lst