[libcxx-commits] [PATCH] D102328: [libc++] Fix __bitop_unsigned_integer, rename to __is_unsigned_integer, and fix constexprness of std::countr_zero(__uint128_t)

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed May 12 12:42:36 PDT 2021

Quuxplusone added inline comments.

Comment at: libcxx/include/bit:84
-    static_assert(__bitop_unsigned_integer<_Tp>::value, "__rotl requires unsigned");
+    static_assert(is_unsigned<_Tp>::value, "__rotl requires an unsigned integer type");
     const unsigned int __dig = numeric_limits<_Tp>::digits;
I relaxed the static_assert on the internal functions for the same (bad) intuitive reason that the internal functions are provided in the first place. We never actually //use// any of these internal functions, but //if we did//, it might be nice to be able to use them on, I dunno, `char` or something. I'm highly ambivalent about whether we should be asserting anything at all, or if we do, what it should be. So I'll take suggestions if anyone's got any.

Comment at: libcxx/include/bit:119-125
         const unsigned int __ulldigits = numeric_limits<unsigned long long>::digits;
-        while ((__iter = __libcpp_ctz(static_cast<unsigned long long>(__t))) == __ulldigits)
+        while (static_cast<unsigned long long>(__t) == 0uLL)
-            __ret += __iter;
+            __ret += __ulldigits;
             __t >>= __ulldigits;
+        return __ret + __libcpp_ctz(static_cast<unsigned long long>(__t));
This is a bugfix: `std::countr_zero(__uint128_t(1) << 64)` had been trying to evaluate `__libcpp_ctz(0uLL)` in constexpr context, and that's not allowed. The old tests didn't catch it but the new tests do.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list