Skip to content

Commit 3be69af

Browse files
Dani-HubDani-Hub
committed
New issue from Stephen Howe: "Complexity of inplace_merge() is incorrect"
1 parent 6c14394 commit 3be69af

File tree

1 file changed

+128
-0
lines changed

1 file changed

+128
-0
lines changed

‎xml/issue4196.xml‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
<?xml version='1.0' encoding='utf-8' standalone='no'?>
2+
<!DOCTYPE issue SYSTEM "lwg-issue.dtd">
3+
4+
<issue num="4196" status="New">
5+
<title>Complexity of `inplace_merge()` is incorrect</title>
6+
<section><sref ref="[alg.merge]"/></section>
7+
<submitter>Stephen Howe</submitter>
8+
<date>22 Jan 2025</date>
9+
<priority>99</priority>
10+
11+
<discussion>
12+
<p>
13+
For <paper num="N5001"/>, section <sref ref="[alg.merge]"/> p5, it says for `merge()` <i>Complexity</i> (emphasis mine):
14+
</p>
15+
<blockquote style="border-left: 3px solid #ccc;padding-left: 15px;">
16+
<p>
17+
For the overloads with no `ExecutionPolicy`, <b>at most</b> <tt><i>N</i> - 1</tt> comparisons and applications of each
18+
projection
19+
</p>
20+
</blockquote>
21+
<p>
22+
For <paper num="N5001"/>, section <sref ref="[alg.merge]"/> p11, it says from `inplace_merge()` <i>Complexity</i> (emphasis mine):
23+
</p>
24+
<blockquote style="border-left: 3px solid #ccc;padding-left: 15px;">
25+
<p>
26+
<i>Complexity</i>: Let <tt><i>N</i> = last - first</tt>:
27+
</p>
28+
<ol style="list-style-type: none">
29+
<li><p>(11.1) &mdash; For the overloads with no `ExecutionPolicy`, and if enough additional memory is available, <b>exactly</b>
30+
<tt><i>N</i> - 1</tt> comparisons.</p></li>
31+
<li><p>(11.2) &mdash; Otherwise, <tt>&#x1d4aa;(<i>N</i> log <i>N</i>)</tt> comparisons.</p></li>
32+
</ol>
33+
</blockquote>
34+
<p>
35+
This wording should be (emphasis mine)
36+
</p>
37+
<blockquote>
38+
<p>
39+
<i>Complexity</i>: Let <tt><i>N</i> = last - first</tt>:
40+
</p>
41+
<ol style="list-style-type: none">
42+
<li><p>(11.1) &mdash; For the overloads with no `ExecutionPolicy`, and if enough additional memory is available, <b>at most</b>
43+
<tt><i>N</i> - 1</tt> comparisons.</p></li>
44+
<li><p>(11.2) &mdash; Otherwise, <tt>&#x1d4aa;(<i>N</i> log <i>N</i>)</tt> comparisons.</p></li>
45+
</ol>
46+
</blockquote>
47+
<p>
48+
Consider the 2 sequences in a `std::vector` of `int`s and assume that `inplace_merge` has enough memory:
49+
</p>
50+
<blockquote><pre>
51+
{ 1 }, { 2, 3, 4, 5, 6, 7 )
52+
</pre></blockquote>
53+
<p>
54+
<tt><i>N</i></tt> is `7`, 7 elements. So <tt><i>N</i> - 1</tt> is `6`
55+
<p/>
56+
If you `inplace_merge()` the two sequences, the `1` is compared with `2` and `1` is output. But now the 1st
57+
sequence is over, so the 2nd sequence is copied. Only 1 comparison is done, not 6.
58+
</p>
59+
60+
<note>2025-01-25; Daniel comments</note>
61+
<p>
62+
The <a href="https://sgistl.github.io/inplace_merge.html">SGI STL archive</a> correctly says "at most" as well.
63+
</p>
64+
</discussion>
65+
66+
<resolution>
67+
<p>
68+
This wording is relative to <paper num="N5001"/>.
69+
</p>
70+
71+
<ol>
72+
73+
<li><p>Modify <sref ref="[alg.merge]"/> as indicated:</p>
74+
75+
<blockquote>
76+
<pre>
77+
template&lt;class BidirectionalIterator&gt;
78+
constexpr void inplace_merge(BidirectionalIterator first,
79+
BidirectionalIterator middle,
80+
BidirectionalIterator last);
81+
template&lt;class ExecutionPolicy, class BidirectionalIterator&gt;
82+
void inplace_merge(ExecutionPolicy&amp;&amp; exec,
83+
BidirectionalIterator first,
84+
BidirectionalIterator middle,
85+
BidirectionalIterator last);
86+
template&lt;class BidirectionalIterator, class Compare&gt;
87+
constexpr void inplace_merge(BidirectionalIterator first,
88+
BidirectionalIterator middle,
89+
BidirectionalIterator last, Compare comp);
90+
template&lt;class ExecutionPolicy, class BidirectionalIterator, class Compare&gt;
91+
void inplace_merge(ExecutionPolicy&amp;&amp; exec,
92+
BidirectionalIterator first,
93+
BidirectionalIterator middle,
94+
BidirectionalIterator last, Compare comp);
95+
template&lt;bidirectional_iterator I, sentinel_for&lt;I&gt; S, class Comp = ranges::less,
96+
class Proj = identity&gt;
97+
requires sortable&lt;I, Comp, Proj&gt;
98+
constexpr I ranges::inplace_merge(I first, I middle, S last, Comp comp = {}, Proj proj = {});
99+
</pre>
100+
<blockquote>
101+
<p>
102+
-7- [&hellip;]
103+
<p/>
104+
-8- <i>Preconditions</i>: [&hellip;]
105+
<p/>
106+
-9- <i>Effects</i>: Merges two sorted consecutive ranges `[first, middle)` and `[middle, last)`,
107+
putting the result of the merge into the range `[first, last)`. The resulting range is sorted
108+
with respect to `comp` and `proj`.
109+
<p/>
110+
-10- <i>Returns</i>: `last` for the overload in namespace `ranges`.
111+
<p/>
112+
-11- <i>Complexity</i>: Let <tt><i>N</i> = last - first</tt>:
113+
</p>
114+
<ol style="list-style-type: none">
115+
<li><p>(11.1) &mdash; For the overloads with no `ExecutionPolicy`, and if enough additional memory is available,
116+
<del>exactly</del><ins>at most</ins> <tt><i>N</i> - 1</tt> comparisons.</p></li>
117+
<li><p>(11.2) &mdash; Otherwise, <tt>&#x1d4aa;(<i>N</i> log <i>N</i>)</tt> comparisons.</p></li>
118+
</ol>
119+
<p>
120+
In either case, twice as many projections as comparisons.
121+
</p>
122+
</blockquote>
123+
</blockquote>
124+
</li>
125+
</ol>
126+
</resolution>
127+
128+
</issue>

0 commit comments

Comments
 (0)