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

Conversation

winner245
Copy link
Contributor

@winner245 winner245 commented Jan 12, 2025

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_types, an issue reported in #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_types. 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.

@winner245 winner245 force-pushed the fix-bitwise-logic-in-find branch 9 times, most recently from c4f700f to 4207592 Compare January 12, 2025 20:29
@winner245 winner245 force-pushed the fix-bitwise-logic-in-find branch from 4207592 to c5c8107 Compare February 5, 2025 04:15
Copy link

github-actions bot commented Feb 5, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@winner245 winner245 force-pushed the fix-bitwise-logic-in-find branch 7 times, most recently from bcde6ab to bd4c17d Compare February 5, 2025 23:49
@winner245 winner245 marked this pull request as ready for review February 6, 2025 15:20
@winner245 winner245 requested a review from a team as a code owner February 6, 2025 15:20
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

This PR fixes an ambiguous call encountered when using the std::ranges::find or std::find algorithms with vector&lt;bool&gt; with small size_types. The ambiguity arises from integral promotions during the internal bitwise arithmetic of the find algorithms for small integral types. This results in multiple viable candidates: __libcpp_ctz(unsigned), __libcpp_ctz(unsigned long), and __libcpp_ctz(unsigned long long), leading to an ambiguous call error. To fix this ambiguity, we invoke a dispatcher function __countr_zero which directs the call to the appropriate overload of __libcpp_ctz. This partially fixes #122528.

Additionally, we have enhanced the implementation of __countr_zero in the following two ways:

  • Constexpr enhancement: The function __countr_zero is now made constexpr starting from C++11, instead of C++14;
  • Runtime overhead reduction: We have eliminated runtime overhead by utilizing constexpr if for C++11 and above, while using SFINAE-based overloading for C++03.

Full diff: https://github.com/llvm/llvm-project/pull/122641.diff

6 Files Affected:

  • (modified) libcxx/include/__algorithm/find.h (+5-5)
  • (modified) libcxx/include/__bit/countr.h (+66-11)
  • (modified) libcxx/include/__bit_reference (+15)
  • (modified) libcxx/include/__fwd/bit_reference.h (+6-3)
  • (modified) libcxx/test/std/algorithms/alg.nonmodifying/alg.find/find.pass.cpp (+30)
  • (modified) libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp (+52-18)
diff --git a/libcxx/include/__algorithm/find.h b/libcxx/include/__algorithm/find.h
index 24b8b2f96443c95..a7d9374b3a1c89d 100644
--- a/libcxx/include/__algorithm/find.h
+++ b/libcxx/include/__algorithm/find.h
@@ -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;
@@ -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));
 }
diff --git a/libcxx/include/__bit/countr.h b/libcxx/include/__bit/countr.h
index 2f7571133bd03a3..e80be8a39520c20 100644
--- a/libcxx/include/__bit/countr.h
+++ b/libcxx/include/__bit/countr.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)
@@ -38,20 +40,26 @@ _LIBCPP_BEGIN_NAMESPACE_STD
   return __builtin_ctzll(__x);
 }
 
+#ifndef _LIBCPP_CXX03_LANG
+// constexpr implementation for C++11 and later
+
+// 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 constexpr int __countr_zero_impl(_Tp __t) _NOEXCEPT {
+  static_assert(is_unsigned<_Tp>::value, "__countr_zero_impl only works with unsigned types");
+  if constexpr (sizeof(_Tp) <= sizeof(unsigned int)) {
     return std::__libcpp_ctz(static_cast<unsigned int>(__t));
-  else if (sizeof(_Tp) <= sizeof(unsigned long))
+  } else if 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 constexpr (sizeof(_Tp) <= sizeof(unsigned long long)) {
     return std::__libcpp_ctz(static_cast<unsigned long long>(__t));
-  else {
+  } else {
+#  if _LIBCPP_STD_VER == 11
+    // A recursive constexpr implementation for C++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) {
@@ -59,8 +67,55 @@ template <class _Tp>
       __t >>= __ulldigits;
     }
     return __ret + std::__libcpp_ctz(static_cast<unsigned long long>(__t));
+#  endif
   }
