Skip to content

Commit e53bea5

Browse files
authored
[libc++] Fix ambiguous call in {ranges, std}::count (#122529)
This PR fixes an ambiguous call encountered while using the `std::ranges::count` and `std::count` algorithms with `vector<bool>` with small `size_type`s. The ambiguity arises from integral promotions during the internal bitwise arithmetic of the `count` algorithms for small integral types. This results in multiple viable candidates: `__libcpp_popcount(unsigned)`,` __libcpp_popcount(unsigned long)`, and `__libcpp_popcount(unsigned long long)`, leading to an ambiguous call error. To resolve this ambiguity, we introduce a dispatcher function, `__popcount`, which directs calls to the appropriate overloads of `__libcpp_popcount`. This closes #122528.
1 parent d7879e5 commit e53bea5

File tree

4 files changed

+161
-67
lines changed

4 files changed

+161
-67
lines changed

libcxx/include/__algorithm/count.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,18 @@ __count_bool(__bit_iterator<_Cp, _IsConst> __first, typename __size_difference_t
5555
if (__first.__ctz_ != 0) {
5656
__storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
5757
__storage_type __dn = std::min(__clz_f, __n);
58-
__storage_type __m = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
59-
__r = std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_) & __m);
58+
__storage_type __m = std::__middle_mask<__storage_type>(__clz_f - __dn, __first.__ctz_);
59+
__r = std::__popcount(__storage_type(std::__invert_if<!_ToCount>(*__first.__seg_) & __m));
6060
__n -= __dn;
6161
++__first.__seg_;
6262
}
6363
// do middle whole words
6464
for (; __n >= __bits_per_word; ++__first.__seg_, __n -= __bits_per_word)
65-
__r += std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_));
65+
__r += std::__popcount(std::__invert_if<!_ToCount>(*__first.__seg_));
6666
// do last partial word
6767
if (__n > 0) {
68-
__storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
69-
__r += std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_) & __m);
68+
__storage_type __m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
69+
__r += std::__popcount(__storage_type(std::__invert_if<!_ToCount>(*__first.__seg_) & __m));
7070
}
7171
return __r;
7272
}

libcxx/include/__bit/popcount.h

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <__bit/rotate.h>
1616
#include <__concepts/arithmetic.h>
1717
#include <__config>
18+
#include <__type_traits/is_unsigned.h>
1819
#include <limits>
1920

2021
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -38,31 +39,48 @@ inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int __libcpp_popcount(unsigned lo
3839
return __builtin_popcountll(__x);
3940
}
4041

