[libc++][NFC] Merges unused functions in callers.

[Mordante]

Summary

This is a follow up based on a request of @jloser in D127594.

As drive-by qualified the function calls in the <bit> header.

[Mordante]

Attempt to fix CI and improve a unit test.

[jloser]

libcxx/test/libcxx/numerics/bit.ops.pass.cpp

34

This is a drop in test coverage - I think we should keep this one for __bit_log2. In general, I think we should:

• Keep existing test coverage for the private functions kept (i.e. ones that had two or more call sites used by public functions).
• Verify existing coverage for public functions where we removed the private function it delegated to if we removed the private function (since it only had one call site).

How does that sound? If you're up for it, we could go one step further and verify the noexcept properties.

[Mordante]

Thanks for the reviews!

libcxx/test/libcxx/numerics/bit.ops.pass.cpp

34

I disagree it's a drop in test coverage. The function is indirectly tested by its callers.
But I don't mind to restore the test, so I'll add it back.

This is the only function "dropped" the other two remaining functions are still tested.
There is test coverage for the public functions, I didn't verify all, but I trust our contributors and reviewers. These tests are more detailed than the sanity checks in this file. For example test/std/numerics/bit/bitops.count/countl_one.pass.cpp.

I think it's useful to validate the noexcept status when it's not mandated by the Standard.

[Mordante]

Rebased and addresses review comments.

[Mordante]

Poke CI

[jloser]

libcxx/test/libcxx/numerics/bit.ops.pass.cpp

34

Valid point. It is indirectly tested with bit_floor and bit_width, though I do like the direct testing of it - thanks for adding that back.

Appreciate the added tests in general - LGTM!

libcxx/include/bit

 #include <__bits> // __libcpp_clz
 #include <__concepts/arithmetic.h>
 #include <__config>
 #include <limits>
 #include <type_traits>
 #include <version>
 
 #if defined(__IBMCPP__)
-#include "__support/ibm/support.h"
+#  include "__support/ibm/support.h"
 #endif
 #if defined(_LIBCPP_COMPILER_MSVC)
-#include <intrin.h>
+#  include <intrin.h>
 #endif
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template<class _Tp>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
-_Tp __rotl(_Tp __t, unsigned int __cnt) _NOEXCEPT
-{
-    static_assert(__libcpp_is_unsigned_integer<_Tp>::value, "__rotl requires an unsigned integer type");
-    const unsigned int __dig = numeric_limits<_Tp>::digits;
-    if ((__cnt % __dig) == 0)
-        return __t;
-    return (__t << (__cnt % __dig)) | (__t >> (__dig - (__cnt % __dig)));
-}
-
-template<class _Tp>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
 _Tp __rotr(_Tp __t, unsigned int __cnt) _NOEXCEPT
 {
     static_assert(__libcpp_is_unsigned_integer<_Tp>::value, "__rotr requires an unsigned integer type");
     const unsigned int __dig = numeric_limits<_Tp>::digits;
     if ((__cnt % __dig) == 0)
         return __t;
     return (__t >> (__cnt % __dig)) | (__t << (__dig - (__cnt % __dig)));
 }
 
 template<class _Tp>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