-#endif // __has_builtin(__builtin_ctzg)
+}
+
+#else
+// implementation for C++03
+
+template < class _Tp, __enable_if_t<is_unsigned<_Tp>::value && sizeof(_Tp) <= sizeof(unsigned int), int> = 0>
+_LIBCPP_HIDE_FROM_ABI int __countr_zero_impl(_Tp __t) {
+  return std::__libcpp_ctz(static_cast<unsigned int>(__t));
+}
+
+template < class _Tp,
+           __enable_if_t<is_unsigned<_Tp>::value && (sizeof(_Tp) > sizeof(unsigned int)) &&
+                             sizeof(_Tp) <= sizeof(unsigned long),
+                         int> = 0 >
+_LIBCPP_HIDE_FROM_ABI int __countr_zero_impl(_Tp __t) {
+  return std::__libcpp_ctz(static_cast<unsigned long>(__t));
+}
+
+template < class _Tp,
+           __enable_if_t<is_unsigned<_Tp>::value && (sizeof(_Tp) > sizeof(unsigned long)) &&
+                             sizeof(_Tp) <= sizeof(unsigned long long),
+                         int> = 0 >
+_LIBCPP_HIDE_FROM_ABI int __countr_zero_impl(_Tp __t) {
+  return std::__libcpp_ctz(static_cast<unsigned long long>(__t));
+}
+
+template < class _Tp, __enable_if_t<is_unsigned<_Tp>::value && (sizeof(_Tp) > sizeof(unsigned long long)), int> = 0 >
+_LIBCPP_HIDE_FROM_ABI int __countr_zero_impl(_Tp __t) {
+  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 // _LIBCPP_CXX03_LANG
+
+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)
+  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
diff --git a/libcxx/include/__bit_reference b/libcxx/include/__bit_reference
index aad470394732c6e..5a44769c44b2db0 100644
--- a/libcxx/include/__bit_reference
+++ b/libcxx/include/__bit_reference
@@ -16,6 +16,7 @@
 #include <__algorithm/min.h>
 #include <__assert>
 #include <__bit/countr.h>
+#include <__bit/invert_if.h>
 #include <__compare/ordering.h>
 #include <__config>
 #include <__cstddef/ptrdiff_t.h>
@@ -62,6 +63,20 @@ struct __size_difference_type_traits<_Cp, __void_t<typename _Cp::difference_type
   using size_type       = typename _Cp::size_type;
 };
 
+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>(~static_cast<_StorageType>(0)) >> __clz);
+}
+
+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>(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.
diff --git a/libcxx/include/__fwd/bit_reference.h b/libcxx/include/__fwd/bit_reference.h
index d65f043e89ad614..73f3bc07736ddf9 100644
--- a/libcxx/include/__fwd/bit_reference.h
+++ b/libcxx/include/__fwd/bit_reference.h
@@ -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
@@ -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
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/find.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/find.pass.cpp
index c41246522fdebac..599b6355af565b1 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/find.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/find.pass.cpp
@@ -14,6 +14,7 @@
 // MSVC warning C4389: '==': signed/unsigned mismatch
 // MSVC warning C4805: '==': unsafe mix of type 'char' and type 'bool' in operation
 // ADDITIONAL_COMPILE_FLAGS(cl-style-warnings): /wd4245 /wd4305 /wd4310 /wd4389 /wd4805
+// XFAIL: FROZEN-CXX03-HEADERS-FIXME
 
 // <algorithm>
 
@@ -28,6 +29,7 @@
 #include <vector>
 #include <type_traits>
 
+#include "sized_allocator.h"
 #include "test_macros.h"
 #include "test_iterators.h"
 #include "type_algorithms.h"
@@ -206,6 +208,33 @@ struct TestIntegerPromotions {
   }
 };
 
