New issue from Casey: unique_lock self-move-assignment is broken

Author: jwakely

xml/issue4172.xml

@@ -0,0 +1,106 @@
+<?xml version='1.0' encoding='utf-8' standalone='no'?>
+<!DOCTYPE issue SYSTEM "lwg-issue.dtd">
+
+<issue num="4172" status="New">
+<title>unique_lock self-move-assignment is broken</title>
+<section><sref ref="[thread.lock.unique.cons]"/></section>
+<submitter>Casey Carter</submitter>
+<date>13 Nov 2024</date>
+<priority>99</priority>
+
+<discussion>
+<p>
+The postconditions in <sref ref="[thread.lock.unique.cons]"/> paragraph 19:
+
+<blockquote>
+<i>Postconditions</i>:
+`pm == u_p.pm` and `owns == u_p.owns`
+(where `u_p` is the state of `u` just prior to this construction),
+`u.pm == 0` and `u.owns == false`.
+</blockquote>
+
+contradict themselves if `*this` and the parameter `u` refer to the same object.
+(Presumably "this construction" means the assignment, and it is copy-pasta from
+the move constructor postconditions.) Apparently
+`unique_lock` didn't get the memo that we require well-defined behavior
+from self-move-assignment as of LWG <iref ref="2839"/>.
+</p>
+<p>
+Also, the move assignment operator doesn't specify what it returns.
+</p>
+</discussion>
+
+<resolution>
+<p>
+This wording is relative to <paper num="N4993"/>.
+</p>
+
+<blockquote class="note">
+Drafting Note: I've chosen to use the move-into-temporary-and-swap idiom here
+to keep things short and sweet.
+Since move construction, swap, and destruction are all `noexcept`,
+I've promoted move assignment from "<i>Throws</i>: Nothing" to `noexcept` as well.
+This is consistent with eliminating the implicit narrow contract condition
+that `*this` and `u` refer to distinct objects.
+</blockquote>
+
+<ol>
+<li>
+<p>
+In the class synopsis in <sref ref="[thread.lock.unique.general]"/>,
+annotate the move assignment operator as `noexcept`:
+</p>
+
+<blockquote><pre><code>
+  namespace std {
+    template&lt;class Mutex&gt;
+    class unique_lock {
+      [...]
+      unique_lock&amp; operator=(unique_lock&amp;&amp; u) <ins>noexcept</ins>;
+      [...]
+    };
+  }
+</code></pre></blockquote>
+</li>
+
+<li>
+<p>
+Modify <sref ref="[thread.lock.unique.cons]"/> as follows:
+</p>
+
+<blockquote>
+<pre><code>
+unique_lock&amp; operator=(unique_lock&amp;&amp; u) <ins>noexcept</ins>;
+</code></pre>
+<p>
+-18- <i>Effects</i>:
+<del>If `owns` calls `pm->unlock()`.</del>
+<ins>Equivalent to: `unique_lock{std::move(u)}.swap(*this)`.</ins>
+</p>
+<p>
+<ins>-?- <i>Returns</i>: `*this`.</ins>
+</p>
+<p>
+<del>-19- <i>Postconditions</i>:
+`pm == u_p.pm` and `owns == u_p.owns`
+(where `u_p` is the state of `u` just prior to this construction),
+`u.pm == 0` and `u.owns == false`.
+</del>
+</p>
+<p>
+<del>-20- [<i>Note 1</i>:
+With a recursive mutex it is possible for both `*this` and u to own
+the same mutex before the assignment.
+In this case, *this will own the mutex after the assignment and u will not.
+&mdash; <i>end note</i>]</del>
+</p>
+<p>
+<del>-21- Throws: Nothing.</del>
+</p>
+</blockquote>
+</li>
+</ol>
+
+</resolution>
+
+</issue>