Skip to content

Commit 0f1d23a

Browse files
committed
Address @ldionne's review comments
1 parent 69dd756 commit 0f1d23a

File tree

4 files changed

+20
-27
lines changed

4 files changed

+20
-27
lines changed

libcxx/include/__algorithm/fill_n.h

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,7 @@ __fill_n_bool(__bit_iterator<_Cp, false> __first, typename __size_difference_typ
4141
if (__first.__ctz_ != 0) {
4242
__storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
4343
__storage_type __dn = std::min(__clz_f, __n);
44-
__storage_type __m = std::__middle_mask<__storage_type>(__first.__ctz_, __clz_f - __dn);
45-
if (_FillVal)
46-
*__first.__seg_ |= __m;
47-
else
48-
*__first.__seg_ &= ~__m;
44+
*__first.__seg_ = std::__fill_range_in_word(*__first.__seg_, __first.__ctz_, __clz_f - __dn, _FillVal);
4945
__n -= __dn;
5046
++__first.__seg_;
5147
}
@@ -56,11 +52,7 @@ __fill_n_bool(__bit_iterator<_Cp, false> __first, typename __size_difference_typ
5652
// do last partial word
5753
if (__n > 0) {
5854
__first.__seg_ += __nw;
59-
__storage_type __m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
60-
if (_FillVal)
61-
*__first.__seg_ |= __m;
62-
else
63-
*__first.__seg_ &= ~__m;
55+
*__first.__seg_ = std::__fill_range_in_word(*__first.__seg_, 0u, __bits_per_word - __n, _FillVal);
6456
}
6557
}
6658

libcxx/include/__fwd/bit_reference.h

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99
#ifndef _LIBCPP___FWD_BIT_REFERENCE_H
1010
#define _LIBCPP___FWD_BIT_REFERENCE_H
1111

12+
#include <__assert>
1213
#include <__config>
1314
#include <__type_traits/enable_if.h>
1415
#include <__type_traits/is_unsigned.h>
16+
#include <climits>
1517

1618
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
1719
# pragma GCC system_header
@@ -25,20 +27,18 @@ class __bit_iterator;
2527
template <class, class = void>
2628
struct __size_difference_type_traits;
2729

30+
// This function is designed to operate correctly even for smaller integral types like `uint8_t`, `uint16_t`,
31+
// or `unsigned short`. Casting back to _StorageType is crucial to prevent undefined behavior that can arise
32+
// from integral promotions.
33+
// See https://github.com/llvm/llvm-project/pull/122410
2834
template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> = 0>
29-
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __leading_mask(unsigned __shift) {
30-
return static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __shift);
31-
}
32-
33-
template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> = 0>
34-
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __trailing_mask(unsigned __shift) {
35-
return static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __shift);
36-
}
37-
38-
template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> = 0>
39-
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __middle_mask(unsigned __lshift, unsigned __rshift) {
40-
return static_cast<_StorageType>(
41-
std::__leading_mask<_StorageType>(__lshift) & std::__trailing_mask<_StorageType>(__rshift));
35+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR _StorageType
36+
__fill_range_in_word(_StorageType __word, unsigned __ctz, unsigned __clz, bool __fill_val) {
37+
_LIBCPP_ASSERT_VALID_INPUT_RANGE(
38+
__ctz + __clz < sizeof(_StorageType) * CHAR_BIT, "__fill_range_in_word called with invalid range");
39+
_StorageType __m = static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __ctz) &
40+
static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __clz);
41+
return __fill_val ? __word | __m : __word & ~__m;
4242
}
4343

4444
_LIBCPP_END_NAMESPACE_STD

libcxx/test/support/sized_allocator.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,15 @@
1616

1717
#include "test_macros.h"
1818

19-
template <typename T, typename SIZE_TYPE = std::size_t, typename DIFF_TYPE = std::ptrdiff_t>
19+
template <typename T, typename Size = std::size_t, typename Difference = std::ptrdiff_t>
2020
class sized_allocator {
2121
template <typename U, typename Sz, typename Diff>
2222
friend class sized_allocator;
2323

2424
public:
2525
using value_type = T;
26-
using size_type = SIZE_TYPE;
27-
using difference_type = DIFF_TYPE;
26+
using size_type = Size;
27+
using difference_type = Difference;
2828
using propagate_on_container_swap = std::true_type;
2929

3030
TEST_CONSTEXPR_CXX20 explicit sized_allocator(int d = 0) : data_(d) {}
@@ -55,4 +55,4 @@ class sized_allocator {
5555
}
5656
};
5757

58-
#endif
58+
#endif // TEST_SUPPORT_SIZED_ALLOCATOR_H

libcxx/utils/libcxx/test/params.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
"-Wno-reserved-module-identifier",
3232
'-Wdeprecated-copy',
3333
'-Wdeprecated-copy-dtor',
34+
"-Wshift-negative-value",
3435
# GCC warns about places where we might want to add sized allocation/deallocation
3536
# functions, but we know better what we're doing/testing in the test suite.
3637
"-Wno-sized-deallocation",

0 commit comments

Comments
 (0)