Unify __countr_zero_impl for C++03 and later

Author: winner245

libcxx/include/__bit/countr.h

@@ -12,10 +12,10 @@
 #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/enable_if.h>
 #include <__type_traits/is_unsigned.h>
 #include <limits>
 
@@ -40,74 +40,37 @@ _LIBCPP_BEGIN_NAMESPACE_STD
   return __builtin_ctzll(__x);
 }
 
-#ifndef _LIBCPP_CXX03_LANG
-// constexpr implementation for C++11 and later
-
+// 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 constexpr int __countr_zero_impl(_Tp __t) _NOEXCEPT {
+[[__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 constexpr (sizeof(_Tp) <= sizeof(unsigned int)) {
+  // Use constexpr if as a C++17 extension for clang
+  if _LIBCPP_CONSTEXPR (sizeof(_Tp) <= sizeof(unsigned int)) {
     return std::__libcpp_ctz(static_cast<unsigned int>(__t));
-  } else if constexpr (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 constexpr (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 {
-#  if _LIBCPP_STD_VER == 11
-    // A recursive constexpr implementation for C++11
+#if _LIBCPP_STD_VER == 11
+    // A constexpr implementation for C++11 using variable declaration as a C++14 extension for clang
     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
+#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
-  }
-}
-
-#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;
+#endif
   }
-  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");

libcxx/include/__bit_reference

@@ -18,7 +18,6 @@
 #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>
@@ -68,12 +67,21 @@ 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 behaviors or
+// ambiguities due to integral promotions for the small integral types, all 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>(~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");

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

@@ -211,7 +211,29 @@ struct TestIntegerPromotions {
   }
 };
 
-TEST_CONSTEXPR_CXX20 void test_bit_iterator_with_custom_sized_types() {
+TEST_CONSTEXPR_CXX20 bool test() {
+  types::for_each(types::integer_types(), TestTypes<char>());
+  types::for_each(types::integer_types(), TestTypes<int>());
+  types::for_each(types::integer_types(), TestTypes<long long>());
+#if TEST_STD_VER >= 20
+  Test<TriviallyComparable<char>, TriviallyComparable<char>>().operator()<TriviallyComparable<char>*>();
+  Test<TriviallyComparable<wchar_t>, TriviallyComparable<wchar_t>>().operator()<TriviallyComparable<wchar_t>*>();
+#endif
+
+  // TODO: Remove the `_LIBCPP_ENABLE_EXPERIMENTAL` check once we have the FTM guarded or views::join isn't
+  // experimental anymore
+#if TEST_STD_VER >= 20 && (!defined(_LIBCPP_VERSION) || defined(_LIBCPP_ENABLE_EXPERIMENTAL))
+  {
+    std::vector<std::vector<int>> vec = {{1, 2, 3}, {4, 5, 6}, {7, 8, 9}};
+    auto view                         = vec | std::views::join;
+    assert(std::find(view.begin(), view.end(), 4) == std::next(view.begin(), 3));
+  }
+#endif
+
+  types::for_each(types::integral_types(), TestIntegerPromotions());
+
+  // 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));
@@ -236,27 +258,6 @@ TEST_CONSTEXPR_CXX20 void test_bit_iterator_with_custom_sized_types() {
     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>());
-  types::for_each(types::integer_types(), TestTypes<long long>());
-#if TEST_STD_VER >= 20
-  Test<TriviallyComparable<char>, TriviallyComparable<char>>().operator()<TriviallyComparable<char>*>();
-  Test<TriviallyComparable<wchar_t>, TriviallyComparable<wchar_t>>().operator()<TriviallyComparable<wchar_t>*>();
-#endif
-
-#if TEST_STD_VER >= 20
-  {
-    std::vector<std::vector<int>> vec = {{1, 2, 3}, {4, 5, 6}, {7, 8, 9}};
-    auto view                         = vec | std::views::join;
-    assert(std::find(view.begin(), view.end(), 4) == std::next(view.begin(), 3));
-  }
-#endif
-
-  types::for_each(types::integral_types(), TestIntegerPromotions());
-  test_bit_iterator_with_custom_sized_types();
 
   { // Test vector<bool>::iterator optimization
     std::vector<bool> vec(256 + 8);

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

@@ -124,33 +124,6 @@ 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> {
@@ -245,7 +218,32 @@ constexpr bool test() {
     }
   }
 
-  test_bititer_with_custom_sized_types();
+  // 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, 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);
+  }
 
   return true;
 }