41-
#if _LIBCPP_STD_VER >= 20
42-
43-
template <__libcpp_unsigned_integer _Tp>
44-
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr int popcount(_Tp __t) noexcept {
45-
# if __has_builtin(__builtin_popcountg)
46-
return __builtin_popcountg(__t);
47-
# else // __has_builtin(__builtin_popcountg)
48-
if (sizeof(_Tp) <= sizeof(unsigned int))
42+
template <class _Tp>
43+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int __popcount_impl(_Tp __t) _NOEXCEPT {
44+
if _LIBCPP_CONSTEXPR (sizeof(_Tp) <= sizeof(unsigned int)) {
4945
return std::__libcpp_popcount(static_cast<unsigned int>(__t));
50-
else if (sizeof(_Tp) <= sizeof(unsigned long))
46+
} else if _LIBCPP_CONSTEXPR (sizeof(_Tp) <= sizeof(unsigned long)) {
5147
return std::__libcpp_popcount(static_cast<unsigned long>(__t));
52-
else if (sizeof(_Tp) <= sizeof(unsigned long long))
48+
} else if _LIBCPP_CONSTEXPR (sizeof(_Tp) <= sizeof(unsigned long long)) {
5349
return std::__libcpp_popcount(static_cast<unsigned long long>(__t));
54-
else {
50+
} else {
51+
#if _LIBCPP_STD_VER == 11
52+
return __t != 0 ? std::__libcpp_popcount(static_cast<unsigned long long>(__t)) +
53+
std::__popcount_impl<_Tp>(__t >> numeric_limits<unsigned long long>::digits)
54+
: 0;
55+
#else
5556
int __ret = 0;
5657
while (__t != 0) {
5758
__ret += std::__libcpp_popcount(static_cast<unsigned long long>(__t));
58-
__t >>= numeric_limits<unsigned long long>::digits;
59+
__t >>= std::numeric_limits<unsigned long long>::digits;
5960
}
6061
return __ret;
62+
#endif
6163
}
62-
# endif // __has_builtin(__builtin_popcountg)
6364
}
6465

65-
#endif // _LIBCPP_STD_VER >= 20
66+
template <class _Tp>
67+
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int __popcount(_Tp __t) _NOEXCEPT {
68+
static_assert(is_unsigned<_Tp>::value, "__popcount only works with unsigned types");
69+
#if __has_builtin(__builtin_popcountg) // TODO (LLVM 21): This can be dropped once we only support Clang >= 19.
70+
return __builtin_popcountg(__t);
71+
#else
72+
return std::__popcount_impl(__t);
73+
#endif
74+
}
75+
76+
#if _LIBCPP_STD_VER >= 20
77+
78+
template <__libcpp_unsigned_integer _Tp>
79+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr int popcount(_Tp __t) noexcept {
80+
return std::__popcount(__t);
81+
}
82+
83+
#endif
6684

6785
_LIBCPP_END_NAMESPACE_STD
6886

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

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@
1515

1616
// ADDITIONAL_COMPILE_FLAGS(has-fconstexpr-steps): -fconstexpr-steps=20000000
1717
// ADDITIONAL_COMPILE_FLAGS(has-fconstexpr-ops-limit): -fconstexpr-ops-limit=80000000
18+
// XFAIL: FROZEN-CXX03-HEADERS-FIXME
1819

1920
#include <algorithm>
2021
#include <cassert>
2122
#include <cstddef>
2223
#include <vector>
2324

25+
#include "sized_allocator.h"
2426
#include "test_macros.h"
2527
#include "test_iterators.h"
2628
#include "type_algorithms.h"
@@ -39,16 +41,47 @@ struct Test {
3941
TEST_CONSTEXPR_CXX20 bool test() {
4042
types::for_each(types::cpp17_input_iterator_list<const int*>(), Test());
4143

42-
if (TEST_STD_AT_LEAST_20_OR_RUNTIME_EVALUATED) {
43-
std::vector<bool> vec(256 + 64);
44-
for (ptrdiff_t i = 0; i != 256; ++i) {
45-
for (size_t offset = 0; offset != 64; ++offset) {
46-
std::fill(vec.begin(), vec.end(), false);
47-
std::fill(vec.begin() + offset, vec.begin() + i + offset, true);
48-
assert(std::count(vec.begin() + offset, vec.begin() + offset + 256, true) == i);
49-
assert(std::count(vec.begin() + offset, vec.begin() + offset + 256, false) == 256 - i);
44+
// Tests for std::count with std::vector<bool>::iterator optimizations.
45+
{
46+
{ // check that vector<bool>::iterator optimization works as expected
47+
std::vector<bool> vec(256 + 64);
48+
for (ptrdiff_t i = 0; i != 256; ++i) {
49+
for (size_t offset = 0; offset != 64; ++offset) {
50+
std::fill(vec.begin(), vec.end(), false);
51+
std::fill(vec.begin() + offset, vec.begin() + i + offset, true);
52+
assert(std::count(vec.begin() + offset, vec.begin() + offset + 256, true) == i);
53+
assert(std::count(vec.begin() + offset, vec.begin() + offset + 256, false) == 256 - i);
54+
}
5055
}
5156
}
57+
58+
// Fix std::count for std::vector<bool> with small storage types, e.g., std::uint16_t, unsigned short.
59+
// See https://github.com/llvm/llvm-project/issues/122528
60+
{
61+
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
62+
std::vector<bool, Alloc> in(100, true, Alloc(1));
63+
assert(std::count(in.begin(), in.end(), true) == 100);
64+
}
65+
{
66+
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
67+
std::vector<bool, Alloc> in(199, true, Alloc(1));
68+
assert(std::count(in.begin(), in.end(), true) == 199);
69+
}
70+
{
71+
using Alloc = sized_allocator<bool, unsigned short, short>;
72+
std::vector<bool, Alloc> in(200, true, Alloc(1));
73+
assert(std::count(in.begin(), in.end(), true) == 200);
74+
}
75+
{
76+
using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
77+
std::vector<bool, Alloc> in(205, true, Alloc(1));
78+
assert(std::count(in.begin(), in.end(), true) == 205);
79+
}
80+
{
81+
using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
82+
std::vector<bool, Alloc> in(257, true, Alloc(1));
83+
assert(std::count(in.begin(), in.end(), true) == 257);
84+
}
5285
}
5386

5487
return true;

0 commit comments

Comments
 (0)