Skip to content

Commit d4e2246

Browse files
committed
Fix copy_backward for vector<bool> with small storage types
1 parent 2e6402c commit d4e2246

File tree

5 files changed

+303
-11
lines changed

5 files changed

+303
-11
lines changed

libcxx/include/__algorithm/copy_backward.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
5252
difference_type __dn = std::min(static_cast<difference_type>(__last.__ctz_), __n);
5353
__n -= __dn;
5454
unsigned __clz = __bits_per_word - __last.__ctz_;
55-
__storage_type __m = (~__storage_type(0) << (__last.__ctz_ - __dn)) & (~__storage_type(0) >> __clz);
55+
__storage_type __m = std::__middle_mask<__storage_type>(__clz, __last.__ctz_ - __dn);
5656
__storage_type __b = *__last.__seg_ & __m;
5757
*__result.__seg_ &= ~__m;
5858
*__result.__seg_ |= __b;
@@ -69,7 +69,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
6969
__n -= __nw * __bits_per_word;
7070
// do last word
7171
if (__n > 0) {
72-
__storage_type __m = ~__storage_type(0) << (__bits_per_word - __n);
72+
__storage_type __m = std::__leading_mask<__storage_type>(__bits_per_word - __n);
7373
__storage_type __b = *--__last.__seg_ & __m;
7474
*--__result.__seg_ &= ~__m;
7575
*__result.__seg_ |= __b;
@@ -94,12 +94,12 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
9494
difference_type __dn = std::min(static_cast<difference_type>(__last.__ctz_), __n);
9595
__n -= __dn;
9696
unsigned __clz_l = __bits_per_word - __last.__ctz_;
97-
__storage_type __m = (~__storage_type(0) << (__last.__ctz_ - __dn)) & (~__storage_type(0) >> __clz_l);
97+
__storage_type __m = std::__middle_mask<__storage_type>(__clz_l, __last.__ctz_ - __dn);
9898
__storage_type __b = *__last.__seg_ & __m;
9999
unsigned __clz_r = __bits_per_word - __result.__ctz_;
100100
__storage_type __ddn = std::min(__dn, static_cast<difference_type>(__result.__ctz_));
101101
if (__ddn > 0) {
102-
__m = (~__storage_type(0) << (__result.__ctz_ - __ddn)) & (~__storage_type(0) >> __clz_r);
102+
__m = std::__middle_mask<__storage_type>(__clz_r, __result.__ctz_ - __ddn);
103103
*__result.__seg_ &= ~__m;
104104
if (__result.__ctz_ > __last.__ctz_)
105105
*__result.__seg_ |= __b << (__result.__ctz_ - __last.__ctz_);
@@ -112,7 +112,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
112112
// __result.__ctz_ == 0
113113
--__result.__seg_;
114114
__result.__ctz_ = static_cast<unsigned>(-__dn & (__bits_per_word - 1));
115-
__m = ~__storage_type(0) << __result.__ctz_;
115+
__m = std::__leading_mask<__storage_type>(__result.__ctz_);
116116
*__result.__seg_ &= ~__m;
117117
__last.__ctz_ -= __dn + __ddn;
118118
*__result.__seg_ |= __b << (__result.__ctz_ - __last.__ctz_);
@@ -123,7 +123,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
123123
// __result.__ctz_ != 0 || __n == 0
124124
// do middle words
125125
unsigned __clz_r = __bits_per_word - __result.__ctz_;
126-
__storage_type __m = ~__storage_type(0) >> __clz_r;
126+
__storage_type __m = std::__trailing_mask<__storage_type>(__clz_r);
127127
for (; __n >= __bits_per_word; __n -= __bits_per_word) {
128128
__storage_type __b = *--__last.__seg_;
129129
*__result.__seg_ &= ~__m;
@@ -133,11 +133,11 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
133133
}
134134
// do last word
135135
if (__n > 0) {
136-
__m = ~__storage_type(0) << (__bits_per_word - __n);
136+
__m = std::__leading_mask<__storage_type>(__bits_per_word - __n);
137137
__storage_type __b = *--__last.__seg_ & __m;
138138
__clz_r = __bits_per_word - __result.__ctz_;
139139
__storage_type __dn = std::min(__n, static_cast<difference_type>(__result.__ctz_));
140-
__m = (~__storage_type(0) << (__result.__ctz_ - __dn)) & (~__storage_type(0) >> __clz_r);
140+
__m = std::__middle_mask<__storage_type>(__clz_r, __result.__ctz_ - __dn);
141141
*__result.__seg_ &= ~__m;
142142
*__result.__seg_ |= __b >> (__bits_per_word - __result.__ctz_);
143143
__result.__ctz_ = static_cast<unsigned>(((-__dn & (__bits_per_word - 1)) + __result.__ctz_) % __bits_per_word);
@@ -146,7 +146,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> _
146146
// __result.__ctz_ == 0
147147
--__result.__seg_;
148148
__result.__ctz_ = static_cast<unsigned>(-__n & (__bits_per_word - 1));
149-
__m = ~__storage_type(0) << __result.__ctz_;
149+
__m = std::__leading_mask<__storage_type>(__result.__ctz_);
150150
*__result.__seg_ &= ~__m;
151151
*__result.__seg_ |= __b << (__result.__ctz_ - (__bits_per_word - __n - __dn));
152152
}

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: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ __fill_masked_range(_StoragePointer __word, unsigned __ctz, unsigned __clz, bool
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_backward.pass.cpp

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,13 @@
1313
// constexpr OutIter // constexpr after C++17
1414
// copy_backward(InIter first, InIter last, OutIter result);
1515

16+
// XFAIL: FROZEN-CXX03-HEADERS-FIXME
17+
1618
#include <algorithm>
1719
#include <cassert>
1820
#include <vector>
1921

22+
#include "sized_allocator.h"
2023
#include "test_macros.h"
2124
#include "test_iterators.h"
2225
#include "type_algorithms.h"
@@ -111,6 +114,145 @@ TEST_CONSTEXPR_CXX20 bool test() {
111114
assert(test_vector_bool(256));
112115
}
113116

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

0 commit comments

Comments
 (0)