Quuxplusone added inline comments.
================
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);
----------------
mclow.lists wrote:
> 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.
The optimizer should take care of that just fine. But, `if constexpr` also
prevents us from unnecessarily trying to instantiate, say,
`static_cast<unsigned long long>(__t)`, in the case that `__t` is of a
user-defined type where that conversion's definition is ill-formed or
explicitly deleted. I'm still extremely skeptical that any user-defined type is
ever allowed to reach this code without UB; but if it is, then instantiating
just the bare minimum of conversions might be a QoI issue. (The standard
doesn't say anything about what this template should do for user-defined types,
so keeping the surface area small is a good idea.)
================
Comment at: libcxx/include/bit:384
+log2p1(_Tp __t) noexcept
+{ return __t == 0 ? 0 : __bit_log2(__t) + 1; }
+
----------------
mclow.lists wrote:
> EricWF wrote:
> > `__bit_log2` needs to be qualified to prevent ADL.
> See above. ADL on what? There are no user-defined types here.
FWIW, my intuition still agrees with Eric's: adding `_VSTD::` throughout might
not help anything, but if it wouldn't hurt, //and// if it stops every future
code-reviewer from making the same exact comment about ADL, shouldn't you just
do it? Wouldn't it save everyone some time?
================
Comment at: libcxx/test/std/numerics/bit/nothing_to_do.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
Peanut gallery says: I thought `nothing_to_do.pass.cpp` files were obsolete
these days?
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
