Skip to content

Commit 4070e2a

Browse files
committed
New P/R for 4121
1 parent 844da8d commit 4070e2a

File tree

1 file changed

+82
-3
lines changed

1 file changed

+82
-3
lines changed

xml/issue4121.xml

Lines changed: 82 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,7 @@ for associative containers, because `c.end()` might not be a good hint."
5050
"It might be suboptimal, but it still works."
5151
</p>
5252

53-
</discussion>
54-
55-
<resolution>
53+
<superseded>
5654
<p>
5755
This wording is relative to <paper num="N4986"/>.
5856
</p>
@@ -96,6 +94,87 @@ constexpr auto <i>container-append</i>(Container&amp; c) { <i>// exposition
9694

9795
</li>
9896
</ol>
97+
</superseded>
98+
99+
<note>2025-03-13; Jonathan provides improved wording for Tim's suggestion</note>
100+
<p>
101+
It's true that for some cases it might be optimal to use `c.emplace(ref)`
102+
instead of `c.emplace_hint(c.end(), ref)` but I don't think I care.
103+
A bad hint is not expensive, it's just an extra comparison then the hint
104+
is ignored.
105+
And this code path isn't going to be used for `std::set` or `std::map`,
106+
only for user-defined associative containers that don't have a `from_range_t`
107+
constructor or a `C(Iter,Sent)` constructor.
108+
I think just fixing the original issue is all we need,
109+
rather than trying to handle every possible way to insert elements.
110+
</p>
111+
<p>
112+
This is a simpler, portable reproducer that doesn't depend on the current
113+
implementation status of `std::set` in libstdc++:
114+
</p>
115+
<blockquote><pre><code>
116+
#include &lt;ranges&gt;
117+
#include &lt;set&gt;
118+
struct Set : std::set&lt;int&gt; {
119+
Set() { }; // No other constructors
120+
};
121+
int main() {
122+
int a[1];
123+
auto okay = std::ranges::to&lt;std::set&lt;int&gt;&gt;(a);
124+
auto ohno = std::ranges::to&lt;Set&gt;(a);
125+
}
126+
</code></pre></blockquote>
127+
128+
</discussion>
129+
130+
<resolution>
131+
<p>
132+
This wording is relative to <paper num="N5001"/>.
133+
</p>
134+
135+
<ol>
136+
<li><p>Modify <sref ref="[range.utility.conv.general]"/> as indicated:</p>
137+
138+
<blockquote>
139+
<p>
140+
-4- Let <code><i>container-appendable</i></code> be defined as follows:
141+
</p>
142+
<blockquote><pre>
143+
template&lt;class Container, class Ref&gt;
144+
constexpr bool <i>container-appendable</i> = <i>// exposition only</i>
145+
requires(Container&amp; c, Ref&amp;&amp; ref) {
146+
requires (requires { c.emplace_back(std::forward&lt;Ref&gt;(ref)); } ||
147+
requires { c.push_back(std::forward&lt;Ref&gt;(ref)); } ||
148+
<ins>requires { c.emplace_hint(c.end(), std::forward&lt;Ref&gt;(ref)); } ||</ins>
149+
requires { c.emplace(c.end(), std::forward&lt;Ref&gt;(ref)); } ||
150+
requires { c.insert(c.end(), std::forward&lt;Ref&gt;(ref)); });
151+
};
152+
</pre></blockquote>
153+
<p>
154+
-5- Let <code><i>container-append</i></code> be defined as follows:
155+
</p>
156+
<blockquote><pre>
157+
template&lt;class Container&gt;
158+
constexpr auto <i>container-append</i>(Container&amp; c) { <i>// exposition only</i>
159+
return [&amp;c]&lt;class Ref&gt;(Ref&amp;&amp; ref) {
160+
if constexpr (requires { c.emplace_back(declval&lt;Ref&gt;()); })
161+
c.emplace_back(std::forward&lt;Ref&gt;(ref));
162+
else if constexpr (requires { c.push_back(declval&lt;Ref&gt;()); })
163+
c.push_back(std::forward&lt;Ref&gt;(ref));
164+
<ins>else if constexpr (requires { c.emplace_hint(c.end(), declval&lt;Ref&gt;()); })
165+
c.emplace_hint(c.end(), std::forward&lt;Ref&gt;(ref));</ins>
166+
else if constexpr (requires { c.emplace(c.end(), declval&lt;Ref&gt;()); })
167+
c.emplace(c.end(), std::forward&lt;Ref&gt;(ref));
168+
else
169+
c.insert(c.end(), std::forward&lt;Ref&gt;(ref));
170+
};
171+
};
172+
</pre></blockquote>
173+
</blockquote>
174+
175+
</li>
176+
</ol>
177+
99178
</resolution>
100179

101180
</issue>

0 commit comments

Comments
 (0)