Skip to content

Commit 5d49033

Browse files
committed
New issue from Hewill: "The Standard Library should not use predicates of the form pred(*i) != false"
1 parent c79e6b4 commit 5d49033

File tree

1 file changed

+322
-0
lines changed

1 file changed

+322
-0
lines changed

xml/issue4127.xml

Lines changed: 322 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,322 @@
1+
<?xml version='1.0' encoding='utf-8' standalone='no'?>
2+
<!DOCTYPE issue SYSTEM "lwg-issue.dtd">
3+
4+
<issue num="4127" status="New">
5+
<title>The Standard Library should not use predicates of the form <code>pred(*i) != false</code></title>
6+
<section><sref ref="[func.search.bm]"/><sref ref="[func.search.bmh]"/>
7+
<sref ref="[associative.reqmts.general]"/><sref ref="[list.ops]"/>
8+
<sref ref="[alg.find]"/><sref ref="[alg.find.first.of]"/>
9+
<sref ref="[alg.adjacent.find]"/><sref ref="[alg.count]"/>
10+
<sref ref="[alg.mismatch]"/><sref ref="[alg.search]"/>
11+
<sref ref="[alg.sorting.general]"/></section>
12+
<submitter>Hewill Kang</submitter>
13+
<date>25 Jul 2024</date>
14+
<priority>99</priority>
15+
16+
<discussion>
17+
<p>
18+
The <code><i>boolean-testable</i></code> concept introduced in <paper num="P1964R2"/>
19+
places appropriate constraints on boolean-ish types, so that <code>!pred(x)</code> or
20+
<code>i != last &amp;&amp; pred(*i)</code> always has a valid definition.
21+
<p/>
22+
Subsequently, <paper num="P2167R3"/> applied this concept to <sref ref="[algorithms.requirements]"/>
23+
p6, requiring that <code>decltype(pred(*first))</code> should model <code><i>boolean-testable</i></code>.
24+
<p/>
25+
However, <code><i>boolean-testable</i></code> does not guarantee that
26+
<code>pred(*i) != false</code> is valid, because the type may overload <code>operator==</code>.
27+
It is necessary to replace this form of expression with an appropriate one as we do in
28+
<a href="https://wg21.link/P1964R2#wording">P1964R2</a>.
29+
</p>
30+
31+
<note>2024-07-27; Daniel comments</note>
32+
<p>
33+
I would recommend to add a wrapping "<tt><ins>bool(</ins>pred([&hellip;])<ins>)</ins></tt>" and possibly
34+
even extend to "<tt><ins>bool(</ins>pred([&hellip;])<ins>)</ins></tt> <ins>is <tt>true</tt></ins>"
35+
following our usual wording convention, since an English phrase of the form "if <tt>pred([&hellip;])</tt>"
36+
where <tt>pred([&hellip;])</tt> is potential non-<tt>bool</tt> and the English "if" is not a C++ language
37+
<tt>if</tt> (with built-in conversion semantics) doesn't sound like a meaningful phrase to me.
38+
</p>
39+
</discussion>
40+
41+
<resolution>
42+
<p>
43+
This wording is relative to <paper num="N4986"/>.
44+
</p>
45+
46+
<ol>
47+
<li><p>Modify <sref ref="[func.search.bm]"/> as indicated:</p>
48+
49+
<blockquote>
50+
<pre>
51+
boyer_moore_searcher(RandomAccessIterator1 pat_first,
52+
RandomAccessIterator1 pat_last,
53+
Hash hf = Hash(),
54+
BinaryPredicate pred = BinaryPredicate());
55+
</pre>
56+
<blockquote>
57+
<p>
58+
-1- <i>Preconditions</i>: The value type of <code>RandomAccessIterator1</code> meets the
59+
<i>Cpp17DefaultConstructible</i>, the <i>Cpp17CopyConstructible</i>, and the
60+
<i>Cpp17CopyAssignable</i> requirements.
61+
<p/>
62+
-2- Let <code>V</code> be <code>iterator_traits&lt;RandomAccessIterator1&gt;::value_type</code>.
63+
For any two values <code>A</code> and <code>B</code> of type <code>V</code>, if
64+
<code>pred(A, B) <del>== true</del></code>, then <code>hf(A) == hf(B)</code> is
65+
<code>true</code>.
66+
</p>
67+
[&hellip;]
68+
<p>
69+
-7- <i>Returns</i>: A pair of iterators <code>i</code> and <code>j</code> such that
70+
</p>
71+
<ol style="list-style-type: none">
72+
<li><p>(7.1) &mdash; <code>i</code> is the first iterator in the range [<code>first</code>,
73+
<code>last - (pat_last_ - pat_first_)</code>) such that for every non-negative integer
74+
<code>n</code> less than <code>pat_last_ - pat_first_</code> the following condition holds:
75+
<code>pred(*(i + n), *(pat_first_ + n)) <del>!= false</del></code>, and
76+
</p>
77+
</li>
78+
<li>
79+
<p>[&hellip;]</p>
80+
</li>
81+
</ol>
82+
</blockquote>
83+
</blockquote>
84+
85+
</li>
86+
87+
<li><p>Modify <sref ref="[func.search.bmh]"/> as indicated:</p>
88+
89+
<blockquote>
90+
<pre>
91+
boyer_moore_horspool_searcher(RandomAccessIterator1 pat_first,
92+
RandomAccessIterator1 pat_last,
93+
Hash hf = Hash(),
94+
BinaryPredicate pred = BinaryPredicate());
95+
</pre>
96+
<blockquote>
97+
<p>
98+
-1- <i>Preconditions</i>: The value type of <code>RandomAccessIterator1</code> meets the
99+
<i>Cpp17DefaultConstructible</i>, <i>Cpp17CopyConstructible</i>, and <i>Cpp17CopyAssignable</i>
100+
requirements.
101+
<p/>
102+
-2- Let <code>V</code> be <code>iterator_traits&lt;RandomAccessIterator1&gt;::value_type</code>.
103+
For any two values <code>A</code> and <code>B</code> of type <code>V</code>, if
104+
<code>pred(A, B) <del>== true</del></code>, then <code>hf(A) == hf(B)</code> is <code>true</code>.
105+
</p>
106+
[&hellip;]
107+
<p>
108+
-7- <i>Returns</i>: A pair of iterators <code>i</code> and <code>j</code> such that
109+
</p>
110+
<ol style="list-style-type: none">
111+
<li><p>
112+
(7.1) &mdash; <code>i</code> is the first iterator in the range [<code>first</code>,
113+
<code>last - (pat_last_ - pat_first_)</code>) such that for every non-negative integer <code>n</code>
114+
less than <code>pat_last_ - pat_first_</code> the following condition holds:
115+
<code>pred(*(i + n), *(pat_first_ + n)) <del>!= false</del></code>, and
116+
</p></li>
117+
<li>
118+
<p>[&hellip;]</p>
119+
</li>
120+
</ol>
121+
</blockquote>
122+
</blockquote>
123+
124+
</li>
125+
126+
<li><p>Modify <sref ref="[associative.reqmts.general]"/> as indicated:</p>
127+
128+
<blockquote>
129+
<p>
130+
-3- The phrase "equivalence of keys" means the equivalence relation imposed by the comparison object.
131+
That is, two keys <code>k1</code> and <code>k2</code> are considered to be equivalent if for the
132+
comparison object <code>comp</code>, <code><ins>!</ins>comp(k1, k2) <del>== false</del> &amp;&amp;
133+
<ins>!</ins>comp(k2, k1) <del>== false</del></code>.
134+
</p>
135+
[&hellip;]
136+
<p>
137+
-177- The fundamental property of iterators of associative containers is that they iterate through
138+
the containers in the non-descending order of keys where non-descending is defined by the comparison
139+
that was used to construct them. For any two dereferenceable iterators <code>i</code> and <code>j</code>
140+
such that distance from <code>i</code> to <code>j</code> is positive, the following condition holds:
141+
</p>
142+
<pre><blockquote><ins>!</ins>value_comp(*j, *i) <del>== false</del></blockquote></pre>
143+
<p>
144+
-178- For associative containers with unique keys the stronger condition holds:
145+
</p>
146+
<pre><blockquote>value_comp(*i, *j) <del>!= false</del></blockquote></pre>
147+
</blockquote>
148+
149+
</li>
150+
151+
<li><p>Modify <sref ref="[list.ops]"/> as indicated:</p>
152+
153+
<blockquote>
154+
<pre>
155+
size_type remove(const T&amp; value);
156+
template&lt;class Predicate&gt; size_type remove_if(Predicate pred);
157+
</pre>
158+
<blockquote>
159+
<p>
160+
-15- <i>Effects</i>: Erases all the elements in the list referred to by a list iterator
161+
<code>i</code> for which the following conditions hold: <code>*i == value, pred(*i)
162+
<del>!= false</del></code>. Invalidates only the iterators and references to
163+
the erased elements.
164+
<p/>
165+
-16- <i>Returns</i>: The number of elements erased.
166+
<p/>
167+
-17- <i>Throws</i>: Nothing unless an exception is thrown by <code>*i == value</code> or
168+
<code>pred(*i) <del>!= false</del></code>.
169+
<p/>
170+
[&hellip;]
171+
</p>
172+
</blockquote>
173+
</blockquote>
174+
175+
</li>
176+
177+
<li><p>Modify <sref ref="[alg.find]"/> as indicated:</p>
178+
179+
<blockquote>
180+
<p>
181+
-1- Let <i>E</i> be:
182+
</p>
183+
<ol style="list-style-type: none">
184+
<li>
185+
<p>(1.1) &mdash; <code>*i == value</code> for <code>find</code>;</p>
186+
</li>
187+
<li>
188+
<p>(1.2) &mdash; <code>pred(*i) <del>!= false</del></code> for <code>find_if</code>;</p>
189+
</li>
190+
<li>
191+
<p>(1.3) &mdash; <code><ins>!</ins>pred(*i) <del>== false</del></code> for <code>find_if_not</code>;
192+
</p>
193+
</li>
194+
[&hellip;]
195+
</ol>
196+
</blockquote>
197+
198+
</li>
199+
200+
<li><p>Modify <sref ref="[alg.find.first.of]"/> as indicated:</p>
201+
202+
<blockquote>
203+
<p>
204+
-1- Let <i>E</i> be:
205+
</p>
206+
<ol style="list-style-type: none">
207+
<li><p>
208+
(1.1) &mdash; <code>*i == *j</code> for the overloads with no parameter <code>pred</code>;
209+
</p></li>
210+
<li><p>
211+
(1.2) &mdash; <code>pred(*i, *j) <del>!= false</del></code> for the overloads with a parameter
212+
<code>pred</code> and no parameter <code>proj1</code>;
213+
</p></li>
214+
[&hellip;]
215+
</ol>
216+
</blockquote>
217+
218+
</li>
219+
220+
<li><p>Modify <sref ref="[alg.adjacent.find]"/> as indicated:</p>
221+
222+
<blockquote>
223+
<p>
224+
-1- Let <i>E</i> be:
225+
</p>
226+
<ol style="list-style-type: none">
227+
<li><p>
228+
(1.1) &mdash; <code>*i == *(i + 1)</code> for the overloads with no parameter <code>pred</code>;
229+
</p></li>
230+
<li><p>
231+
(1.2) &mdash; <code>pred(*i, *(i + 1)) <del>!= false</del></code> for the overloads with a
232+
parameter <code>pred</code> and no parameter <code>proj1</code>;
233+
</p></li>
234+
[&hellip;]
235+
</ol>
236+
</blockquote>
237+
238+
</li>
239+
240+
<li><p>Modify <sref ref="[alg.count]"/> as indicated:</p>
241+
242+
<blockquote>
243+
<p>
244+
-1- Let <i>E</i> be:
245+
</p>
246+
<ol style="list-style-type: none">
247+
<li><p>
248+
(1.1) &mdash; <code>*i == value</code> for the overloads with no parameter <code>pred</code>
249+
or <code>proj</code>;
250+
</p></li>
251+
<li><p>
252+
(1.2) &mdash; <code>pred(*i) <del>!= false</del></code> for the overloads with a parameter
253+
<code>pred</code> but no parameter <code>proj</code>;
254+
</p></li>
255+
[&hellip;]
256+
</ol>
257+
</blockquote>
258+
259+
</li>
260+
261+
<li><p>Modify <sref ref="[alg.mismatch]"/> as indicated:</p>
262+
263+
<blockquote>
264+
<p>
265+
-2- Let <i>E</i> be:
266+
</p>
267+
<ol style="list-style-type: none">
268+
<li><p>
269+
(2.1) &mdash; <code>!(*(first1 + n) == *(first2 + n))</code> for the overloads with no
270+
parameter <code>pred</code>;
271+
</p></li>
272+
<li><p>
273+
(2.2) &mdash; <code><ins>!</ins>pred(*(first1 + n), *(first2 + n)) <del>== false</del></code>
274+
for the overloads with a parameter <code>pred</code> and no parameter <code>proj1</code>;
275+
</p></li>
276+
[&hellip;]
277+
</ol>
278+
</blockquote>
279+
280+
</li>
281+
282+
<li><p>Modify <sref ref="[alg.search]"/> as indicated:</p>
283+
284+
<blockquote>
285+
<p>
286+
-1- <i>Returns</i>: The first iterator <code>i</code> in the range [<code>first1</code>,
287+
<code>last1 - (last2-first2)</code>) such that for every non-negative integer <code>n</code>
288+
less than <code>last2 - first2</code> the following corresponding conditions hold:
289+
<code>*(i + n) == *(first2 + n), pred(*(i + n), *(first2 + n)) <del>!= false</del></code>.
290+
Returns <code>first1</code> if [<code>first2</code>, <code>last2</code>) is empty, otherwise
291+
returns <code>last1</code> if no such iterator is found.
292+
</p>
293+
[&hellip;]
294+
<p>
295+
-6- <i>Returns</i>: The first iterator <code>i</code> in the range [<code>first</code>,
296+
<code>last-count</code>) such that for every non-negative integer <code>n</code> less than
297+
<code>count</code> the following corresponding conditions hold: <code>*(i + n) == value,
298+
pred(*(i + n), value) <del>!= false</del></code>. Returns <code>last</code> if no such
299+
iterator is found.
300+
</p>
301+
</blockquote>
302+
303+
</li>
304+
305+
<li><p>Modify <sref ref="[alg.sorting.general]"/> as indicated:</p>
306+
307+
<blockquote>
308+
<p>
309+
-3- For all algorithms that take <code>Compare</code>, there is a version that uses
310+
<code>operator&lt;</code> instead. That is, <code>comp(*i, *j) <del>!= false</del></code>
311+
defaults to <code>*i &lt; *j <del>!= false</del></code>. For algorithms other than those
312+
described in <sref ref="[alg.binary.search]"/>, <code>comp</code> shall induce a strict
313+
weak ordering on the values.
314+
</p>
315+
</blockquote>
316+
317+
</li>
318+
319+
</ol>
320+
</resolution>
321+
322+
</issue>

0 commit comments

Comments
 (0)