Skip to content

[libc++] Fix ambiguous call in {ranges, std}::find #122641

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

Merged
merged 4 commits into from
Mar 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions libcxx/include/__algorithm/find.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,10 @@ __find_bool(__bit_iterator<_Cp, _IsConst> __first, typename __size_difference_ty
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));
__storage_type __m = std::__middle_mask<__storage_type>(__clz_f - __dn, __first.__ctz_);
__storage_type __b = std::__invert_if<!_ToFind>(*__first.__seg_) & __m;
if (__b)
return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
return _It(__first.__seg_, static_cast<unsigned>(std::__countr_zero(__b)));
if (__n == __dn)
return __first + __n;
__n -= __dn;
Expand All @@ -119,14 +119,14 @@ __find_bool(__bit_iterator<_Cp, _IsConst> __first, typename __size_difference_ty
for (; __n >= __bits_per_word; ++__first.__seg_, __n -= __bits_per_word) {
__storage_type __b = std::__invert_if<!_ToFind>(*__first.__seg_);
if (__b)
return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
return _It(__first.__seg_, static_cast<unsigned>(std::__countr_zero(__b)));
}
// do last partial word
if (__n > 0) {
__storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
__storage_type __m = std::__trailing_mask<__storage_type>(__bits_per_word - __n);
__storage_type __b = std::__invert_if<!_ToFind>(*__first.__seg_) & __m;
if (__b)
return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
return _It(__first.__seg_, static_cast<unsigned>(std::__countr_zero(__b)));
}
return _It(__first.__seg_, static_cast<unsigned>(__n));
}
Expand Down
38 changes: 27 additions & 11 deletions libcxx/include/__bit/countr.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@
#ifndef _LIBCPP___BIT_COUNTR_H
#define _LIBCPP___BIT_COUNTR_H

#include <__assert>
#include <__bit/rotate.h>
#include <__concepts/arithmetic.h>
#include <__config>
#include <__type_traits/is_unsigned.h>
#include <limits>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
Expand All @@ -38,29 +40,43 @@ _LIBCPP_BEGIN_NAMESPACE_STD
return __builtin_ctzll(__x);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a follow-up patch, we should replace uses of __libcpp_ctz in the codebase by calls to __countr_zero (there's only a few). This will eventually allow us to clean up __libcpp_ctz altogether.

}

