-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[libc++] Fix ambiguous call in {ranges, std}::count #122529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-libcxx Author: Peng Liu (winner245) ChangesThis PR fixes an ambiguous call encountered when using the Fixes #122528. Full diff: https://github.com/llvm/llvm-project/pull/122529.diff 6 Files Affected:
diff --git a/libcxx/include/__algorithm/count.h b/libcxx/include/__algorithm/count.h
index 6910b4f43e9934..7eac1f6a76023a 100644
--- a/libcxx/include/__algorithm/count.h
+++ b/libcxx/include/__algorithm/count.h
@@ -55,18 +55,21 @@ __count_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n)
if (__first.__ctz_ != 0) {
__storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
__storage_type __dn = std::min(__clz_f, __n);
- __storage_type __m = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
- __r = std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_) & __m);
+ __storage_type __m = __middle_mask<__storage_type>(__first.__ctz_, __clz_f - __dn);
+ // __r = std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_) & __m);
+ __r = std::popcount(__storage_type(std::__invert_if<!_ToCount>(*__first.__seg_) & __m));
__n -= __dn;
++__first.__seg_;
}
// do middle whole words
for (; __n >= __bits_per_word; ++__first.__seg_, __n -= __bits_per_word)
- __r += std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_));
+ // __r += std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_));
+ __r += std::popcount(__storage_type(std::__invert_if<!_ToCount>(*__first.__seg_)));
// do last partial word
if (__n > 0) {
- __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
- __r += std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_) & __m);
+ __storage_type __m = __trailing_mask<__storage_type>(__bits_per_word - __n);
+ // __r += std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_) & __m);
+ __r += std::popcount(__storage_type(std::__invert_if<!_ToCount>(*__first.__seg_) & __m));
}
return __r;
}
diff --git a/libcxx/include/__bit/popcount.h b/libcxx/include/__bit/popcount.h
index 5cf0a01d073382..a1ff92f072ad0b 100644
--- a/libcxx/include/__bit/popcount.h
+++ b/libcxx/include/__bit/popcount.h
@@ -15,6 +15,8 @@
#include <__bit/rotate.h>
#include <__concepts/arithmetic.h>
#include <__config>
+#include <__type_traits/enable_if.h>
+#include <__type_traits/is_unsigned.h>
#include <limits>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -62,6 +64,50 @@ template <__libcpp_unsigned_integer _Tp>
# endif // __has_builtin(__builtin_popcountg)
}
+#else
+
+template <class _Tp, __enable_if_t<__has_builtin(__builtin_popcountg) && is_unsigned<_Tp>::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int popcount(_Tp __t) _NOEXCEPT {
+ return __builtin_popcountg(__t);
+}
+
+template <
+ class _Tp,
+ __enable_if_t<!__has_builtin(__builtin_popcountg) && is_unsigned<_Tp>::value && sizeof(_Tp) <= sizeof(unsigned int),
+ int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int popcount(_Tp __t) _NOEXCEPT {
+ return std::__libcpp_popcount(static_cast<unsigned int>(__t));
+}
+
+template < class _Tp,
+ __enable_if_t<!__has_builtin(__builtin_popcountg) && is_unsigned<_Tp>::value &&
+ (sizeof(_Tp) > sizeof(unsigned int)) && sizeof(_Tp) <= sizeof(unsigned long),
+ int> = 0 >
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int popcount(_Tp __t) _NOEXCEPT {
+ return std::__libcpp_popcount(static_cast<unsigned long>(__t));
+}
+
+template < class _Tp,
+ __enable_if_t<!__has_builtin(__builtin_popcountg) && is_unsigned<_Tp>::value &&
+ (sizeof(_Tp) > sizeof(unsigned long)) && sizeof(_Tp) <= sizeof(unsigned long long),
+ int> = 0 >
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int popcount(_Tp __t) _NOEXCEPT {
+ return std::__libcpp_popcount(static_cast<unsigned long long>(__t));
+}
+
+template < class _Tp,
+ __enable_if_t<!__has_builtin(__builtin_popcountg) && is_unsigned<_Tp>::value &&
+ (sizeof(_Tp) > sizeof(unsigned long long)),
+ int> = 0 >
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 int popcount(_Tp __t) _NOEXCEPT {
+ int __ret = 0;
+ while (__t != 0) {
+ __ret += std::__libcpp_popcount(static_cast<unsigned long long>(__t));
+ __t >>= numeric_limits<unsigned long long>::digits;
+ }
+ return __ret;
+}
+
#endif // _LIBCPP_STD_VER >= 20
_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/__fwd/bit_reference.h b/libcxx/include/__fwd/bit_reference.h
index 237efb6db66429..c927e65747af08 100644
--- a/libcxx/include/__fwd/bit_reference.h
+++ b/libcxx/include/__fwd/bit_reference.h
@@ -10,6 +10,8 @@
#define _LIBCPP___FWD_BIT_REFERENCE_H
#include <__config>
+#include <__type_traits/enable_if.h>
+#include <__type_traits/is_unsigned.h>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
@@ -20,6 +22,22 @@ _LIBCPP_BEGIN_NAMESPACE_STD
template <class _Cp, bool _IsConst, typename _Cp::__storage_type = 0>
class __bit_iterator;
+template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __leading_mask(unsigned __shift) {
+ return static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __shift);
+}
+
+template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __trailing_mask(unsigned __shift) {
+ return static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __shift);
+}
+
+template <class _StorageType, __enable_if_t<is_unsigned<_StorageType>::value, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __middle_mask(unsigned __lshift, unsigned __rshift) {
+ return static_cast<_StorageType>(
+ std::__leading_mask<_StorageType>(__lshift) & std::__trailing_mask<_StorageType>(__rshift));
+}
+
_LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP___FWD_BIT_REFERENCE_H
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.count/count.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.count/count.pass.cpp
index 7250c49a7ff952..7e5d0c1cc9e470 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.count/count.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.count/count.pass.cpp
@@ -21,6 +21,7 @@
#include <cstddef>
#include <vector>
+#include "sized_allocator.h"
#include "test_macros.h"
#include "test_iterators.h"
#include "type_algorithms.h"
@@ -36,6 +37,29 @@ struct Test {
}
};
+TEST_CONSTEXPR_CXX20 void test_bititer_with_custom_sized_types() {
+ {
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(100, true, Alloc(1));
+ assert(std::count(in.begin(), in.end(), true) == 100);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(200, true, Alloc(1));
+ assert(std::count(in.begin(), in.end(), true) == 200);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
+ std::vector<bool, Alloc> in(200, true, Alloc(1));
+ assert(std::count(in.begin(), in.end(), true) == 200);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
+ std::vector<bool, Alloc> in(200, true, Alloc(1));
+ assert(std::count(in.begin(), in.end(), true) == 200);
+ }
+}
+
TEST_CONSTEXPR_CXX20 bool test() {
types::for_each(types::cpp17_input_iterator_list<const int*>(), Test());
@@ -51,6 +75,8 @@ TEST_CONSTEXPR_CXX20 bool test() {
}
}
+ test_bititer_with_custom_sized_types();
+
return true;
}
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.count/ranges.count.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.count/ranges.count.pass.cpp
index 6030bed47ec6a7..b3a9ccca22e4d0 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.count/ranges.count.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.count/ranges.count.pass.cpp
@@ -29,6 +29,7 @@
#include <ranges>
#include <vector>
+#include "sized_allocator.h"
#include "almost_satisfies_types.h"
#include "test_iterators.h"
@@ -67,13 +68,13 @@ constexpr void test_iterators() {
{
// simple test
{
- int a[] = {1, 2, 3, 4};
+ int a[] = {1, 2, 3, 4};
std::same_as<std::ptrdiff_t> auto ret = std::ranges::count(It(a), Sent(It(a + 4)), 3);
assert(ret == 1);
}
{
- int a[] = {1, 2, 3, 4};
- auto range = std::ranges::subrange(It(a), Sent(It(a + 4)));
+ int a[] = {1, 2, 3, 4};
+ auto range = std::ranges::subrange(It(a), Sent(It(a + 4)));
std::same_as<std::ptrdiff_t> auto ret = std::ranges::count(range, 3);
assert(ret == 1);
}
@@ -83,13 +84,13 @@ constexpr void test_iterators() {
// check that an empty range works
{
std::array<int, 0> a = {};
- auto ret = std::ranges::count(It(a.data()), Sent(It(a.data() + a.size())), 1);
+ auto ret = std::ranges::count(It(a.data()), Sent(It(a.data() + a.size())), 1);
assert(ret == 0);
}
{
std::array<int, 0> a = {};
- auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data() + a.size())));
- auto ret = std::ranges::count(range, 1);
+ auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data() + a.size())));
+ auto ret = std::ranges::count(range, 1);
assert(ret == 0);
}
}
@@ -98,13 +99,13 @@ constexpr void test_iterators() {
// check that a range with a single element works
{
std::array a = {2};
- auto ret = std::ranges::count(It(a.data()), Sent(It(a.data() + a.size())), 2);
+ auto ret = std::ranges::count(It(a.data()), Sent(It(a.data() + a.size())), 2);
assert(ret == 1);
}
{
std::array a = {2};
- auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data() + a.size())));
- auto ret = std::ranges::count(range, 2);
+ auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data() + a.size())));
+ auto ret = std::ranges::count(range, 2);
assert(ret == 1);
}
}
@@ -113,13 +114,13 @@ constexpr void test_iterators() {
// check that 0 is returned with no match
{
std::array a = {1, 1, 1};
- auto ret = std::ranges::count(It(a.data()), Sent(It(a.data() + a.size())), 0);
+ auto ret = std::ranges::count(It(a.data()), Sent(It(a.data() + a.size())), 0);
assert(ret == 0);
}
{
std::array a = {1, 1, 1};
- auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data() + a.size())));
- auto ret = std::ranges::count(range, 0);
+ auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data() + a.size())));
+ auto ret = std::ranges::count(range, 0);
assert(ret == 0);
}
}
@@ -128,13 +129,13 @@ constexpr void test_iterators() {
// check that more than one element is counted
{
std::array a = {3, 3, 4, 3, 3};
- auto ret = std::ranges::count(It(a.data()), Sent(It(a.data() + a.size())), 3);
+ auto ret = std::ranges::count(It(a.data()), Sent(It(a.data() + a.size())), 3);
assert(ret == 4);
}
{
std::array a = {3, 3, 4, 3, 3};
- auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data() + a.size())));
- auto ret = std::ranges::count(range, 3);
+ auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data() + a.size())));
+ auto ret = std::ranges::count(range, 3);
assert(ret == 4);
}
}
@@ -143,18 +144,41 @@ constexpr void test_iterators() {
// check that all elements are counted
{
std::array a = {5, 5, 5, 5};
- auto ret = std::ranges::count(It(a.data()), Sent(It(a.data() + a.size())), 5);
+ auto ret = std::ranges::count(It(a.data()), Sent(It(a.data() + a.size())), 5);
assert(ret == 4);
}
{
std::array a = {5, 5, 5, 5};
- auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data() + a.size())));
- auto ret = std::ranges::count(range, 5);
+ auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data() + a.size())));
+ auto ret = std::ranges::count(range, 5);
assert(ret == 4);
}
}
}
+TEST_CONSTEXPR_CXX20 void test_bititer_with_custom_sized_types() {
+ {
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> in(100, true, Alloc(1));
+ assert(std::ranges::count(in, true) == 100);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> in(200, true, Alloc(1));
+ assert(std::ranges::count(in, true) == 200);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
+ std::vector<bool, Alloc> in(200, true, Alloc(1));
+ assert(std::ranges::count(in, true) == 200);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
+ std::vector<bool, Alloc> in(200, true, Alloc(1));
+ assert(std::ranges::count(in, true) == 200);
+ }
+}
+
constexpr bool test() {
test_iterators<int*>();
test_iterators<const int*>();
@@ -167,12 +191,12 @@ constexpr bool test() {
{
// check that projections are used properly and that they are called with the iterator directly
{
- int a[] = {1, 2, 3, 4};
+ int a[] = {1, 2, 3, 4};
auto ret = std::ranges::count(a, a + 4, a + 3, [](int& i) { return &i; });
assert(ret == 1);
}
{
- int a[] = {1, 2, 3, 4};
+ int a[] = {1, 2, 3, 4};
auto ret = std::ranges::count(a, a + 3, [](int& i) { return &i; });
assert(ret == 1);
}
@@ -180,8 +204,10 @@ constexpr bool test() {
{
// check that std::invoke is used
- struct S { int i; };
- S a[] = { S{1}, S{3}, S{2} };
+ struct S {
+ int i;
+ };
+ S a[] = {S{1}, S{3}, S{2}};
std::same_as<std::ptrdiff_t> auto ret = std::ranges::count(a, 4, &S::i);
assert(ret == 0);
}
@@ -189,16 +215,22 @@ constexpr bool test() {
{
// count invocations of the projection
{
- int a[] = {1, 2, 3, 4};
+ int a[] = {1, 2, 3, 4};
int projection_count = 0;
- auto ret = std::ranges::count(a, a + 4, 2, [&](int i) { ++projection_count; return i; });
+ auto ret = std::ranges::count(a, a + 4, 2, [&](int i) {
+ ++projection_count;
+ return i;
+ });
assert(ret == 1);
assert(projection_count == 4);
}
{
- int a[] = {1, 2, 3, 4};
+ int a[] = {1, 2, 3, 4};
int projection_count = 0;
- auto ret = std::ranges::count(a, 2, [&](int i) { ++projection_count; return i; });
+ auto ret = std::ranges::count(a, 2, [&](int i) {
+ ++projection_count;
+ return i;
+ });
assert(ret == 1);
assert(projection_count == 4);
}
@@ -208,7 +240,7 @@ constexpr bool test() {
// check that an immobile type works
struct NonMovable {
NonMovable(const NonMovable&) = delete;
- NonMovable(NonMovable&&) = delete;
+ NonMovable(NonMovable&&) = delete;
constexpr NonMovable(int i_) : i(i_) {}
int i;
@@ -216,12 +248,12 @@ constexpr bool test() {
};
{
NonMovable a[] = {9, 8, 4, 3};
- auto ret = std::ranges::count(a, a + 4, NonMovable(8));
+ auto ret = std::ranges::count(a, a + 4, NonMovable(8));
assert(ret == 1);
}
{
NonMovable a[] = {9, 8, 4, 3};
- auto ret = std::ranges::count(a, NonMovable(8));
+ auto ret = std::ranges::count(a, NonMovable(8));
assert(ret == 1);
}
}
@@ -230,7 +262,7 @@ constexpr bool test() {
// check that difference_type is used
struct DiffTypeIterator {
using difference_type = signed char;
- using value_type = int;
+ using value_type = int;
int* it = nullptr;
@@ -238,7 +270,10 @@ constexpr bool test() {
constexpr DiffTypeIterator(int* i) : it(i) {}
constexpr int& operator*() const { return *it; }
- constexpr DiffTypeIterator& operator++() { ++it; return *this; }
+ constexpr DiffTypeIterator& operator++() {
+ ++it;
+ return *this;
+ }
constexpr void operator++(int) { ++it; }
bool operator==(const DiffTypeIterator&) const = default;
@@ -251,7 +286,7 @@ constexpr bool test() {
assert(ret == 1);
}
{
- int a[] = {5, 5, 4, 3, 2, 1};
+ int a[] = {5, 5, 4, 3, 2, 1};
auto range = std::ranges::subrange(DiffTypeIterator(a), DiffTypeIterator(a + 6));
std::same_as<signed char> decltype(auto) ret = std::ranges::count(range, 4);
assert(ret == 1);
@@ -270,6 +305,8 @@ constexpr bool test() {
}
}
+ test_bititer_with_custom_sized_types();
+
return true;
}
diff --git a/libcxx/test/support/sized_allocator.h b/libcxx/test/support/sized_allocator.h
new file mode 100644
index 00000000000000..9a16430f7eaf9c
--- /dev/null
+++ b/libcxx/test/support/sized_allocator.h
@@ -0,0 +1,58 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TEST_SUPPORT_SIZED_ALLOCATOR_H
+#define TEST_SUPPORT_SIZED_ALLOCATOR_H
+
+#include <cstddef>
+#include <limits>
+#include <memory>
+#include <new>
+
+#include "test_macros.h"
+
+template <typename T, typename SIZE_TYPE = std::size_t, typename DIFF_TYPE = std::ptrdiff_t>
+class sized_allocator {
+ template <typename U, typename Sz, typename Diff>
+ friend class sized_allocator;
+
+public:
+ using value_type = T;
+ using size_type = SIZE_TYPE;
+ using difference_type = DIFF_TYPE;
+ using propagate_on_container_swap = std::true_type;
+
+ TEST_CONSTEXPR_CXX20 explicit sized_allocator(int d = 0) : data_(d) {}
+
+ template <typename U, typename Sz, typename Diff>
+ TEST_CONSTEXPR_CXX20 sized_allocator(const sized_allocator<U, Sz, Diff>& a) TEST_NOEXCEPT : data_(a.data_) {}
+
+ TEST_CONSTEXPR_CXX20 T* allocate(size_type n) {
+ if (n > max_size())
+ TEST_THROW(std::bad_array_new_length());
+ return std::allocator<T>().allocate(n);
+ }
+
+ TEST_CONSTEXPR_CXX20 void deallocate(T* p, size_type n) TEST_NOEXCEPT { std::allocator<T>().deallocate(p, n); }
+
+ TEST_CONSTEXPR size_type max_size() const TEST_NOEXCEPT {
+ return std::numeric_limits<size_type>::max() / sizeof(value_type);
+ }
+
+private:
+ int data_;
+
+ TEST_CONSTEXPR friend bool operator==(const sized_allocator& a, const sized_allocator& b) {
+ return a.data_ == b.data_;
+ }
+ TEST_CONSTEXPR friend bool operator!=(const sized_allocator& a, const sized_allocator& b) {
+ return a.data_ != b.data_;
+ }
+};
+
+#endif
\ No newline at end of file
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
2c5ac98
to
1903706
Compare
1903706
to
d244814
Compare
5faefc0
to
fad654f
Compare
5ed673a
to
9da6c14
Compare
9da6c14
to
7f35b7f
Compare
7f35b7f
to
b51d43a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM with my suggestion applied. Thanks!
c6437ab
to
d2ccb4d
Compare
f70dcb4
to
693faf9
Compare
693faf9
to
c8db94b
Compare
This PR fixes an ambiguous call encountered while using the
std::ranges::count
andstd::count
algorithms withvector<bool>
with smallsize_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.