-int __countr_zero(_Tp __t) _NOEXCEPT
-{
-    static_assert(__libcpp_is_unsigned_integer<_Tp>::value, "__countr_zero requires an unsigned integer type");
-    if (__t == 0)
-        return numeric_limits<_Tp>::digits;
-
-    if      (sizeof(_Tp) <= sizeof(unsigned int))
-        return __libcpp_ctz(static_cast<unsigned int>(__t));
-    else if (sizeof(_Tp) <= sizeof(unsigned long))
-        return __libcpp_ctz(static_cast<unsigned long>(__t));
-    else if (sizeof(_Tp) <= sizeof(unsigned long long))
-        return __libcpp_ctz(static_cast<unsigned long long>(__t));
-    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 + __libcpp_ctz(static_cast<unsigned long long>(__t));
-    }
-}
-
-template<class _Tp>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
 int __countl_zero(_Tp __t) _NOEXCEPT
 {
     static_assert(__libcpp_is_unsigned_integer<_Tp>::value, "__countl_zero requires an unsigned integer type");
     if (__t == 0)
         return numeric_limits<_Tp>::digits;
 
     if      (sizeof(_Tp) <= sizeof(unsigned int))
-        return __libcpp_clz(static_cast<unsigned int>(__t))
+        return std::__libcpp_clz(static_cast<unsigned int>(__t))
               - (numeric_limits<unsigned int>::digits - numeric_limits<_Tp>::digits);
     else if (sizeof(_Tp) <= sizeof(unsigned long))
-        return __libcpp_clz(static_cast<unsigned long>(__t))
+        return std::__libcpp_clz(static_cast<unsigned long>(__t))
               - (numeric_limits<unsigned long>::digits - numeric_limits<_Tp>::digits);
     else if (sizeof(_Tp) <= sizeof(unsigned long long))
-        return __libcpp_clz(static_cast<unsigned long long>(__t))
+        return std::__libcpp_clz(static_cast<unsigned long long>(__t))
               - (numeric_limits<unsigned long long>::digits - numeric_limits<_Tp>::digits);
     else
     {
         int __ret = 0;
         int __iter = 0;
         const unsigned int __ulldigits = numeric_limits<unsigned long long>::digits;
         while (true) {
-            __t = __rotr(__t, __ulldigits);
-            if ((__iter = __countl_zero(static_cast<unsigned long long>(__t))) != __ulldigits)
+            __t = std::__rotr(__t, __ulldigits);
+            if ((__iter = std::__countl_zero(static_cast<unsigned long long>(__t))) != __ulldigits)
                 break;
             __ret += __iter;
             }
         return __ret + __iter;
     }
 }
 