+TEST_CONSTEXPR_CXX20 void test_bit_iterator_with_custom_sized_types() {
+  {
+    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, std::uint32_t, std::int32_t>;
+    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::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);
+  }
+}
+
 TEST_CONSTEXPR_CXX20 bool test() {
   types::for_each(types::integer_types(), TestTypes<char>());
   types::for_each(types::integer_types(), TestTypes<int>());
@@ -226,6 +255,7 @@ TEST_CONSTEXPR_CXX20 bool test() {
 #endif
 
   types::for_each(types::integral_types(), TestIntegerPromotions());
+  test_bit_iterator_with_custom_sized_types();
 
   return true;
 }
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp
index 4ae049c3ec00115..d596e3a5c7970b8 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp
@@ -31,6 +31,7 @@
 #include <vector>
 
 #include "almost_satisfies_types.h"
+#include "sized_allocator.h"
 #include "test_iterators.h"
 
 struct NotEqualityComparable {};
@@ -66,14 +67,14 @@ constexpr void test_iterators() {
   using ValueT = std::iter_value_t<It>;
   { // simple test
     {
-      ValueT a[] = {1, 2, 3, 4};
+      ValueT a[]                = {1, 2, 3, 4};
       std::same_as<It> auto ret = std::ranges::find(It(a), Sent(It(a + 4)), 4);
       assert(base(ret) == a + 3);
       assert(*ret == 4);
     }
     {
-      ValueT a[] = {1, 2, 3, 4};
-      auto range = std::ranges::subrange(It(a), Sent(It(a + 4)));
+      ValueT a[]                = {1, 2, 3, 4};
+      auto range                = std::ranges::subrange(It(a), Sent(It(a + 4)));
       std::same_as<It> auto ret = std::ranges::find(range, 4);
       assert(base(ret) == a + 3);
       assert(*ret == 4);
@@ -83,13 +84,13 @@ constexpr void test_iterators() {
   { // check that an empty range works
     {
       std::array<ValueT, 0> a = {};
-      auto ret = std::ranges::find(It(a.data()), Sent(It(a.data())), 1);
+      auto ret                = std::ranges::find(It(a.data()), Sent(It(a.data())), 1);
       assert(base(ret) == a.data());
     }
     {
       std::array<ValueT, 0> a = {};
-      auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data())));
-      auto ret = std::ranges::find(range, 1);
+      auto range              = std::ranges::subrange(It(a.data()), Sent(It(a.data())));
+      auto ret                = std::ranges::find(range, 1);
       assert(base(ret) == a.data());
     }
   }
@@ -97,12 +98,12 @@ constexpr void test_iterators() {
   { // check that last is returned with no match
     {
       ValueT a[] = {1, 1, 1};
-      auto ret = std::ranges::find(a, a + 3, 0);
+      auto ret   = std::ranges::find(a, a + 3, 0);
       assert(ret == a + 3);
     }
     {
       ValueT a[] = {1, 1, 1};
-      auto ret = std::ranges::find(a, 0);
+      auto ret   = std::ranges::find(a, 0);
       assert(ret == a + 3);
     }
   }
@@ -120,6 +121,33 @@ class TriviallyComparable {
   bool operator==(const TriviallyComparable&) const = default;
 };
 
+constexpr void test_bit_iterator_with_custom_sized_types() {
+  {
+    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, std::uint32_t, std::int32_t>;
+    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::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);
+  }
+}
+
 constexpr bool test() {
   types::for_each(types::type_list<char, wchar_t, int, long, TriviallyComparable<char>, TriviallyComparable<wchar_t>>{},
                   []<class T> {
@@ -148,7 +176,7 @@ constexpr bool test() {
         int comp;
         int other;
       };
-      S a[] = { {0, 0}, {0, 2}, {0, 1} };
+      S a[]    = {{0, 0}, {0, 2}, {0, 1}};
       auto ret = std::ranges::find(a, 0, &S::comp);
       assert(ret == a);
       assert(ret->comp == 0);
@@ -159,7 +187,7 @@ constexpr bool test() {
         int comp;
         int other;
       };
-      S a[] = { {0, 0}, {0, 2}, {0, 1} };
+      S a[]    = {{0, 0}, {0, 2}, {0, 1}};
       auto ret = std::ranges::find(a, a + 3, 0, &S::comp);
       assert(ret == a);
       assert(ret->comp == 0);
@@ -169,7 +197,7 @@ constexpr bool test() {
 
   {
     // check that an iterator is returned with a borrowing range
-    int a[] = {1, 2, 3, 4};
+    int a[]                     = {1, 2, 3, 4};
     std::same_as<int*> auto ret = std::ranges::find(std::views::all(a), 1);
     assert(ret == a);
     assert(*ret == 1);
@@ -178,23 +206,31 @@ 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::find(a, a + 4, 2, [&](int i) { ++projection_count; return i; });
+      auto ret             = std::ranges::find(a, a + 4, 2, [&](int i) {
+        ++projection_count;
+        return i;
+      });
       assert(ret == a + 1);
       assert(*ret == 2);
       assert(projection_count == 2);
     }
     {
-      int a[] = {1, 2, 3, 4};
+      int a[]              = {1, 2, 3, 4};
       int projection_count = 0;
-      auto ret = std::ranges::find(a, 2, [&](int i) { ++projection_count; return i; });
+      auto ret             = std::ranges::find(a, 2, [&](int i) {
+        ++projection_count;
+        return i;
+      });
       assert(ret == a + 1);
       assert(*ret == 2);
       assert(projection_count == 2);
     }
   }
 
+  test_bit_iterator_with_custom_sized_types();
+
   return true;
 }
 
@@ -210,9 +246,7 @@ class Comparable {
           return size;
         }()) {}
 
-  bool operator==(const Comparable& other) const {
-    return comparable_data[other.index_] == comparable_data[index_];
-  }
+  bool operator==(const Comparable& other) const { return comparable_data[other.index_] == comparable_data[index_]; }
 
   friend bool operator==(const Comparable& lhs, long long rhs) { return comparable_data[lhs.index_] == rhs; }
 };

@winner245 winner245 force-pushed the fix-bitwise-logic-in-find branch 4 times, most recently from db2ced2 to ac37a90 Compare February 12, 2025 04:53
@winner245 winner245 force-pushed the fix-bitwise-logic-in-find branch 3 times, most recently from 642a502 to 57c0e7f Compare February 23, 2025 03:27
@winner245 winner245 force-pushed the fix-bitwise-logic-in-find branch 2 times, most recently from 0eaf979 to 33708cf Compare February 23, 2025 19:44
@@ -38,29 +40,45 @@ _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.

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)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#if __has_builtin(__builtin_ctzg)
#if __has_builtin(__builtin_ctzg) // TODO(LLVM 21): This can be dropped once we only support Clang >= 19.

@winner245 winner245 force-pushed the fix-bitwise-logic-in-find branch from 33708cf to ad2d346 Compare February 26, 2025 21:03
@winner245 winner245 force-pushed the fix-bitwise-logic-in-find branch from ad2d346 to bcd553d Compare March 3, 2025 22:44
@ldionne ldionne merged commit 3bd71cb into llvm:main Mar 13, 2025
85 checks passed
@winner245 winner245 deleted the fix-bitwise-logic-in-find branch March 13, 2025 20:05
frederik-h pushed a commit to frederik-h/llvm-project that referenced this pull request Mar 18, 2025
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants