Skip to content

Commit 3bd71cb

Browse files
authored
[libc++] Fix ambiguous call in {ranges, std}::find (llvm#122641)
This PR fixes an ambiguous call encountered when using the `std::ranges::find` or `std::find` algorithms with `vector<bool>` with small `allocator_traits::size_type`s, an issue reported in llvm#122528. The ambiguity arises from integral promotions during the internal bitwise arithmetic of the `find` algorithms when applied to `vector<bool>` with small integral `size_type`s. This leads to multiple viable candidates for small integral types: __libcpp_ctz(unsigned), __libcpp_ctz(unsigned long), and __libcpp_ctz(unsigned long long), none of which represent a single best viable match, resulting in an ambiguous call error. To resolve this, we propose invoking an internal function __countr_zero as a dispatcher that directs the call to the appropriate overload of __libcpp_ctz. Necessary amendments have also been made to __countr_zero.
1 parent e61859f commit 3bd71cb

File tree

7 files changed

+151
-39
lines changed

7 files changed

+151
-39
lines changed

libcxx/include/__algorithm/find.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,10 @@ __find_bool(__bit_iterator<_Cp, _IsConst> __first, typename __size_difference_ty
106106
if (__first.__ctz_ != 0) {
107107
__storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
108108
__storage_type __dn = std::min(__clz_f, __n);
109-
__storage_type __m = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
109+
__storage_type __m = std::__middle_mask<__storage_type>(__clz_f - __dn, __first.__ctz_);
110110
__storage_type __b = std::__invert_if<!_ToFind>(*__first.__seg_) & __m;
111111
if (__b)
112-
return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
112+
return _It(__first.__seg_, static_cast<unsigned>(std::__countr_zero(__b)));
113113
if (__n == __dn)
114114
return __first + __n;
115115
__n -= __dn;
@@ -119,14 +119,14 @@ __find_bool(__bit_iterator<_Cp, _IsConst> __first, typename __size_difference_ty
119119
for (; __n >= __bits_per_word; ++__first.__seg_, __n -= __bits_per_word) {
120120
__storage_type __b = std::__invert_if<!_ToFind>(*__first.__seg_);
121121
if (__b)
122-
return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
122+
return _It(__first.__seg_, static_cast<unsigned>(std::__countr_zero(__b)));
123123
}
124124
// do last partial word
125125
if (__n > 0) {
126-
__storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
126+
__storage_type __m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
127127
__storage_type __b = std::__invert_if<!_ToFind>(*__first.__seg_) & __m;
128128
if (__b)
129-
return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
129+
return _It(__first.__seg_, static_cast<unsigned>(std::__countr_zero(__b)));
130130
}
131131
return _It(__first.__seg_, static_cast<unsigned>(__n));
132132
}

libcxx/include/__bit/countr.h

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@
1212
#ifndef _LIBCPP___BIT_COUNTR_H
1313
#define _LIBCPP___BIT_COUNTR_H
1414

15+
#include <__assert>
1516
#include <__bit/rotate.h>
1617
#include <__concepts/arithmetic.h>
1718
#include <__config>
19+
#include <__type_traits/is_unsigned.h>
1820
#include <limits>
1921

2022
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -38,29 +40,43 @@ _LIBCPP_BEGIN_NAMESPACE_STD
3840
return __builtin_ctzll(__x);
3941
}
4042

43+
// A constexpr implementation for C++11 and later (using clang extensions for constexpr support)
44+
// Precondition: __t != 0 (the caller __countr_zero handles __t == 0 as a special case)
4145
template <class _Tp>
42-
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 int __countr_zero(_Tp __t) _NOEXCEPT {
43-
#if __has_builtin(__builtin_ctzg)
44-
return __builtin_ctzg(__t, numeric_limits<_Tp>::digits);
45-
#else // __has_builtin(__builtin_ctzg)
46-
if (__t == 0)
47-
return numeric_limits<_Tp>::digits;
48-
if (sizeof(_Tp) <= sizeof(unsigned int))
46+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int __countr_zero_impl(_Tp __t) _NOEXCEPT {
47+
_LIBCPP_ASSERT_INTERNAL(__t != 0, "__countr_zero_impl called with zero value");
48+
static_assert(is_unsigned<_Tp>::value, "__countr_zero_impl only works with unsigned types");
49+
if _LIBCPP_CONSTEXPR (sizeof(_Tp) <= sizeof(unsigned int)) {
4950
return std::__libcpp_ctz(static_cast<unsigned int>(__t));
50-
else if (sizeof(_Tp) <= sizeof(unsigned long))
51+
} else if _LIBCPP_CONSTEXPR (sizeof(_Tp) <= sizeof(unsigned long)) {
5152
return std::__libcpp_ctz(static_cast<unsigned long>(__t));
52-
else if (sizeof(_Tp) <= sizeof(unsigned long long))
53+
} else if _LIBCPP_CONSTEXPR (sizeof(_Tp) <= sizeof(unsigned long long)) {
5354
return std::__libcpp_ctz(static_cast<unsigned long long>(__t));
54-
else {
55+
} else {
56+
#if _LIBCPP_STD_VER == 11
57+
unsigned long long __ull = static_cast<unsigned long long>(__t);
58+
const unsigned int __ulldigits = numeric_limits<unsigned long long>::digits;
59+
return __ull == 0ull ? __ulldigits + std::__countr_zero_impl<_Tp>(__t >> __ulldigits) : std::__libcpp_ctz(__ull);
60+
#else
5561
int __ret = 0;
5662
const unsigned int __ulldigits = numeric_limits<unsigned long long>::digits;
5763
while (static_cast<unsigned long long>(__t) == 0uLL) {
5864
__ret += __ulldigits;
5965
__t >>= __ulldigits;
6066
}
6167
return __ret + std::__libcpp_ctz(static_cast<unsigned long long>(__t));
68+
#endif
6269
}
63-
#endif // __has_builtin(__builtin_ctzg)
70+
}
71+
72+
template <class _Tp>
73+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int __countr_zero(_Tp __t) _NOEXCEPT {
74+
static_assert(is_unsigned<_Tp>::value, "__countr_zero only works with unsigned types");
75+
#if __has_builtin(__builtin_ctzg) // TODO (LLVM 21): This can be dropped once we only support Clang >= 19.
76+
return __builtin_ctzg(__t, numeric_limits<_Tp>::digits);
77+
#else
78+
return __t != 0 ? std::__countr_zero_impl(__t) : numeric_limits<_Tp>::digits;
79+
#endif
6480
}
6581

6682
#if _LIBCPP_STD_VER >= 20

libcxx/include/__bit_reference

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,30 @@ struct __size_difference_type_traits<_Cp, __void_t<typename _Cp::difference_type
6969
using size_type = typename _Cp::size_type;
7070
};
7171

72+
// The `__x_mask` functions are designed to work exclusively with any unsigned `_StorageType`s, including small
73+
// integral types such as unsigned char/short, `uint8_t`, and `uint16_t`. To prevent undefined behavior or
74+
// ambiguities due to integral promotions for the small integral types, all intermediate bitwise operations are
75+
// explicitly cast back to the unsigned `_StorageType`.
76+
77+
// Creates a mask of type `_StorageType` with a specified number of leading zeros (__clz) and sets all remaining
78+
// bits to one.
79+
template <class _StorageType>
80+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __trailing_mask(unsigned __clz) {
81+
static_assert(is_unsigned<_StorageType>::value, "__trailing_mask only works with unsigned types");
82+
return static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __clz;
83+
}
84+
85+
// Creates a mask of type `_StorageType` with a specified number of leading zeros (__clz), a specified number of
86+
// trailing zeros (__ctz), and sets all bits in between to one.
87+
template <class _StorageType>
88+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __middle_mask(unsigned __clz, unsigned __ctz) {
89+
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);
92+
}
93+
7294
// This function is designed to operate correctly even for smaller integral types like `uint8_t`, `uint16_t`,
73-
// or `unsigned short`. Casting back to _StorageType is crucial to prevent undefined behavior that can arise
74-
// from integral promotions.
95+
// or `unsigned short`.
7596
// See https://github.com/llvm/llvm-project/pull/122410.
7697
template <class _StoragePointer>
7798
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
@@ -81,12 +102,11 @@ __fill_masked_range(_StoragePointer __word, unsigned __clz, unsigned __ctz, bool
81102
using _StorageType = typename pointer_traits<_StoragePointer>::element_type;
82103
_LIBCPP_ASSERT_VALID_INPUT_RANGE(
83104
__ctz + __clz < sizeof(_StorageType) * CHAR_BIT, "__fill_masked_range called with invalid range");
84-
_StorageType __m = static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __clz) &
85-
static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __ctz);
105+
_StorageType __m = std::__middle_mask<_StorageType>(__clz, __ctz);
86106
if (__fill_val)
87107
*__word |= __m;
88108
else
89-
*__word &= static_cast<_StorageType>(~__m);
109+
*__word &= ~__m;
90110
}
91111

