Skip to content

Commit 3d1a1b7

Browse files
committed
New issue from Hewill: "optional::value_or return statement is inconsistent with Mandates"
1 parent dae4666 commit 3d1a1b7

File tree

1 file changed

+163
-0
lines changed

1 file changed

+163
-0
lines changed

xml/issue4406.xml

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
<?xml version='1.0' encoding='utf-8' standalone='no'?>
2+
<!DOCTYPE issue SYSTEM "lwg-issue.dtd">
3+
4+
<issue num="4406" status="New">
5+
<title>`optional::value_or` return statement is inconsistent with <i>Mandates</i></title>
6+
<section>
7+
<sref ref="[optional.observe]"/>
8+
<sref ref="[optional.ref.observe]"/>
9+
<sref ref="[expected.object.obs]"/>
10+
</section>
11+
<submitter>Hewill Kang</submitter>
12+
<date>06 Sep 2025</date>
13+
<priority>99</priority>
14+
15+
<discussion>
16+
<p>
17+
<code>optional&lt;T&gt;::value_or(U&amp;&amp;)</code> requires <code>is_convertible_v&lt;U&amp;&amp;, T&gt;</code>
18+
to ensure that `T` can be convert from `U` when `optional` has no value.
19+
<p/>
20+
However, the return statement explicitly constructs `T` by `static_cast`, which is not checked by
21+
`is_convertible_v` since it only checks for implicit conversions.
22+
<p/>
23+
This results in rare cases where <i>Mandates</i> may not be violated, but `value_or` is ill-formed
24+
(<a href="https://godbolt.org/z/z5jjhY7c4">demo</a>):
25+
</p>
26+
<blockquote><pre>
27+
struct S {
28+
operator int() const;
29+
explicit operator int() = delete;
30+
};
31+
32+
int main() {
33+
std::optional&lt;int&gt;{}.value_or(S{}); // <span style="color:#C80000;font-weight:bold">fire</span>
34+
}
35+
</pre></blockquote>
36+
<p>
37+
It is reasonable to create objects that stick to <i>Mandates</i>. The same goes for `expected::value_or`.
38+
<p/>
39+
<b>Daniel:</b>
40+
<p/>
41+
This issue has considerable overlap with LWG <iref ref="4281"/>.
42+
</p>
43+
</discussion>
44+
45+
<resolution>
46+
<p>
47+
This wording is relative to <paper num="N5014"/>.
48+
</p>
49+
50+
<ol>
51+
52+
<li><p>Modify <sref ref="[optional.observe]"/> as indicated:</p>
53+
54+
<blockquote>
55+
<pre>
56+
template&lt;class U = remove_cv_t&lt;T&gt;&gt; constexpr T value_or(U&amp;&amp; v) const &amp;;
57+
</pre>
58+
<blockquote>
59+
<p>
60+
-15- <i>Mandates</i>: <code>is_copy_constructible_v&lt;T&gt; &amp;&amp; is_convertible_v&lt;U&amp;&amp;, T&gt;</code>
61+
is `true`.
62+
<p/>
63+
-16- <i>Effects</i>: Equivalent to:
64+
</p>
65+
<blockquote><pre>
66+
<del>return has_value() ? **this : static_cast&lt;T&gt;(std::forward&lt;U&gt;(v));</del>
67+
<ins>if (has_value())
68+
return **this;
69+
return std::forward&lt;U&gt;(v);</ins>
70+
</pre></blockquote>
71+
</blockquote>
72+
<pre>
73+
template&lt;class U = remove_cv_t&lt;T&gt;&gt; constexpr T value_or(U&amp;&amp; v) &amp;&amp;;
74+
</pre>
75+
<blockquote>
76+
<p>
77+
-17- <i>Mandates</i>: <code>is_move_constructible_v&lt;T&gt; &amp;&amp; is_convertible_v&lt;U&amp;&amp;, T&gt;</code>
78+
is `true`.
79+
<p/>
80+
-18- <i>Effects</i>: Equivalent to:
81+
</p>
82+
<blockquote><pre>
83+
<del>return has_value() ? std::move(**this) : static_cast&lt;T&gt;(std::forward&lt;U&gt;(v));</del>
84+
<ins>if (has_value())
85+
return std::move(**this);
86+
return std::forward&lt;U&gt;(v);</ins>
87+
</pre></blockquote>
88+
</blockquote>
89+
</blockquote>
90+
</li>
91+
92+
<li><p>Modify <sref ref="[optional.ref.observe]"/> as indicated:</p>
93+
94+
<blockquote>
95+
<pre>
96+
template&lt;class U = remove_cv_t&lt;T&gt;&gt; constexpr remove_cv_t&lt;T&gt; value_or(U&amp;&amp; u) const;
97+
</pre>
98+
<blockquote>
99+
<p>
100+
-8- Let <code>X</code> be <code>remove_cv_t&lt;T&gt;</code>.
101+
<p/>
102+
-9- <i>Mandates</i>: <code>is_constructible_v&lt;X, T&amp;&gt; &amp;&amp; is_convertible_v&lt;U, X&gt;</code>
103+
is `true`.
104+
<p/>
105+
-10- <i>Effects</i>: Equivalent to:
106+
</p>
107+
<blockquote><pre>
108+
<del>return has_value() ? *<i>val</i> : static_cast&lt;X&gt;(std::forward&lt;U&gt;(u));</del>
109+
<ins>if (has_value())
110+
return *<i>val</i>;
111+
return std::forward&lt;U&gt;(u);
112+
</ins>
113+
</pre></blockquote>
114+
</blockquote>
115+
</blockquote>
116+
</li>
117+
118+
<li><p>Modify <sref ref="[expected.object.obs]"/> as indicated:</p>
119+
120+
<blockquote>
121+
<pre>
122+
template&lt;class U = remove_cv_t&lt;T&gt;&gt; constexpr T value_or(U&amp;&amp; v) const &amp;;
123+
</pre>
124+
<blockquote>
125+
<p>
126+
-18- <i>Mandates</i>: <code>is_copy_constructible_v&lt;T&gt;</code> is `true` and
127+
<code>is_convertible_v&lt;U, T&gt;</code> is `true`.
128+
<p/>
129+
<del>-19- <i>Returns</i>: <code>has_value() ? **this : static_cast&lt;T&gt;(std::forward&lt;U&gt;(v))</code>.</del>
130+
<p/>
131+
<ins>-?- <i>Effects</i>: Equivalent to:</ins>
132+
</p>
133+
<blockquote><pre>
134+
<ins>if (has_value())
135+
return **this;
136+
return std::forward&lt;U&gt;(v);</ins>
137+
</pre></blockquote>
138+
</blockquote>
139+
<pre>
140+
template&lt;class U = remove_cv_t&lt;T&gt;&gt; constexpr T value_or(U&amp;&amp; v) &amp;&amp;;
141+
</pre>
142+
<blockquote>
143+
<p>
144+
-20- <i>Mandates</i>: <code>is_move_constructible_v&lt;T&gt;</code> is `true` and
145+
<code>is_convertible_v&lt;U, T&gt;</code> is `true`.
146+
<p/>
147+
<del>-21- <i>Returns</i>: <code>has_value() ? std::move(**this) : static_cast&lt;T&gt;(std::forward&lt;U&gt;(v))</code>.</del>
148+
<p/>
149+
<ins>-?- <i>Effects</i>: Equivalent to:</ins>
150+
</p>
151+
<blockquote><pre>
152+
<ins>if (has_value())
153+
return std::move(**this);
154+
return std::forward&lt;U&gt;(v);</ins>
155+
</pre></blockquote>
156+
</blockquote>
157+
</blockquote>
158+
</li>
159+
</ol>
160+
161+
</resolution>
162+
163+
</issue>

0 commit comments

Comments
 (0)