|
| 1 | +<?xml version='1.0' encoding='utf-8' standalone='no'?> |
| 2 | +<!DOCTYPE issue SYSTEM "lwg-issue.dtd"> |
| 3 | + |
| 4 | +<issue num="4179" status="New"> |
| 5 | +<title>Wrong range in [alg.search]</title> |
| 6 | +<section><sref ref="[alg.search]"/></section> |
| 7 | +<submitter>Oscar Slotosch</submitter> |
| 8 | +<date>05 Dec 2024</date> |
| 9 | +<priority>99</priority> |
| 10 | + |
| 11 | +<discussion> |
| 12 | +<p> |
| 13 | +Originally reported as editorial request <a href="https://github.com/cplusplus/draft/issues/7474">#7474</a>: |
| 14 | +<p/> |
| 15 | +During the qualification of the C++ STL Validas has pointed me to the following issue: |
| 16 | +<p/> |
| 17 | +The specification of <sref ref="[alg.search]"/> has a wrong range. Currently the range is |
| 18 | +"<tt>[first1, last1 - (last2 - first2))</tt>" (exclusive) but should be |
| 19 | +"<tt>[first1, last1 - (last2 - first2)]</tt>" (inclusive). So please correct the closing ")" to "]". |
| 20 | +Otherwise the last occurrence will not be found. We observed the issue in C++14 and C++17 |
| 21 | +and cppreference.com. |
| 22 | +<p/> |
| 23 | +The implementations do the right thing and work correctly and find even the last occurrence. |
| 24 | +<p/> |
| 25 | +For example in the list `{1, 2, 3, 4, 5}` the pattern `{4, 5}` should be found (obviously). |
| 26 | +In the case the last element is not included it will not be found. |
| 27 | +<p/> |
| 28 | +<a href="https://godbolt.org/z/daMa5nTY9">Demonstration on godbolt</a> shows that the implementation |
| 29 | +is working and finds the pattern. |
| 30 | +</p> |
| 31 | + |
| 32 | +<note>2024-12-07; Daniel comments and provides wording</note> |
| 33 | +<p> |
| 34 | +The mentioned wording issue is present in all previous standard versions, including the |
| 35 | +<a href="https://sgistl.github.io/search.html">SGI STL</a>. It needs to be fixed in all search |
| 36 | +variants specified in <sref ref="[alg.search]"/> (except for the form using a `Searcher`). |
| 37 | +</p> |
| 38 | +</discussion> |
| 39 | + |
| 40 | +<resolution> |
| 41 | +<p> |
| 42 | +This wording is relative to <paper num="N4993"/>. |
| 43 | +</p> |
| 44 | + |
| 45 | +<ol> |
| 46 | +<li><p>Modify <sref ref="[alg.search]"/> as indicated:</p> |
| 47 | + |
| 48 | +<blockquote> |
| 49 | +<pre> |
| 50 | +template<class ForwardIterator1, class ForwardIterator2> |
| 51 | + constexpr ForwardIterator1 |
| 52 | + search(ForwardIterator1 first1, ForwardIterator1 last1, |
| 53 | + ForwardIterator2 first2, ForwardIterator2 last2); |
| 54 | +[…] |
| 55 | +template<class ExecutionPolicy, class ForwardIterator1, class ForwardIterator2, |
| 56 | + class BinaryPredicate> |
| 57 | +ForwardIterator1 |
| 58 | + search(ExecutionPolicy&& exec, |
| 59 | + ForwardIterator1 first1, ForwardIterator1 last1, |
| 60 | + ForwardIterator2 first2, ForwardIterator2 last2, |
| 61 | + BinaryPredicate pred); |
| 62 | +</pre> |
| 63 | +<blockquote> |
| 64 | +<p> |
| 65 | +-1- <i>Returns</i>: The first iterator `i` in the range <tt>[first1, last1 - (last2 - first2)<ins>]</ins><del>)</del></tt> |
| 66 | +such that for every nonnegative integer `n` less than `last2 - first2` the following |
| 67 | +corresponding conditions hold: `*(i + n) == *(first2 + n), pred(*(i + n)`, `*(first2 + n)) != false`. |
| 68 | +Returns `first1` if `[first2, last2)` is empty, otherwise returns `last1` if no such iterator is found. |
| 69 | +<p/> |
| 70 | +-2- […] |
| 71 | +</p> |
| 72 | +</blockquote> |
| 73 | +<pre> |
| 74 | +template<forward_iterator I1, sentinel_for<I1> S1, forward_iterator I2, |
| 75 | + sentinel_for<I2> S2, class Pred = ranges::equal_to, |
| 76 | + class Proj1 = identity, class Proj2 = identity> |
| 77 | + requires indirectly_comparable<I1, I2, Pred, Proj1, Proj2> |
| 78 | + constexpr subrange<I1> |
| 79 | + ranges::search(I1 first1, S1 last1, I2 first2, S2 last2, Pred pred = {}, |
| 80 | + Proj1 proj1 = {}, Proj2 proj2 = {}); |
| 81 | +template<forward_range R1, forward_range R2, class Pred = ranges::equal_to, |
| 82 | + class Proj1 = identity, class Proj2 = identity> |
| 83 | + requires indirectly_comparable<iterator_t<R1>, iterator_t<R2>, Pred, Proj1, Proj2> |
| 84 | + constexpr borrowed_subrange_t<R1> |
| 85 | + ranges::search(R1&& r1, R2&& r2, Pred pred = {}, |
| 86 | + Proj1 proj1 = {}, Proj2 proj2 = {}); |
| 87 | +</pre> |
| 88 | +<blockquote> |
| 89 | +<p> |
| 90 | +-3- <i>Returns</i>: |
| 91 | +</p> |
| 92 | +<ol style="list-style-type: none"> |
| 93 | +<li><p>(3.1) — `{i, i + (last2 - first2)}`, where `i` is the first iterator in the range |
| 94 | +<tt>[first1, last1 - (last2 - first2)<ins>]</ins><del>)</del></tt> such that for every non-negative integer `n` |
| 95 | +less than `last2 - first2` the condition</p> |
| 96 | +<blockquote><pre> |
| 97 | +bool(invoke(pred, invoke(proj1, *(i + n)), invoke(proj2, *(first2 + n)))) |
| 98 | +</pre></blockquote> |
| 99 | +<p>is `true`.</p> |
| 100 | +</li> |
| 101 | +<li><p>(3.2) — Returns `{last1, last1}` if no such iterator exists.</p></li> |
| 102 | +</ol> |
| 103 | +<p> |
| 104 | +-4- […] |
| 105 | +</p> |
| 106 | +</blockquote> |
| 107 | +<pre> |
| 108 | +template<class ForwardIterator, class Size, class T = iterator_traits<ForwardIterator>::value_type> |
| 109 | + constexpr ForwardIterator |
| 110 | + search_n(ForwardIterator first, ForwardIterator last, |
| 111 | + Size count, const T& value); |
| 112 | +[…] |
| 113 | +template<class ExecutionPolicy, class ForwardIterator, class Size, |
| 114 | + class T = iterator_traits<ForwardIterator>::value_type, |
| 115 | + class BinaryPredicate> |
| 116 | + ForwardIterator |
| 117 | + search_n(ExecutionPolicy&& exec, |
| 118 | + ForwardIterator first, ForwardIterator last, |
| 119 | + Size count, const T& value, |
| 120 | + BinaryPredicate pred); |
| 121 | +</pre> |
| 122 | +<blockquote> |
| 123 | +<p> |
| 124 | +-5- […] |
| 125 | +<p/> |
| 126 | +-6- […] |
| 127 | +<p/> |
| 128 | +-7- <i>Returns</i>: The first iterator `i` in the range <tt>[first, last-count<ins>]</ins><del>)</del></tt> |
| 129 | +such that for every non-negative integer `n` less than `count` the condition <tt><i>E</i></tt> is `true`. |
| 130 | +Returns `last` if no such iterator is found. |
| 131 | +<p/> |
| 132 | +-8- […] |
| 133 | +</p> |
| 134 | +</blockquote> |
| 135 | +<pre> |
| 136 | +template<forward_iterator I, sentinel_for<I> S, |
| 137 | + class Pred = ranges::equal_to, class Proj = identity, |
| 138 | + class T = projected_value_t<I, Proj>> |
| 139 | + requires indirectly_comparable<I, const T*, Pred, Proj> |
| 140 | + constexpr subrange<I> |
| 141 | + ranges::search_n(I first, S last, iter_difference_t<I> count, |
| 142 | + const T& value, Pred pred = {}, Proj proj = {}); |
| 143 | +template<forward_range R, class Pred = ranges::equal_to, |
| 144 | + class Proj = identity, class T = projected_value_t<iterator_t<R>, Proj>> |
| 145 | + requires indirectly_comparable<iterator_t<R>, const T*, Pred, Proj> |
| 146 | + constexpr borrowed_subrange_t<R> |
| 147 | + ranges::search_n(R&& r, range_difference_t<R> count, |
| 148 | + const T& value, Pred pred = {}, Proj proj = {}); |
| 149 | +</pre> |
| 150 | +<blockquote> |
| 151 | +<p> |
| 152 | +-9- <i>Returns</i>: `{i, i + count}` where `i` is the first iterator in the range |
| 153 | +<tt>[first, last - count<ins>]</ins><del>)</del></tt> such that for every non-negative integer |
| 154 | +`n` less than `count`, the following condition holds: `invoke(pred, invoke(proj, *(i + n)), value)`. |
| 155 | +Returns `{last, last}` if no such iterator is found. |
| 156 | +<p/> |
| 157 | +-10- […] |
| 158 | +</p> |
| 159 | +</blockquote> |
| 160 | +</blockquote> |
| 161 | + |
| 162 | +</li> |
| 163 | + |
| 164 | +</ol> |
| 165 | +</resolution> |
| 166 | + |
| 167 | +</issue> |
0 commit comments