On Thu, Mar 6, 2025 at 8:53 AM Jonathan Wakely <jwak...@redhat.com> wrote:
>
> LWG 4172 was approved in Hagenberg, February 2025, fixing
> std::unique_lock and std::shared_lock to work correctly for
> self-move-assignment.
>
> Our std::shared_lock was already doing the right thing (contradicting
> the standard) so just add a comment there. Our std::unique_lock needs to
> be fixed to do the right thing.
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/unique_lock.h (unique_lock::operator=): Fix for
>         self-move-assignment.
>         * include/std/shared_mutex (shared_lock::operator=): Add
>         comment.
>         * testsuite/30_threads/shared_lock/cons/lwg4172.cc: New test.
>         * testsuite/30_threads/unique_lock/cons/lwg4172.cc: New test.

LGTM

> ---
>
> Tested x86_64-linux.
>
>  libstdc++-v3/include/bits/unique_lock.h       |  9 ++----
>  libstdc++-v3/include/std/shared_mutex         |  2 ++
>  .../30_threads/shared_lock/cons/lwg4172.cc    | 28 +++++++++++++++++++
>  .../30_threads/unique_lock/cons/lwg4172.cc    | 27 ++++++++++++++++++
>  4 files changed, 59 insertions(+), 7 deletions(-)
>  create mode 100644 
> libstdc++-v3/testsuite/30_threads/shared_lock/cons/lwg4172.cc
>  create mode 100644 
> libstdc++-v3/testsuite/30_threads/unique_lock/cons/lwg4172.cc
>
> diff --git a/libstdc++-v3/include/bits/unique_lock.h 
> b/libstdc++-v3/include/bits/unique_lock.h
> index 22ea7e9d772..5b1518745ca 100644
> --- a/libstdc++-v3/include/bits/unique_lock.h
> +++ b/libstdc++-v3/include/bits/unique_lock.h
> @@ -126,14 +126,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>        unique_lock& operator=(unique_lock&& __u) noexcept
>        {
> -       if(_M_owns)
> -         unlock();
> -
> +       // _GLIBCXX_RESOLVE_LIB_DEFECTS
> +       // 4172. unique_lock self-move-assignment is broken
>         unique_lock(std::move(__u)).swap(*this);
> -
> -       __u._M_device = 0;
> -       __u._M_owns = false;
> -
>         return *this;
>        }
>
> diff --git a/libstdc++-v3/include/std/shared_mutex 
> b/libstdc++-v3/include/std/shared_mutex
> index cbdf58f403b..94c8532399d 100644
> --- a/libstdc++-v3/include/std/shared_mutex
> +++ b/libstdc++-v3/include/std/shared_mutex
> @@ -780,6 +780,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        shared_lock&
>        operator=(shared_lock&& __sl) noexcept
>        {
> +       // _GLIBCXX_RESOLVE_LIB_DEFECTS
> +       // 4172. unique_lock self-move-assignment is broken
>         shared_lock(std::move(__sl)).swap(*this);
>         return *this;
>        }
> diff --git a/libstdc++-v3/testsuite/30_threads/shared_lock/cons/lwg4172.cc 
> b/libstdc++-v3/testsuite/30_threads/shared_lock/cons/lwg4172.cc
> new file mode 100644
> index 00000000000..0a3bf10b8bb
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/30_threads/shared_lock/cons/lwg4172.cc
> @@ -0,0 +1,28 @@
> +// { dg-do run { target c++14 } }
> +
> +// LWG 4172. unique_lock self-move-assignment is broken
> +
> +#include <shared_mutex>
> +#include <testsuite_hooks.h>
> +
> +void
> +test_self_move()
> +{
> +  struct Lockable
> +  {
> +    bool locked = false;
> +    void lock_shared() { locked = true; }
> +    void unlock_shared() { locked = false; }
> +    bool try_lock_shared() { if (locked) return false; return locked = true; 
> }
> +  };
> +
> +  Lockable x;
> +  std::shared_lock<Lockable> l(x);
> +  l = std::move(l);
> +  VERIFY(x.locked);
> +}
> +
> +int main()
> +{
> +  test_self_move();
> +}
> diff --git a/libstdc++-v3/testsuite/30_threads/unique_lock/cons/lwg4172.cc 
> b/libstdc++-v3/testsuite/30_threads/unique_lock/cons/lwg4172.cc
> new file mode 100644
> index 00000000000..37542b586a9
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/30_threads/unique_lock/cons/lwg4172.cc
> @@ -0,0 +1,27 @@
> +// { dg-do run { target c++11 } }
> +
> +// LWG 4172. unique_lock self-move-assignment is broken
> +
> +#include <mutex>
> +#include <testsuite_hooks.h>
> +
> +void
> +test_self_move()
> +{
> +  struct Lockable
> +  {
> +    bool locked = false;
> +    void lock() { locked = true; }
> +    void unlock() { locked = false; }
> +  };
> +
> +  Lockable x;
> +  std::unique_lock<Lockable> l(x);
> +  l = std::move(l);
> +  VERIFY(x.locked);
> +}
> +
> +int main()
> +{
> +  test_self_move();
> +}
> --
> 2.48.1
>

Reply via email to