Tweak 3891 P/R to use remove_cv_t<T> for local variable

xml/issue3891.xml

@@ -10,7 +10,7 @@
 
 <discussion>
 <p>
-Currently the <tt>value_type</tt> of <tt>std::expected</tt> can be a <i>cv</i>-qualified type, which is  possibly intented.
+Currently the <tt>value_type</tt> of <tt>std::expected</tt> can be a <i>cv</i>-qualified type, which is  possibly intended.
 However, LWG <iref ref="3870"/> disallows <tt>std::construct_at</tt> to construct objects via <tt><i>cv</i> T*</tt>, which
 breaks <tt>std::expected&lt;<i>cv</i> T, E&gt;</tt> because some operations are specified with <tt>std::construct_at</tt>
 (<sref ref="[expected.object.assign]"/>, <sref ref="[expected.object.swap]"/>).
@@ -29,9 +29,7 @@ source is destroyed immediately anyway. The else branch should use
 <code>remove_cv_t</code> too."
 </p>
 
-</discussion>
-
-<resolution>
+<superseded>
 <p>
 This wording is relative to <paper num="N4928"/>.
 </p>
@@ -190,6 +188,176 @@ rhs.<i>has_val</i> = true;
 
 </li>
 
+</ol>
+</superseded>
+
+<note>2024-10-02; Jonathan provides improved wording</note>
+<p>
+Removed the use of `value()` in the [expected.object.swap] p2 <i>Effects</i>:
+and added `remove_cv_t` to the local `T` in the `else`-branch.
+</p>
+
+</discussion>
+
+<resolution>
+<p>
+This wording is relative to <paper num="N4988"/>.
+</p>
+
+<blockquote class="note">
+<p>
+[<i>Drafting note</i>: When assignment and <tt>swap</tt> need to backup the old value by move construction,
+the source should be considered <i>cv</i>-unqualified, as the backup mechanism is only used internally.]
+</p>
+</blockquote>
+
+<ol>
+
+<li><p>Modify <sref ref="[expected.object.general]"/> as indicated:</p>
+
+<blockquote>
+<pre>
+[&hellip;]
+bool <i>has_val</i>;      // <i>exposition only</i>
+union {
+  <ins>remove_cv_t&lt;</ins>T<ins>&gt;</ins> <i>val</i>;       // <i>exposition only</i>
+  E <i>unex</i>;          // <i>exposition only</i>
+};
+[&hellip;]
+</pre>
+</blockquote>
+
+</li>
+
+<li><p>Modify <sref ref="[expected.object.assign]"/> as indicated:</p>
+
+<blockquote>
+<pre>
+constexpr expected&amp; operator=(const expected&amp; rhs);
+</pre>
+<blockquote>
+<p>
+-2- <i>Effects</i>:
+</p>
+<ol style="list-style-type: none">
+<li><p>(2.1) &mdash; If <tt>this-&gt;has_value() &amp;&amp; rhs.has_value()</tt> is <tt>true</tt>,
+equivalent to <tt><ins>value()</ins><del><i>val</i></del> = *rhs</tt>.</p></li>
+<li><p>[&hellip;]</p></li>
+</ol>
+</blockquote>
+[&hellip;]
+<pre>
+constexpr expected&amp; operator=(expected&amp;&amp; rhs) noexcept(<i>see below</i>);
+</pre>
+<blockquote>
+<p>
+[&hellip;]
+<p/>
+-6- <i>Effects</i>:
+</p>
+<ol style="list-style-type: none">
+<li><p>(6.1) &mdash; If <tt>this-&gt;has_value() &amp;&amp; rhs.has_value()</tt> is <tt>true</tt>,
+equivalent to <tt><ins>value()</ins><del><i>val</i></del> = std::move(*rhs)</tt>.</p></li>
+<li><p>[&hellip;]</p></li>
+</ol>
+</blockquote>
+[&hellip;]
+<pre>
+template&lt;class U = T&gt;
+  constexpr expected&amp; operator=(U&amp;&amp; v);
+</pre>
+<blockquote>
+<p>
+[&hellip;]
+<p/>
+-10- <i>Effects</i>:
+</p>
+<ol style="list-style-type: none">
+<li><p>(10.1) &mdash; If <tt>has_value()</tt> is <tt>true</tt>, equivalent to
+ <tt><ins>value()</ins><del><i>val</i></del> = std::forward&lt;U&gt;(v)</tt>.</p></li>
+<li><p>[&hellip;]</p></li>
+</ol>
+</blockquote>
+</blockquote>
+
+</li>
+
+<li><p>Modify Table 64: <tt>swap(expected&amp;)</tt> effects <sref ref="[tab:expected.object.swap]"/> as indicated:</p>
+
+<blockquote>
+
+<table border="1">
+<caption>Table 64 &mdash; <tt>swap(expected&amp;)</tt> effects <sref ref="[tab:expected.object.swap]"/></caption>
+<tr style="text-align:center">
+<th></th>
+<th><tt>this-&gt;has_value()</tt></th>
+<th><tt>!this-&gt;has_value()</tt></th>
+</tr>
+<tr>
+<td><tt>rhs.has_value()</tt></td>
+<td>equivalent to: <tt>using std::swap;</tt><br/>
+<tt>swap(<ins>value()</ins><del><i>val</i></del>, rhs.<ins>value()</ins><del><i>val</i></del>);</tt></td>
+<td>calls <tt>rhs.swap(*this)</tt></td>
+</tr>
+
+<tr>
+<td colspan="3" align="center">
+<tt>[&hellip;]</tt>
+</td>
+</tr>
+</table>
+</blockquote>
+
+</li>
+
+<li><p>Modify <sref ref="[expected.object.swap]"/> as indicated:</p>
+
+<blockquote>
+<pre>
+constexpr void swap(expected&amp; rhs) noexcept(<i>see below</i>);
+</pre>
+<blockquote>
+<p>
+-1- <i>Constraints</i>: [&hellip;]
+<p/>
+-2- <i>Effects</i>: See Table 64 <sref ref="[tab:expected.object.swap]"/>.
+<p/>
+For the case where <tt>rhs.value()</tt> is <tt>false</tt> and <tt>this-&gt;has_value()</tt> is <tt>true</tt>, equivalent to:
+</p>
+<blockquote>
+<pre>
+if constexpr (is_nothrow_move_constructible_v&lt;E&gt;) {
+  E tmp(std::move(rhs.<i>unex</i>));
+  destroy_at(addressof(rhs.<i>unex</i>));
+  try {
+    construct_at(addressof(rhs.<i>val</i>), std::move(<i>val</i>));
+    destroy_at(addressof(<i>val</i>));
+    construct_at(addressof(<i>unex</i>), std::move(tmp));
+  } catch(...) {
+    construct_at(addressof(rhs.<i>unex</i>), std::move(tmp));
+    throw;
+  }
+} else {
+  remove_cv_t&lt;T&gt; tmp(std::move(<i>val</i>));
+  destroy_at(addressof(<i>val</i>));
+  try {
+    construct_at(addressof(<i>unex</i>), std::move(rhs.<i>unex</i>));
+    destroy_at(addressof(rhs.<i>unex</i>));
+    construct_at(addressof(rhs.<i>val</i>), std::move(tmp));
+  } catch (...) {
+    construct_at(addressof(<i>val</i>), std::move(tmp));
+    throw;
+  }
+}
+<i>has_val</i> = false;
+rhs.<i>has_val</i> = true;
+</pre>
+</blockquote>
+</blockquote>
+</blockquote>
+
+</li>
+
 </ol>
 </resolution>