-template<class _Tp>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
-int __countl_one(_Tp __t) _NOEXCEPT
-{
-    static_assert(__libcpp_is_unsigned_integer<_Tp>::value, "__countl_one requires an unsigned integer type");
-    return __t != numeric_limits<_Tp>::max()
-        ? __countl_zero(static_cast<_Tp>(~__t))
-        : numeric_limits<_Tp>::digits;
-}
-
-template<class _Tp>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
-int __countr_one(_Tp __t) _NOEXCEPT
-{
-    static_assert(__libcpp_is_unsigned_integer<_Tp>::value, "__countr_one requires an unsigned integer type");
-    return __t != numeric_limits<_Tp>::max()
-        ? __countr_zero(static_cast<_Tp>(~__t))
-        : numeric_limits<_Tp>::digits;
-}
-
-template<class _Tp>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
-int __popcount(_Tp __t) _NOEXCEPT
-{
-    static_assert(__libcpp_is_unsigned_integer<_Tp>::value, "__popcount requires an unsigned integer type");
-    if      (sizeof(_Tp) <= sizeof(unsigned int))
-        return __libcpp_popcount(static_cast<unsigned int>(__t));
-    else if (sizeof(_Tp) <= sizeof(unsigned long))
-        return __libcpp_popcount(static_cast<unsigned long>(__t));
-    else if (sizeof(_Tp) <= sizeof(unsigned long long))
-        return __libcpp_popcount(static_cast<unsigned long long>(__t));
-    else
-    {
-        int __ret = 0;
-        while (__t != 0)
-        {
-            __ret += __libcpp_popcount(static_cast<unsigned long long>(__t));
-            __t >>= numeric_limits<unsigned long long>::digits;
-        }
-        return __ret;
-    }
-}
-
-// integral log base 2
-template<class _Tp>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11
-unsigned __bit_log2(_Tp __t) _NOEXCEPT
-{
-    static_assert(__libcpp_is_unsigned_integer<_Tp>::value, "__bit_log2 requires an unsigned integer type");
-    return numeric_limits<_Tp>::digits - 1 - __countl_zero(__t);
-}
-
-template <class _Tp>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
-bool __has_single_bit(_Tp __t) _NOEXCEPT
-{
-    static_assert(__libcpp_is_unsigned_integer<_Tp>::value, "__has_single_bit requires an unsigned integer type");
-    return __t != 0 && (((__t & (__t - 1)) == 0));
-}
-
 #if _LIBCPP_STD_VER > 17
 
 template <__libcpp_unsigned_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr _Tp rotl(_Tp __t, unsigned int __cnt) noexcept {
-  return __rotl(__t, __cnt);
+  const unsigned int __dig = numeric_limits<_Tp>::digits;
+  if ((__cnt % __dig) == 0)
+    return __t;
+  return (__t << (__cnt % __dig)) | (__t >> (__dig - (__cnt % __dig)));
 }
 
 template <__libcpp_unsigned_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr _Tp rotr(_Tp __t, unsigned int __cnt) noexcept {
-  return __rotr(__t, __cnt);
+  return std::__rotr(__t, __cnt);
 }
 
 template <__libcpp_unsigned_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr int countl_zero(_Tp __t) noexcept {
-  return __countl_zero(__t);
+  return std::__countl_zero(__t);
 }
 
 template <__libcpp_unsigned_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr int countl_one(_Tp __t) noexcept {
-  return __countl_one(__t);
+  return __t != numeric_limits<_Tp>::max() ? std::countl_zero(static_cast<_Tp>(~__t)) : numeric_limits<_Tp>::digits;
 }
 
 template <__libcpp_unsigned_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr int countr_zero(_Tp __t) noexcept {
-  return __countr_zero(__t);
+  if (__t == 0)
+    return numeric_limits<_Tp>::digits;
+
+  if (sizeof(_Tp) <= sizeof(unsigned int))
+    return std::__libcpp_ctz(static_cast<unsigned int>(__t));
+  else if (sizeof(_Tp) <= sizeof(unsigned long))
+    return std::__libcpp_ctz(static_cast<unsigned long>(__t));
+  else if (sizeof(_Tp) <= sizeof(unsigned long long))
+    return std::__libcpp_ctz(static_cast<unsigned long long>(__t));
+  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));
+  }
 }
 
 template <__libcpp_unsigned_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr int countr_one(_Tp __t) noexcept {
-  return __countr_one(__t);
+  return __t != numeric_limits<_Tp>::max() ? std::countr_zero(static_cast<_Tp>(~__t)) : numeric_limits<_Tp>::digits;
 }
 
 template <__libcpp_unsigned_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr int popcount(_Tp __t) noexcept {
-  return __popcount(__t);
+  if (sizeof(_Tp) <= sizeof(unsigned int))
+    return std::__libcpp_popcount(static_cast<unsigned int>(__t));
+  else if (sizeof(_Tp) <= sizeof(unsigned long))
+    return std::__libcpp_popcount(static_cast<unsigned long>(__t));
+  else if (sizeof(_Tp) <= sizeof(unsigned long long))
+    return std::__libcpp_popcount(static_cast<unsigned long long>(__t));
+  else {
+    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;
+  }
 }
 
 template <__libcpp_unsigned_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr bool has_single_bit(_Tp __t) noexcept {
-  return __has_single_bit(__t);
+  return __t != 0 && (((__t & (__t - 1)) == 0));
+}
+
+// integral log base 2
+template <__libcpp_unsigned_integer _Tp>
+_LIBCPP_HIDE_FROM_ABI constexpr _Tp __bit_log2(_Tp __t) noexcept {
+  return numeric_limits<_Tp>::digits - 1 - std::countl_zero(__t);
 }
 
 template <__libcpp_unsigned_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr _Tp bit_floor(_Tp __t) noexcept {
-  return __t == 0 ? 0 : _Tp{1} << __bit_log2(__t);
+  return __t == 0 ? 0 : _Tp{1} << std::__bit_log2(__t);
 }
 
 template <__libcpp_unsigned_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr _Tp bit_ceil(_Tp __t) noexcept {
   if (__t < 2)
     return 1;
-  const unsigned __n = numeric_limits<_Tp>::digits - countl_zero((_Tp)(__t - 1u));
+  const unsigned __n = numeric_limits<_Tp>::digits - std::countl_zero((_Tp)(__t - 1u));
   _LIBCPP_ASSERT(__n != numeric_limits<_Tp>::digits, "Bad input to bit_ceil");
 
   if constexpr (sizeof(_Tp) >= sizeof(unsigned))
     return _Tp{1} << __n;
   else {
     const unsigned __extra = numeric_limits<unsigned>::digits - numeric_limits<_Tp>::digits;
     const unsigned __retVal = 1u << (__n + __extra);
     return (_Tp)(__retVal >> __extra);
   }
 }
 
 template <__libcpp_unsigned_integer _Tp>
 _LIBCPP_HIDE_FROM_ABI constexpr int bit_width(_Tp __t) noexcept {
-  return __t == 0 ? 0 : __bit_log2(__t) + 1;
+  return __t == 0 ? 0 : std::__bit_log2(__t) + 1;
 }
 
