Skip to content

Commit ac6addf

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

File tree

5 files changed

+30
-28
lines changed

5 files changed

+30
-28
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+
std::__fill_masked_range(std::__to_address(__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+
std::__fill_masked_range(std::__to_address(__first.__seg_), 0u, __bits_per_word - __n, _FillVal);
6456
}
6557
}
6658

libcxx/include/__bit_reference

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include <__algorithm/copy_n.h>
1414
#include <__algorithm/min.h>
15+
#include <__assert>
1516
#include <__bit/countr.h>
1617
#include <__compare/ordering.h>
1718
#include <__config>
@@ -22,9 +23,12 @@
2223
#include <__memory/construct_at.h>
2324
#include <__memory/pointer_traits.h>
2425
#include <__type_traits/conditional.h>
26+
#include <__type_traits/enable_if.h>
2527
#include <__type_traits/is_constant_evaluated.h>
28+
#include <__type_traits/is_unsigned.h>
2629
#include <__type_traits/void_t.h>
2730
#include <__utility/swap.h>
31+
#include <climits>
2832

2933
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
3034
# pragma GCC system_header
@@ -55,6 +59,23 @@ struct __size_difference_type_traits<_Cp, __void_t<typename _Cp::difference_type
5559
using size_type = typename _Cp::size_type;
5660
};
5761

62+
// This function is designed to operate correctly even for smaller integral types like `uint8_t`, `uint16_t`,
63+
// or `unsigned short`. Casting back to _StorageType is crucial to prevent undefined behavior that can arise
64+
// from integral promotions.
65+
// See https://github.com/llvm/llvm-project/pull/122410
66+
template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> >
67+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
68+
__fill_masked_range(_StorageType* __word, unsigned __ctz, unsigned __clz, bool __fill_val) {
69+
_LIBCPP_ASSERT_VALID_INPUT_RANGE(
70+
__ctz + __clz < sizeof(_StorageType) * CHAR_BIT, "__fill_masked_range called with invalid range");
71+
_StorageType __m = static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __ctz) &
72+
static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __clz);
73+
if (__fill_val)
74+
*__word |= __m;
75+
else
76+
*__word &= static_cast<_StorageType>(~__m);
77+
}
78+
5879
template <class _Cp, bool = __has_storage_type<_Cp>::value>
5980
class __bit_reference {
6081
using __storage_type _LIBCPP_NODEBUG = typename _Cp::__storage_type;

libcxx/include/__fwd/bit_reference.h

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,8 @@ template <class, class = void>
2626
struct __size_difference_type_traits;
2727

2828
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));
42-
}
29+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
30+
__fill_masked_range(_StorageType* __word, unsigned __ctz, unsigned __clz, bool __fill_val);
4331

4432
_LIBCPP_END_NAMESPACE_STD
4533

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)