Skip to content

Commit a53a235

Browse files
committed
Ensure converstion to bool is checked
1 parent 8dd5a73 commit a53a235

File tree

7 files changed

+53
-80
lines changed

7 files changed

+53
-80
lines changed

libcxx/docs/ReleaseNotes/21.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ Improvements and New Features
8484
- The ``flat_map::insert`` and ``flat_set::insert_range`` have been optimized, resulting in a performance improvement of up
8585
to 10x for inserting elements into a ``flat_map`` when the input range is a ``flat_map`` or a ``zip_view``.
8686

87+
- The ``std::copy`` and ``std::ranges::copy`` algorithms have been optimized for copying from ``random_access_iterator``s
88+
to ``std::vector<bool>::iterator``, resulting in a performance improvement of up to 3x. As a result, range-based
89+
operations of ``std::vector<bool>``, including construction, ``assign_range``, ``insert_range``, ``append_range``, and
90+
their iterator-pair counterparts, have also benefited from a similar 3x speedup.
91+
8792
Deprecations and Removals
8893
-------------------------
8994

libcxx/include/__algorithm/copy.h

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,11 @@ struct __copy_impl {
250250
__storage_type __w = *__result.__seg_;
251251
__storage_type __m = std::__middle_mask<__storage_type>(__clz - __dn, __result.__ctz_);
252252
__w &= ~__m;
253-
for (__storage_type __i = 0; __i < __dn; ++__i, ++__first)
254-
__w |= static_cast<__storage_type>(*__first) << __result.__ctz_++;
253+
for (__storage_type __i = 0; __i < __dn; ++__i, ++__first) {
254+
if (*__first)
255+
__w |= static_cast<__storage_type>(1) << (__result.__ctz_ + __i);
256+
}
257+
__result.__ctz_ += __dn;
255258
*__result.__seg_ = __w;
256259
if (__result.__ctz_ == __bits_per_word) {
257260
__result.__ctz_ = 0;
@@ -265,15 +268,19 @@ struct __copy_impl {
265268
__n -= __nw * __bits_per_word;
266269
for (; __nw; --__nw) {
267270
__storage_type __w = 0;
268-
for (__storage_type __i = 0; __i < __bits_per_word; ++__i, ++__first)
269-
__w |= static_cast<__storage_type>(*__first) << __i;
271+
for (__storage_type __i = 0; __i < __bits_per_word; ++__i, ++__first) {
272+
if (*__first)
273+
__w |= static_cast<__storage_type>(1) << __i;
274+
}
270275
*__result.__seg_++ = __w;
271276
}
272277
// do last partial word, if present
273278
if (__n) {
274279
__storage_type __w = 0;
275-
for (__storage_type __i = 0; __i < __n; ++__i, ++__first)
276-
__w |= static_cast<__storage_type>(*__first) << __i;
280+
for (__storage_type __i = 0; __i < __n; ++__i, ++__first) {
281+
if (*__first)
282+
__w |= static_cast<__storage_type>(1) << __i;
283+
}
277284
__storage_type __m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
278285
*__result.__seg_ &= ~__m;
279286
*__result.__seg_ |= __w;

libcxx/test/benchmarks/algorithms/modifying/copy.bench.cpp

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -90,30 +90,6 @@ int main(int argc, char** argv) {
9090
#endif
9191
}
9292

93-
// {std,ranges}::copy(random_access_iterator, random_access_iterator, vector<bool>)
94-
{
95-
auto bm = []<template <class> class Iter>(std::string name, auto copy) {
96-
benchmark::RegisterBenchmark(name, [copy](auto& st) {
97-
std::size_t const n = st.range(0);
98-
std::vector<int> in(n, 1);
99-
std::vector<bool> out(n);
100-
auto first = Iter(in.begin());
101-
auto last = Iter(in.end());
102-
auto dst = out.begin();
103-
for ([[maybe_unused]] auto _ : st) {
104-
benchmark::DoNotOptimize(in);
105-
benchmark::DoNotOptimize(out);
106-
auto result = copy(first, last, dst);
107-
benchmark::DoNotOptimize(result);
108-
}
109-
})->Range(64, 1 << 20);
110-
};
111-
bm.operator()<random_access_iterator>("std::copy(random_access_iterator, vector<bool>)", std_copy);
112-
#if TEST_STD_VER >= 23 // vector<bool>::iterator is not an output_iterator before C++23
113-
bm.operator()<random_access_iterator>("rng::copy(random_access_iterator, vector<bool>)", std::ranges::copy);
114-
#endif
115-
}
116-
11793
benchmark::Initialize(&argc, argv);
11894
benchmark::RunSpecifiedBenchmarks();
11995
benchmark::Shutdown();

libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -530,13 +530,13 @@ void sequence_container_benchmarks(std::string container) {
530530

531531
#if defined(__cpp_lib_containers_ranges) && __cpp_lib_containers_ranges >= 202202L
532532
{ // Range-ctor
533-
auto bm = [&generators, &bench_vb, &tostr]<template <class, class> class Range>(std::string range) {
533+
auto bm = [&generators, &bench_vb, &tostr]<template <class> class Iter>(std::string range) {
534534
for (auto gen : generators)
535535
bench_vb("ctor(" + range + ")" + tostr(gen), [gen](auto& st) {
536536
auto const size = st.range(0);
537537
std::vector<int> in;
538538
std::generate_n(std::back_inserter(in), size, gen);
539-
Range rg(std::ranges::begin(in), std::ranges::end(in));
539+
std::ranges::subrange rg(Iter(std::ranges::begin(in)), Iter(std::ranges::end(in)));
540540
benchmark::DoNotOptimize(in);
541541

542542
for ([[maybe_unused]] auto _ : st) {
@@ -545,18 +545,18 @@ void sequence_container_benchmarks(std::string container) {
545545
}
546546
});
547547
};
548-
bm.template operator()<random_access_range_wrapper>("ra_range");
548+
bm.template operator()<cpp20_random_access_iterator>("ra_range");
549549
}
550550
{ // Range-assignment
551-
auto bm = [&generators, &bench_vb, &tostr]<template <class, class> class Range>(std::string range) {
551+
auto bm = [&generators, &bench_vb, &tostr]<template <class> class Iter>(std::string range) {
552552
for (auto gen : generators)
553553
bench_vb("assign_range(" + range + ")" + tostr(gen), [gen](auto& st) {
554554
auto const size = st.range(0);
555555
std::vector<int> in1, in2;
556556
std::generate_n(std::back_inserter(in1), size, gen);
557557
std::generate_n(std::back_inserter(in2), size, gen);
558-
Range rg1(std::ranges::begin(in1), std::ranges::end(in1));
559-
Range rg2(std::ranges::begin(in2), std::ranges::end(in2));
558+
std::ranges::subrange rg1(Iter(std::ranges::begin(in1)), Iter(std::ranges::end(in1)));
559+
std::ranges::subrange rg2(Iter(std::ranges::begin(in2)), Iter(std::ranges::end(in2)));
560560
DoNotOptimizeData(in1);
561561
DoNotOptimizeData(in2);
562562

@@ -570,49 +570,49 @@ void sequence_container_benchmarks(std::string container) {
570570
}
571571
});
572572
};
573-
bm.template operator()<random_access_range_wrapper>("ra_range");
573+
bm.template operator()<cpp20_random_access_iterator>("ra_range");
574574
}
575575
{ // Range-insertion
576-
auto bm = [&generators, &bench_vb, &tostr]<template <class, class> class Range>(std::string range) {
576+
auto bm = [&generators, &bench_vb, &tostr]<template <class> class Iter>(std::string range) {
577577
for (auto gen : generators)
578578
bench_vb("insert_range(" + range + ")" + tostr(gen), [gen](auto& st) {
579579
auto const size = st.range(0);
580580
std::vector<int> in;
581581
Container c;
582582
std::generate_n(std::back_inserter(in), size, gen);
583583
std::generate_n(std::back_inserter(c), size, gen);
584-
Range rg(std::ranges::begin(in), std::ranges::end(in));
584+
std::ranges::subrange rg(Iter(std::ranges::begin(in)), Iter(std::ranges::end(in)));
585585
DoNotOptimizeData(in);
586586
DoNotOptimizeData(c);
587587

588588
for ([[maybe_unused]] auto _ : st) {
589-
c.insert_range(c.begin(), in);
589+
c.insert_range(c.begin(), rg);
590590
c.erase(c.begin() + size, c.end()); // avoid growing indefinitely
591591
DoNotOptimizeData(c);
592592
}
593593
});
594594
};
595-
bm.template operator()<random_access_range_wrapper>("ra_range");
595+
bm.template operator()<cpp20_random_access_iterator>("ra_range");
596596
}
597597
{ // Range-append
598-
auto bm = [&generators, &bench_vb, &tostr]<template <class, class> class Range>(std::string range) {
598+
auto bm = [&generators, &bench_vb, &tostr]<template <class> class Iter>(std::string range) {
599599
for (auto gen : generators)
600600
bench_vb("append_range(" + range + ")" + tostr(gen), [gen](auto& st) {
601601
auto const size = st.range(0);
602602
std::vector<int> in;
603603
std::generate_n(std::back_inserter(in), size, gen);
604-
Range rg(std::ranges::begin(in), std::ranges::end(in));
604+
std::ranges::subrange rg(Iter(std::ranges::begin(in)), Iter(std::ranges::end(in)));
605605
DoNotOptimizeData(in);
606606

607607
Container c;
608608
for ([[maybe_unused]] auto _ : st) {
609-
c.append_range(in);
609+
c.append_range(rg);
610610
c.erase(c.begin(), c.end()); // avoid growing indefinitely
611611
DoNotOptimizeData(c);
612612
}
613613
});
614614
};
615-
bm.template operator()<random_access_range_wrapper>("ra_range");
615+
bm.template operator()<cpp20_random_access_iterator>("ra_range");
616616
}
617617
#endif
618618
}

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -140,12 +140,12 @@ test_segmented_iterator() { // TODO: Mark as TEST_CONSTEXPR_CXX26 once std::dequ
140140
assert(in == out);
141141
}
142142
{ // Copy from segmented input to vector<bool> output
143-
std::deque<bool> in(199, false);
144-
for (std::size_t i = 0; i < in.size(); i += 2)
145-
in[i] = true;
143+
int a[] = {8, 4, 2, 1, 0};
144+
std::deque<int> in(a, a + sizeof(a) / sizeof(int));
146145
std::vector<bool> out(in.size());
147146
std::copy(in.begin(), in.end(), out.begin());
148-
assert(std::equal(in.begin(), in.end(), out.begin()));
147+
for (std::deque<int>::size_type i = 0; i != in.size(); ++i)
148+
assert(out[i] == (in[i] != 0));
149149
}
150150
{ // Copy from vector<bool> input to segmented output
151151
std::vector<bool> in(199, false);
@@ -175,12 +175,14 @@ test_segmented_iterator() { // TODO: Mark as TEST_CONSTEXPR_CXX26 once std::dequ
175175
assert(out == expected);
176176
}
177177
{ // Copy from segmented input to vector<bool> output
178-
std::vector<std::vector<int>> v{{1, 1}, {1, 0, 1}, {0, 0}, {1, 1, 1}, {1}, {1, 1, 1, 1}, {0, 0, 1, 1, 0, 1, 1}};
178+
std::vector<std::vector<int>> v{{1, 2}, {1, 2, 3}, {0, 0}, {3, 4, 5}, {6}, {7, 8, 9, 6}, {0, 1, 2, 3, 0, 1, 2}};
179179
auto jv = std::ranges::join_view(v);
180-
std::vector<bool> expected(jv.begin(), jv.end());
181-
std::vector<bool> out(expected.size());
180+
std::vector<bool> out(std::ranges::distance(jv.begin(), jv.end()));
182181
std::copy(jv.begin(), jv.end(), out.begin());
183-
assert(out == expected);
182+
std::size_t i = 0;
183+
for (auto it = jv.begin(); it != jv.end(); ++it, ++i)
184+
assert(out[i] == (*it != 0));
185+
assert(i == out.size());
184186
}
185187
#endif
186188
}

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy.pass.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,12 @@ test_segmented_iterator() { // TODO: Mark as TEST_CONSTEXPR_CXX26 once std::dequ
177177
}
178178
#if TEST_STD_VER >= 23
179179
{ // Copy from segmented input to vector<bool> output
180-
std::deque<bool> in(199, false);
181-
for (std::size_t i = 0; i < in.size(); i += 2)
182-
in[i] = true;
180+
int a[] = {8, 4, 2, 1, 0};
181+
std::deque<int> in(a, a + sizeof(a) / sizeof(int));
183182
std::vector<bool> out(in.size());
184183
std::ranges::copy(in, out.begin());
185-
assert(std::equal(in.begin(), in.end(), out.begin()));
184+
for (std::deque<int>::size_type i = 0; i != in.size(); ++i)
185+
assert(out[i] == (in[i] != 0));
186186
}
187187
#endif // TEST_STD_VER >= 23
188188
{ // Copy from vector<bool> input to segmented output
@@ -215,12 +215,14 @@ test_segmented_iterator() { // TODO: Mark as TEST_CONSTEXPR_CXX26 once std::dequ
215215
#endif // TEST_STD_VER >= 20
216216
#if TEST_STD_VER >= 23
217217
{ // Copy from segmented input to vector<bool> output
218-
std::vector<std::vector<int>> v{{1, 1}, {1, 0, 1}, {0, 0}, {1, 1, 1}, {1}, {1, 1, 1, 1}, {0, 0, 1, 1, 0, 1, 1}};
218+
std::vector<std::vector<int>> v{{1, 2}, {1, 2, 3}, {0, 0}, {3, 4, 5}, {6}, {7, 8, 9, 6}, {0, 1, 2, 3, 0, 1, 2}};
219219
auto jv = std::ranges::join_view(v);
220-
std::vector<bool> expected(jv.begin(), jv.end());
221-
std::vector<bool> out(expected.size());
220+
std::vector<bool> out(std::ranges::distance(jv.begin(), jv.end()));
222221
std::ranges::copy(jv, out.begin());
223-
assert(out == expected);
222+
std::size_t i = 0;
223+
for (auto it = jv.begin(); it != jv.end(); ++it, ++i)
224+
assert(out[i] == (*it != 0));
225+
assert(i == out.size());
224226
}
225227
#endif // TEST_STD_VER >= 23
226228
}

libcxx/test/std/containers/from_range_helpers.h

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -50,25 +50,6 @@ constexpr auto wrap_input(std::vector<T>& input) {
5050
return std::ranges::subrange(std::move(b), std::move(e));
5151
}
5252

53-
template <class Iter, class Sent>
54-
class random_access_range_wrapper {
55-
public:
56-
using _Iter = cpp20_random_access_iterator<Iter>;
57-
using _Sent = sized_sentinel<_Iter>;
58-
59-
random_access_range_wrapper() = default;
60-
random_access_range_wrapper(Iter begin, Iter end) : begin_(std::move(begin)), end_(std::move(end)) {}
61-
_Iter begin() { return _Iter(std::move(begin_)); }
62-
_Sent end() { return _Sent(_Iter(std::move(end_))); }
63-
64-
private:
65-
Iter begin_;
66-
Sent end_;
67-
};
68-
69-
template <class Iter, class Sent>
70-
random_access_range_wrapper(Iter, Sent) -> random_access_range_wrapper<Iter, Sent>;
71-
7253
struct KeyValue {
7354
int key; // Only the key is considered for equality comparison.
7455
char value; // Allows distinguishing equivalent instances.

0 commit comments

Comments
 (0)