-enum class endian
-{
-    little = 0xDEAD,
-    big    = 0xFACE,
-#if defined(_LIBCPP_LITTLE_ENDIAN)
-    native = little
-#elif defined(_LIBCPP_BIG_ENDIAN)
-    native = big
-#else
-    native = 0xCAFE
-#endif
+enum class endian {
+  big = 0xFACE,
+#  if defined(_LIBCPP_LITTLE_ENDIAN)
+#  elif defined(_LIBCPP_BIG_ENDIAN)
+#  else
+#  endif
 };
 
 #endif // _LIBCPP_STD_VER > 17
 
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS
 
 #endif // _LIBCPP_BIT

libcxx/test/libcxx/numerics/bit.ops.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 // Test the __XXXX routines in the <bit> header.
 // These are not supposed to be exhaustive tests, just sanity checks.
 
 #include <bit>
 #include <cassert>
 
 #include "test_macros.h"
 
-int main(int, char **)
-{
-
-
-#if TEST_STD_VER > 11
-    {
-    constexpr unsigned v = 0x1237U;
-
-//  These are all constexpr in C++14 and later
-    static_assert( std::__rotl(v, 4) == 0x00012370, "");
-    static_assert( std::__rotr(v, 4) == 0x70000123, "");
-    static_assert( std::__countl_one(v)  == 0, "");
-    static_assert( std::__countr_one(v)  == 3, "");
-    static_assert( std::__countl_zero(v) == 19, "");
-    static_assert( std::__countr_zero(v) == 0, "");
-
-    static_assert( std::__libcpp_popcount(v) == 7, "");
-    static_assert( std::__bit_log2(v) == 12, "");
-    static_assert(!std::__has_single_bit(v), "");
-    }
-#endif
-
-    {
-    const unsigned v = 0x12345678;
+TEST_CONSTEXPR_CXX14 bool test() {
 
-    ASSERT_SAME_TYPE(unsigned, decltype(std::__rotl(v, 3)));
-    ASSERT_SAME_TYPE(unsigned, decltype(std::__rotr(v, 3)));
-
-    ASSERT_SAME_TYPE(int, decltype(std::__countl_one(v)));
-    ASSERT_SAME_TYPE(int, decltype(std::__countr_one(v)));
-    ASSERT_SAME_TYPE(int, decltype(std::__countl_zero(v)));
-    ASSERT_SAME_TYPE(int, decltype(std::__countr_zero(v)));
-
-    ASSERT_SAME_TYPE(int,      decltype(std::__libcpp_popcount(v)));
-    ASSERT_SAME_TYPE(unsigned, decltype(std::__bit_log2(v)));
-    ASSERT_SAME_TYPE(bool,     decltype(std::__has_single_bit(v)));
-
 
-    assert( std::__rotl(v, 3) == 0x91a2b3c0U);
-    assert( std::__rotr(v, 3) == 0x02468acfU);
-
-    assert( std::__countl_one(v)  == 0);
-    assert( std::__countr_one(v)  == 0);
-    assert( std::__countl_zero(v) == 3);
-    assert( std::__countr_zero(v) == 3);
+  assert(std::__rotr(v, 3) == 0x02468acfU);
+  assert(std::__countl_zero(v) == 3);
 
-    assert( std::__libcpp_popcount(v) == 13);
-    assert( std::__bit_log2(v) == 28);
-    assert(!std::__has_single_bit(v));
-    }
+#if TEST_STD_VER > 17
+  ASSERT_SAME_TYPE(unsigned, decltype(std::__bit_log2(v)));
+  assert(std::__bit_log2(v) == 28);
+#endif
+
+  return true;
 
-    return 0;
+int main(int, char**) {
+  test();
+#if TEST_STD_VER > 11
+  static_assert(test(), "");
+#endif
+
 }