// A constexpr implementation for C++11 and later (using clang extensions for constexpr support)
// Precondition: __t != 0 (the caller __countr_zero handles __t == 0 as a special case)
template <class _Tp>
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 int __countr_zero(_Tp __t) _NOEXCEPT {
#if __has_builtin(__builtin_ctzg)
return __builtin_ctzg(__t, numeric_limits<_Tp>::digits);
#else // __has_builtin(__builtin_ctzg)
if (__t == 0)
return numeric_limits<_Tp>::digits;
if (sizeof(_Tp) <= sizeof(unsigned int))
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int __countr_zero_impl(_Tp __t) _NOEXCEPT {
_LIBCPP_ASSERT_INTERNAL(__t != 0, "__countr_zero_impl called with zero value");
static_assert(is_unsigned<_Tp>::value, "__countr_zero_impl only works with unsigned types");
if _LIBCPP_CONSTEXPR (sizeof(_Tp) <= sizeof(unsigned int)) {
return std::__libcpp_ctz(static_cast<unsigned int>(__t));
else if (sizeof(_Tp) <= sizeof(unsigned long))
} else if _LIBCPP_CONSTEXPR (sizeof(_Tp) <= sizeof(unsigned long)) {
return std::__libcpp_ctz(static_cast<unsigned long>(__t));
else if (sizeof(_Tp) <= sizeof(unsigned long long))
} else if _LIBCPP_CONSTEXPR (sizeof(_Tp) <= sizeof(unsigned long long)) {
return std::__libcpp_ctz(static_cast<unsigned long long>(__t));
else {
} else {
#if _LIBCPP_STD_VER == 11
unsigned long long __ull = static_cast<unsigned long long>(__t);
const unsigned int __ulldigits = numeric_limits<unsigned long long>::digits;
return __ull == 0ull ? __ulldigits + std::__countr_zero_impl<_Tp>(__t >> __ulldigits) : std::__libcpp_ctz(__ull);
#else
int __ret = 0;
const unsigned int __ulldigits = numeric_limits<unsigned long long>::digits;
while (static_cast<unsigned long long>(__t) == 0uLL) {
__ret += __ulldigits;
__t >>= __ulldigits;
}
return __ret + std::__libcpp_ctz(static_cast<unsigned long long>(__t));
#endif
}
#endif // __has_builtin(__builtin_ctzg)
}

template <class _Tp>
[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int __countr_zero(_Tp __t) _NOEXCEPT {
static_assert(is_unsigned<_Tp>::value, "__countr_zero only works with unsigned types");
#if __has_builtin(__builtin_ctzg) // TODO (LLVM 21): This can be dropped once we only support Clang >= 19.
return __builtin_ctzg(__t, numeric_limits<_Tp>::digits);
#else
return __t != 0 ? std::__countr_zero_impl(__t) : numeric_limits<_Tp>::digits;
#endif
}

#if _LIBCPP_STD_VER >= 20
Expand Down
30 changes: 25 additions & 5 deletions libcxx/include/__bit_reference
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,30 @@ struct __size_difference_type_traits<_Cp, __void_t<typename _Cp::difference_type
using size_type = typename _Cp::size_type;
};

// The `__x_mask` functions are designed to work exclusively with any unsigned `_StorageType`s, including small
// integral types such as unsigned char/short, `uint8_t`, and `uint16_t`. To prevent undefined behavior or
// ambiguities due to integral promotions for the small integral types, all intermediate bitwise operations are
// explicitly cast back to the unsigned `_StorageType`.

// Creates a mask of type `_StorageType` with a specified number of leading zeros (__clz) and sets all remaining
// bits to one.
template <class _StorageType>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __trailing_mask(unsigned __clz) {
static_assert(is_unsigned<_StorageType>::value, "__trailing_mask only works with unsigned types");
return static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __clz;
}

// Creates a mask of type `_StorageType` with a specified number of leading zeros (__clz), a specified number of
// trailing zeros (__ctz), and sets all bits in between to one.
template <class _StorageType>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __middle_mask(unsigned __clz, unsigned __ctz) {
static_assert(is_unsigned<_StorageType>::value, "__middle_mask only works with unsigned types");
return (static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __ctz) &
std::__trailing_mask<_StorageType>(__clz);
}

// This function is designed to operate correctly even for smaller integral types like `uint8_t`, `uint16_t`,
// or `unsigned short`. Casting back to _StorageType is crucial to prevent undefined behavior that can arise
// from integral promotions.
// or `unsigned short`.
// See https://github.com/llvm/llvm-project/pull/122410.
template <class _StoragePointer>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
Expand All @@ -79,12 +100,11 @@ __fill_masked_range(_StoragePointer __word, unsigned __clz, unsigned __ctz, bool
using _StorageType = typename pointer_traits<_StoragePointer>::element_type;
_LIBCPP_ASSERT_VALID_INPUT_RANGE(
__ctz + __clz < sizeof(_StorageType) * CHAR_BIT, "__fill_masked_range called with invalid range");
_StorageType __m = static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) >> __clz) &
static_cast<_StorageType>(static_cast<_StorageType>(~static_cast<_StorageType>(0)) << __ctz);
_StorageType __m = std::__middle_mask<_StorageType>(__clz, __ctz);
if (__fill_val)
*__word |= __m;
else
*__word &= static_cast<_StorageType>(~__m);
*__word &= ~__m;
}

template <class _Cp, bool = __has_storage_type<_Cp>::value>
Expand Down
9 changes: 6 additions & 3 deletions libcxx/include/__fwd/bit_reference.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@
#define _LIBCPP___FWD_BIT_REFERENCE_H

#include <__config>
#include <__memory/pointer_traits.h>
#include <__type_traits/enable_if.h>
#include <__type_traits/is_unsigned.h>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
Expand All @@ -30,6 +27,12 @@ template <class _StoragePointer>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
__fill_masked_range(_StoragePointer __word, unsigned __ctz, unsigned __clz, bool __fill_val);

template <class _StorageType>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __trailing_mask(unsigned __clz);

template <class _StorageType>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _StorageType __middle_mask(unsigned __clz, unsigned __ctz);

_LIBCPP_END_NAMESPACE_STD

#endif // _LIBCPP___FWD_BIT_REFERENCE_H
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// ADDITIONAL_COMPILE_FLAGS(cl-style-warnings): /wd4245 /wd4305 /wd4310 /wd4389 /wd4805
// ADDITIONAL_COMPILE_FLAGS(has-fconstexpr-steps): -fconstexpr-steps=20000000
// ADDITIONAL_COMPILE_FLAGS(has-fconstexpr-ops-limit): -fconstexpr-ops-limit=80000000
// XFAIL: FROZEN-CXX03-HEADERS-FIXME

// <algorithm>

Expand All @@ -31,6 +32,7 @@
#include <vector>
#include <type_traits>

