Skip to content

Commit b316676

Browse files
committed
Fix copy for vector<bool> with small storage types
1 parent c53caae commit b316676

File tree

5 files changed

+316
-12
lines changed

5 files changed

+316
-12
lines changed

libcxx/include/__algorithm/copy.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
5353
unsigned __clz = __bits_per_word - __first.__ctz_;
5454
difference_type __dn = std::min(static_cast<difference_type>(__clz), __n);
5555
__n -= __dn;
56-
__storage_type __m = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz - __dn));
56+
__storage_type __m = std::__middle_mask<__storage_type>(__clz - __dn, __first.__ctz_);
5757
__storage_type __b = *__first.__seg_ & __m;
5858
*__result.__seg_ &= ~__m;
5959
*__result.__seg_ |= __b;
@@ -73,7 +73,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
7373
// do last word
7474
if (__n > 0) {
7575
__first.__seg_ += __nw;
76-
__storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
76+
__storage_type __m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
7777
__storage_type __b = *__first.__seg_ & __m;
7878
*__result.__seg_ &= ~__m;
7979
*__result.__seg_ |= __b;
@@ -98,11 +98,11 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
9898
unsigned __clz_f = __bits_per_word - __first.__ctz_;
9999
difference_type __dn = std::min(static_cast<difference_type>(__clz_f), __n);
100100
__n -= __dn;
101-
__storage_type __m = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
101+
__storage_type __m = std::__middle_mask<__storage_type>(__clz_f - __dn, __first.__ctz_);
102102
__storage_type __b = *__first.__seg_ & __m;
103103
unsigned __clz_r = __bits_per_word - __result.__ctz_;
104104
__storage_type __ddn = std::min<__storage_type>(__dn, __clz_r);
105-
__m = (~__storage_type(0) << __result.__ctz_) & (~__storage_type(0) >> (__clz_r - __ddn));
105+
__m = std::__middle_mask<__storage_type>(__clz_r - __ddn, __result.__ctz_);
106106
*__result.__seg_ &= ~__m;
107107
if (__result.__ctz_ > __first.__ctz_)
108108
*__result.__seg_ |= __b << (__result.__ctz_ - __first.__ctz_);
@@ -112,7 +112,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
112112
__result.__ctz_ = static_cast<unsigned>((__ddn + __result.__ctz_) % __bits_per_word);
113113
__dn -= __ddn;
114114
if (__dn > 0) {
115-
__m = ~__storage_type(0) >> (__bits_per_word - __dn);
115+
__m = std::__trailing_mask<__storage_type>(__bits_per_word - __dn);
116116
*__result.__seg_ &= ~__m;
117117
*__result.__seg_ |= __b >> (__first.__ctz_ + __ddn);
118118
__result.__ctz_ = static_cast<unsigned>(__dn);
@@ -123,7 +123,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
123123
// __first.__ctz_ == 0;
124124
// do middle words
125125
unsigned __clz_r = __bits_per_word - __result.__ctz_;
126-
__storage_type __m = ~__storage_type(0) << __result.__ctz_;
126+
__storage_type __m = std::__leading_mask<__storage_type>(__result.__ctz_);
127127
for (; __n >= __bits_per_word; __n -= __bits_per_word, ++__first.__seg_) {
128128
__storage_type __b = *__first.__seg_;
129129
*__result.__seg_ &= ~__m;
@@ -134,17 +134,17 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
134134
}
135135
// do last word
136136
if (__n > 0) {
137-
__m = ~__storage_type(0) >> (__bits_per_word - __n);
137+
__m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
138138
__storage_type __b = *__first.__seg_ & __m;
139139
__storage_type __dn = std::min(__n, static_cast<difference_type>(__clz_r));
140-
__m = (~__storage_type(0) << __result.__ctz_) & (~__storage_type(0) >> (__clz_r - __dn));
140+
__m = std::__middle_mask<__storage_type>(__clz_r - __dn, __result.__ctz_);
141141
*__result.__seg_ &= ~__m;
142142
*__result.__seg_ |= __b << __result.__ctz_;
143143
__result.__seg_ += (__dn + __result.__ctz_) / __bits_per_word;
144144
__result.__ctz_ = static_cast<unsigned>((__dn + __result.__ctz_) % __bits_per_word);
145145
__n -= __dn;
146146
if (__n > 0) {
147-
__m = ~__storage_type(0) >> (__bits_per_word - __n);
147+
__m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
148148
*__result.__seg_ &= ~__m;
149149
*__result.__seg_ |= __b >> __dn;
150150
__result.__ctz_ = static_cast<unsigned>(__n);

libcxx/include/__bit_reference

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,20 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __trailing_mask
8282
return static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __clz;
8383
}
8484

85+
// Creates a mask of type `_StorageType` with a specified number of trailing zeros (__ctz) and sets all remaining
86+
// bits to one.
87+
template <class _StorageType>
88+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __leading_mask(unsigned __ctz) {
89+
static_assert(is_unsigned<_StorageType>::value, "__leading_mask only works with unsigned types");
90+
return static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __ctz;
91+
}
92+
8593
// Creates a mask of type `_StorageType` with a specified number of leading zeros (__clz), a specified number of
8694
// trailing zeros (__ctz), and sets all bits in between to one.
8795
template <class _StorageType>
8896
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __middle_mask(unsigned __clz, unsigned __ctz) {
8997
static_assert(is_unsigned<_StorageType>::value, "__middle_mask only works with unsigned types");
90-
return (static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __ctz) &
91-
std::__trailing_mask<_StorageType>(__clz);
98+
return std::__leading_mask<_StorageType>(__ctz) & std::__trailing_mask<_StorageType>(__clz);
9299
}
93100

94101
// This function is designed to operate correctly even for smaller integral types like `uint8_t`, `uint16_t`,

libcxx/include/__fwd/bit_reference.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,14 @@ struct __size_difference_type_traits;
2828

2929
template <class _StoragePointer>
3030
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
31-
__fill_masked_range(_StoragePointer __word, unsigned __ctz, unsigned __clz, bool __fill_val);
31+
__fill_masked_range(_StoragePointer __word, unsigned __clz, unsigned __ctz, bool __fill_val);
3232

3333
template <class _StorageType>
3434
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __trailing_mask(unsigned __clz);
3535

36+
template <class _StorageType>
37+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __leading_mask(unsigned __ctz);
38+
3639
template <class _StorageType>
3740
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __middle_mask(unsigned __clz, unsigned __ctz);
3841

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

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <cassert>
1717
#include <vector>
1818

19+
#include "sized_allocator.h"
1920
#include "test_macros.h"
2021
#include "test_iterators.h"
2122
#include "type_algorithms.h"
@@ -109,6 +110,152 @@ TEST_CONSTEXPR_CXX20 bool test() {
109110
assert(test_vector_bool(256));
110111
}
111112

113+
// Validate std::copy with std::vector<bool> iterators and custom storage types.
114+
// Ensure that assigned bits hold the intended values, while unassigned bits stay unchanged.
115+
{
116+
//// Tests for std::copy with aligned bits
117+
118+
{ // Test the first (partial) word for uint8_t
119+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
120+
std::vector<bool, Alloc> in(8, false, Alloc(1));
121+
std::vector<bool, Alloc> out(8, true, Alloc(1));
122+
std::copy(in.begin() + 1, in.begin() + 2, out.begin() + 1); // out[1] = false
123+
assert(out[1] == false);
124+
for (std::size_t i = 0; i < out.size(); ++i) // Ensure that unassigned bits remain unchanged
125+
if (i != 1)
126+
assert(out[i] == true);
127+
}
128+
{ // Test the last (partial) word for uint8_t
129+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
130+
std::vector<bool, Alloc> in(8, false, Alloc(1));
131+
std::vector<bool, Alloc> out(8, true, Alloc(1));
132+
std::copy(in.begin(), in.begin() + 1, out.begin()); // out[0] = false
133+
assert(out[0] == false);
134+
for (std::size_t i = 1; i < out.size(); ++i) // Ensure that unassigned bits remain unchanged
135+
assert(out[i] == true);
136+
}
137+
{ // Test middle (whole) words for uint8_t
138+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
139+
std::vector<bool, Alloc> in(32, true, Alloc(1));
140+
for (std::size_t i = 0; i < in.size(); i += 2)
141+
in[i] = false;
142+
std::vector<bool, Alloc> out(32, false, Alloc(1));
143+
std::copy(in.begin() + 4, in.end() - 4, out.begin() + 4);
144+
for (std::size_t i = 4; i < static_cast<std::size_t>(in.size() - 4); ++i)
145+
assert(in[i] == out[i]);
146+
for (std::size_t i = 0; i < 4; ++i)
147+
assert(out[i] == false);
148+
for (std::size_t i = 28; i < out.size(); ++i)
149+
assert(out[i] == false);
150+
}
151+
152+
{ // Test the first (partial) word for uint16_t
153+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
154+
std::vector<bool, Alloc> in(16, false, Alloc(1));
155+
std::vector<bool, Alloc> out(16, true, Alloc(1));
156+
std::copy(in.begin() + 1, in.begin() + 3, out.begin() + 1); // out[1..2] = false
157+
assert(out[1] == false);
158+
assert(out[2] == false);
159+
for (std::size_t i = 0; i < out.size(); ++i) // Ensure that unassigned bits remain unchanged
160+
if (i != 1 && i != 2)
161+
assert(out[i] == true);
162+
}
163+
{ // Test the last (partial) word for uint16_t
164+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
165+
std::vector<bool, Alloc> in(16, false, Alloc(1));
166+
std::vector<bool, Alloc> out(16, true, Alloc(1));
167+
std::copy(in.begin(), in.begin() + 2, out.begin()); // out[0..1] = false
168+
assert(out[0] == false);
169+
assert(out[1] == false);
170+
for (std::size_t i = 2; i < out.size(); ++i) // Ensure that unassigned bits remain unchanged
171+
assert(out[i] == true);
172+
}
173+
{ // Test middle (whole) words for uint16_t
174+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
175+
std::vector<bool, Alloc> in(64, true, Alloc(1));
176+
for (std::size_t i = 0; i < in.size(); i += 2)
177+
in[i] = false;
178+
std::vector<bool, Alloc> out(64, false, Alloc(1));
179+
std::copy(in.begin() + 8, in.end() - 8, out.begin() + 8);
180+
for (std::size_t i = 8; i < static_cast<std::size_t>(in.size() - 8); ++i)
181+
assert(in[i] == out[i]);
182+
for (std::size_t i = 0; i < 8; ++i)
183+
assert(out[i] == false);
184+
for (std::size_t i = static_cast<std::size_t>(out.size() - 8); i < out.size(); ++i)
185+
assert(out[i] == false);
186+
}
187+
188+
//// Tests for std::copy with unaligned bits
189+
190+
{ // Test the first (partial) word for uint8_t
191+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
192+
std::vector<bool, Alloc> in(8, false, Alloc(1));
193+
std::vector<bool, Alloc> out(8, true, Alloc(1));
194+
std::copy(in.begin() + 7, in.end(), out.begin()); // out[0] = false
195+
assert(out[0] == false);
196+
for (std::size_t i = 1; i < out.size(); ++i) // Ensure that unassigned bits remain unchanged
197+
assert(out[i] == true);
198+
}
199+
{ // Test the last (partial) word for uint8_t
200+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
201+
std::vector<bool, Alloc> in(8, false, Alloc(1));
202+
std::vector<bool, Alloc> out(8, true, Alloc(1));
203+
std::copy(in.begin(), in.begin() + 1, out.begin() + 2); // out[2] = false
204+
assert(out[2] == false);
205+
for (std::size_t i = 1; i < out.size(); ++i) // Ensure that unassigned bits remain unchanged
206+
if (i != 2)
207+
assert(out[i] == true);
208+
}
209+
{ // Test middle (whole) words for uint8_t
210+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
211+
std::vector<bool, Alloc> in(36, true, Alloc(1));
212+
for (std::size_t i = 0; i < in.size(); i += 2)
213+
in[i] = false;
214+
std::vector<bool, Alloc> out(40, false, Alloc(1));
215+
std::copy(in.begin(), in.end(), out.begin() + 4);
216+
for (std::size_t i = 0; i < in.size(); ++i)
217+
assert(in[i] == out[i + 4]);
218+
for (std::size_t i = 0; i < 4; ++i)
219+
assert(out[i] == false);
220+
}
221+
222+
{ // Test the first (partial) word for uint16_t
223+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
224+
std::vector<bool, Alloc> in(16, false, Alloc(1));
225+
std::vector<bool, Alloc> out(16, true, Alloc(1));
226+
std::copy(in.begin() + 14, in.end(), out.begin()); // out[0..1] = false
227+
assert(out[0] == false);
228+
assert(out[1] == false);
229+
for (std::size_t i = 2; i < out.size(); ++i) // Ensure that unassigned bits remain unchanged
230+
assert(out[i] == true);
231+
}
232+
{ // Test the last (partial) word for uint16_t
233+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
234+
std::vector<bool, Alloc> in(16, false, Alloc(1));
235+
std::vector<bool, Alloc> out(16, true, Alloc(1));
236+
std::copy(in.begin(), in.begin() + 2, out.begin() + 1); // out[1..2] = false
237+
assert(out[1] == false);
238+
assert(out[2] == false);
239+
for (std::size_t i = 0; i < out.size(); ++i) // Ensure that unassigned bits remain unchanged
240+
if (i != 1 && i != 2)
241+
assert(out[i] == true);
242+
}
243+
{ // Test middle (whole) words for uint16_t
244+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
245+
std::vector<bool, Alloc> in(72, true, Alloc(1));
246+
for (std::size_t i = 0; i < in.size(); i += 2)
247+
in[i] = false;
248+
std::vector<bool, Alloc> out(80, false, Alloc(1));
249+
std::copy(in.begin(), in.end(), out.begin() + 4);
250+
for (std::size_t i = 0; i < in.size(); ++i)
251+
assert(in[i] == out[i + 4]);
252+
for (std::size_t i = 0; i < 4; ++i)
253+
assert(out[i] == false);
254+
for (std::size_t i = in.size() + 4; i < out.size(); ++i)
255+
assert(out[i] == false);
256+
}
257+
}
258+
112259
return true;
113260
}
114261

0 commit comments

Comments
 (0)