92112
template <class _Cp, bool = __has_storage_type<_Cp>::value>

libcxx/include/__fwd/bit_reference.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,6 @@
1010
#define _LIBCPP___FWD_BIT_REFERENCE_H
1111

1212
#include <__config>
13-
#include <__memory/pointer_traits.h>
14-
#include <__type_traits/enable_if.h>
15-
#include <__type_traits/is_unsigned.h>
1613

1714
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
1815
# pragma GCC system_header
@@ -33,6 +30,12 @@ template <class _StoragePointer>
3330
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
3431
__fill_masked_range(_StoragePointer __word, unsigned __ctz, unsigned __clz, bool __fill_val);
3532

33+
template <class _StorageType>
34+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __trailing_mask(unsigned __clz);
35+
36+
template <class _StorageType>
37+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __middle_mask(unsigned __clz, unsigned __ctz);
38+
3639
_LIBCPP_END_NAMESPACE_STD
3740

3841
#endif // _LIBCPP___FWD_BIT_REFERENCE_H

libcxx/test/std/algorithms/alg.nonmodifying/alg.find/find.pass.cpp

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
// ADDITIONAL_COMPILE_FLAGS(cl-style-warnings): /wd4245 /wd4305 /wd4310 /wd4389 /wd4805
1717
// ADDITIONAL_COMPILE_FLAGS(has-fconstexpr-steps): -fconstexpr-steps=20000000
1818
// ADDITIONAL_COMPILE_FLAGS(has-fconstexpr-ops-limit): -fconstexpr-ops-limit=80000000
19+
// XFAIL: FROZEN-CXX03-HEADERS-FIXME
1920