#include "sized_allocator.h"
#include "test_macros.h"
#include "test_iterators.h"
#include "type_algorithms.h"
Expand Down Expand Up @@ -228,7 +230,8 @@ TEST_CONSTEXPR_CXX20 bool test() {

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

{ // Test vector<bool>::iterator optimization
{
// Test vector<bool>::iterator optimization
std::vector<bool> vec(256 + 8);
for (ptrdiff_t i = 8; i <= 256; i *= 2) {
for (size_t offset = 0; offset < 8; offset += 2) {
Expand All @@ -238,6 +241,39 @@ TEST_CONSTEXPR_CXX20 bool test() {
assert(std::find(vec.begin() + offset, vec.end(), false) == vec.begin() + offset + i);
}
}

// Verify that the std::vector<bool>::iterator optimization works properly for allocators with custom size types
// Fix https://github.com/llvm/llvm-project/issues/122528
{
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(100, false, Alloc(1));
in[in.size() - 2] = true;
assert(std::find(in.begin(), in.end(), true) == in.end() - 2);
}
{
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(199, false, Alloc(1));
in[in.size() - 2] = true;
assert(std::find(in.begin(), in.end(), true) == in.end() - 2);
}
{
using Alloc = sized_allocator<bool, unsigned short, short>;
std::vector<bool, Alloc> in(200, false, Alloc(1));
in[in.size() - 2] = true;
assert(std::find(in.begin(), in.end(), true) == in.end() - 2);
}
{
using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
std::vector<bool, Alloc> in(205, false, Alloc(1));
in[in.size() - 2] = true;
assert(std::find(in.begin(), in.end(), true) == in.end() - 2);
}
{
using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
std::vector<bool, Alloc> in(257, false, Alloc(1));
in[in.size() - 2] = true;
assert(std::find(in.begin(), in.end(), true) == in.end() - 2);
}
}

return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <vector>

#include "almost_satisfies_types.h"
#include "sized_allocator.h"
#include "test_iterators.h"

struct NotEqualityComparable {};
Expand Down Expand Up @@ -202,7 +203,8 @@ constexpr bool test() {
}
}

{ // Test vector<bool>::iterator optimization
{
// Test vector<bool>::iterator optimization
std::vector<bool> vec(256 + 8);
for (ptrdiff_t i = 8; i <= 256; i *= 2) {
for (size_t offset = 0; offset < 8; offset += 2) {
Expand All @@ -215,6 +217,39 @@ constexpr bool test() {
std::ranges::begin(vec) + offset + i);
}
}

// Verify that the std::vector<bool>::iterator optimization works properly for allocators with custom size types
// See https://github.com/llvm/llvm-project/issues/122528
{
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> in(100, false, Alloc(1));
in[in.size() - 2] = true;
assert(std::ranges::find(in, true) == in.end() - 2);
}
{
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> in(199, false, Alloc(1));
in[in.size() - 2] = true;
assert(std::ranges::find(in, true) == in.end() - 2);
}
{
using Alloc = sized_allocator<bool, unsigned short, short>;
std::vector<bool, Alloc> in(200, false, Alloc(1));
in[in.size() - 2] = true;
assert(std::ranges::find(in, true) == in.end() - 2);
}
{
using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
std::vector<bool, Alloc> in(205, false, Alloc(1));
in[in.size() - 2] = true;
assert(std::ranges::find(in, true) == in.end() - 2);
}
{
using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
std::vector<bool, Alloc> in(257, false, Alloc(1));
in[in.size() - 2] = true;
assert(std::ranges::find(in, true) == in.end() - 2);
}
}

return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
//
//===----------------------------------------------------------------------===//

// ADDITIONAL_COMPILE_FLAGS(has-fconstexpr-steps): -fconstexpr-steps=15000000

// bitset<N>& operator<<=(size_t pos); // constexpr since C++23

#include <bitset>
Expand All @@ -18,20 +20,20 @@

template <std::size_t N>
TEST_CONSTEXPR_CXX23 bool test_left_shift() {
std::vector<std::bitset<N> > const cases = get_test_cases<N>();
for (std::size_t c = 0; c != cases.size(); ++c) {
for (std::size_t s = 0; s <= N+1; ++s) {
std::bitset<N> v1 = cases[c];
std::bitset<N> v2 = v1;
v1 <<= s;
for (std::size_t i = 0; i < v1.size(); ++i)
if (i < s)
assert(v1[i] == 0);
else
assert(v1[i] == v2[i-s]);
}
std::vector<std::bitset<N> > const cases = get_test_cases<N>();
for (std::size_t c = 0; c != cases.size(); ++c) {
for (std::size_t s = 0; s <= N + 1; ++s) {
std::bitset<N> v1 = cases[c];
std::bitset<N> v2 = v1;
v1 <<= s;
for (std::size_t i = 0; i < v1.size(); ++i)
if (i < s)
assert(v1[i] == 0);
else
assert(v1[i] == v2[i - s]);
}
return true;
}
return true;
}

int main(int, char**) {
Expand Down