mclow.lists marked 7 inline comments as done.
mclow.lists added inline comments.
================
Comment at: libcxx/include/bit:236
+
+ if constexpr (sizeof(_Tp) <= sizeof(unsigned int))
+ return __clz(static_cast<unsigned int>(__t))
----------------
EricWF wrote:
> Weird indentation. Clang-format please.
The indentation is purposeful. It illustrates the structure of the code.
================
Comment at: libcxx/include/bit:240
+ else if constexpr (sizeof(_Tp) <= sizeof(unsigned long))
+ return __clz(static_cast<unsigned long>(__t))
+ - (numeric_limits<unsigned long>::digits -
numeric_limits<_Tp>::digits);
----------------
EricWF wrote:
> Do we even need `if constexpr` here?
Yes. When instantiated with `_Tp == unsigned long` (say), this should be a
single call to `__clz`, w/o any other code.
================
Comment at: libcxx/include/bit:251
+ while (true) {
+ __t = rotr(__t, __ulldigits);
+ if ((__iter = countl_zero(static_cast<unsigned long long>(__t)))
!= __ulldigits)
----------------
EricWF wrote:
> This call needs to be qualified.
I don't see why. `_Tp` must be an unsigned integral type.
================
Comment at: libcxx/include/bit:322
+ if constexpr (sizeof(_Tp) <= sizeof(unsigned int))
+ return __popcount(static_cast<unsigned int>(__t));
+ else if constexpr (sizeof(_Tp) <= sizeof(unsigned long))
----------------
EricWF wrote:
> Do we need `if constexpr` here.
Yes. See above.
================
Comment at: libcxx/include/bit:366
+#ifndef _LIBCPP_HAS_NO_BUILTIN_IS_CONSTANT_EVALUATED
+ if (!__builtin_is_constant_evaluated ())
+ _LIBCPP_DEBUG_ASSERT( __n != numeric_limits<_Tp>::digits, "Bad input
to ceil2" );
----------------
EricWF wrote:
> You don't need to guard debug assertions in C++14 constexpr functions.
Good to know; thanks.
================
Comment at: libcxx/include/bit:384
+log2p1(_Tp __t) noexcept
+{ return __t == 0 ? 0 : __bit_log2(__t) + 1; }
+
----------------
EricWF wrote:
> `__bit_log2` needs to be qualified to prevent ADL.
See above. ADL on what? There are no user-defined types here.
================
Comment at: libcxx/test/std/numerics/bit/bitops.count/countr_zero.fail.cpp:29
+{
+ (void) std::countr_zero(static_cast<int>(2)); // expected-error
{{no matching function for call to 'countr_zero'}}
+ (void) std::countr_zero(static_cast<signed int>(2)); // expected-error
{{no matching function for call to 'countr_zero'}}
----------------
EricWF wrote:
> Use `is_invocable`.
I can do that.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D51262/new/
https://reviews.llvm.org/D51262
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