2021
// <algorithm>
2122

@@ -31,6 +32,7 @@
3132
#include <vector>
3233
#include <type_traits>
3334

35+
#include "sized_allocator.h"
3436
#include "test_macros.h"
3537
#include "test_iterators.h"
3638
#include "type_algorithms.h"
@@ -228,7 +230,8 @@ TEST_CONSTEXPR_CXX20 bool test() {
228230

229231
types::for_each(types::integral_types(), TestIntegerPromotions());
230232

231-
{ // Test vector<bool>::iterator optimization
233+
{
234+
// Test vector<bool>::iterator optimization
232235
std::vector<bool> vec(256 + 8);
233236
for (ptrdiff_t i = 8; i <= 256; i *= 2) {
234237
for (size_t offset = 0; offset < 8; offset += 2) {
@@ -238,6 +241,39 @@ TEST_CONSTEXPR_CXX20 bool test() {
238241
assert(std::find(vec.begin() + offset, vec.end(), false) == vec.begin() + offset + i);
239242
}
240243
}
244+
245+
// Verify that the std::vector<bool>::iterator optimization works properly for allocators with custom size types
246+
// Fix https://github.com/llvm/llvm-project/issues/122528
247+
{
248+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
249+
std::vector<bool, Alloc> in(100, false, Alloc(1));
250+
in[in.size() - 2] = true;
251+
assert(std::find(in.begin(), in.end(), true) == in.end() - 2);
252+
}
253+
{
254+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
255+
std::vector<bool, Alloc> in(199, false, Alloc(1));
256+
in[in.size() - 2] = true;
257+
assert(std::find(in.begin(), in.end(), true) == in.end() - 2);
258+
}
259+
{
260+
using Alloc = sized_allocator<bool, unsigned short, short>;
261+
std::vector<bool, Alloc> in(200, false, Alloc(1));
262+
in[in.size() - 2] = true;
263+
assert(std::find(in.begin(), in.end(), true) == in.end() - 2);
264+
}
265+
{
266+
using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
267+
std::vector<bool, Alloc> in(205, false, Alloc(1));
268+
in[in.size() - 2] = true;
269+
assert(std::find(in.begin(), in.end(), true) == in.end() - 2);
270+
}
271+
{
272+
using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
273+
std::vector<bool, Alloc> in(257, false, Alloc(1));
274+
in[in.size() - 2] = true;
275+
assert(std::find(in.begin(), in.end(), true) == in.end() - 2);
276+
}
241277
}
242278

243279
return true;

libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include <vector>
3535

3636
#include "almost_satisfies_types.h"
37+
#include "sized_allocator.h"
3738
#include "test_iterators.h"
3839

3940
struct NotEqualityComparable {};
@@ -202,7 +203,8 @@ constexpr bool test() {
202203
}
203204
}
204205

205-
{ // Test vector<bool>::iterator optimization
206+
{
207+
// Test vector<bool>::iterator optimization
206208
std::vector<bool> vec(256 + 8);
207209
for (ptrdiff_t i = 8; i <= 256; i *= 2) {
208210
for (size_t offset = 0; offset < 8; offset += 2) {
@@ -215,6 +217,39 @@ constexpr bool test() {
215217
std::ranges::begin(vec) + offset + i);
216218
}
217219
}
220+
221+
// Verify that the std::vector<bool>::iterator optimization works properly for allocators with custom size types
222+
// See https://github.com/llvm/llvm-project/issues/122528
223+
{
224+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
225+
std::vector<bool, Alloc> in(100, false, Alloc(1));
226+
in[in.size() - 2] = true;
227+
assert(std::ranges::find(in, true) == in.end() - 2);
228+
}
229+
{
230+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
231+
std::vector<bool, Alloc> in(199, false, Alloc(1));
232+
in[in.size() - 2] = true;
233+
assert(std::ranges::find(in, true) == in.end() - 2);
234+
}
235+
{
236+
using Alloc = sized_allocator<bool, unsigned short, short>;
237+
std::vector<bool, Alloc> in(200, false, Alloc(1));
238+
in[in.size() - 2] = true;
239+
assert(std::ranges::find(in, true) == in.end() - 2);
240+
}
241+
{
242+
using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
243+
std::vector<bool, Alloc> in(205, false, Alloc(1));
244+
in[in.size() - 2] = true;
245+
assert(std::ranges::find(in, true) == in.end() - 2);
246+
}
247+
{
248+
using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
249+
std::vector<bool, Alloc> in(257, false, Alloc(1));
250+
in[in.size() - 2] = true;
251+
assert(std::ranges::find(in, true) == in.end() - 2);
252+
}
218253
}
219254

220255
return true;

libcxx/test/std/utilities/template.bitset/bitset.members/left_shift_eq.pass.cpp

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
// ADDITIONAL_COMPILE_FLAGS(has-fconstexpr-steps): -fconstexpr-steps=15000000
10+
911
// bitset<N>& operator<<=(size_t pos); // constexpr since C++23
1012

1113
#include <bitset>
@@ -18,20 +20,20 @@
1820

1921
template <std::size_t N>
2022
TEST_CONSTEXPR_CXX23 bool test_left_shift() {
21-
std::vector<std::bitset<N> > const cases = get_test_cases<N>();
22-
for (std::size_t c = 0; c != cases.size(); ++c) {
23-
for (std::size_t s = 0; s <= N+1; ++s) {
24-
std::bitset<N> v1 = cases[c];
25-
std::bitset<N> v2 = v1;
26-
v1 <<= s;
27-
for (std::size_t i = 0; i < v1.size(); ++i)
28-
if (i < s)
29-
assert(v1[i] == 0);
30-
else
31-
assert(v1[i] == v2[i-s]);
32-
}
23+
std::vector<std::bitset<N> > const cases = get_test_cases<N>();
24+
for (std::size_t c = 0; c != cases.size(); ++c) {
25+
for (std::size_t s = 0; s <= N + 1; ++s) {
26+
std::bitset<N> v1 = cases[c];
27+
std::bitset<N> v2 = v1;
28+
v1 <<= s;
29+
for (std::size_t i = 0; i < v1.size(); ++i)
30+
if (i < s)
31+
assert(v1[i] == 0);
32+
else
33+
assert(v1[i] == v2[i - s]);
3334
}
34-
return true;
35+
}
36+
return true;
3537
}
3638

3739
int main(int, char**) {

0 commit comments

Comments
 (0)