[libcxx-commits] [libcxx] [libc++] Improve bitset::to_ullong Implementation (PR #121348)

[Peng Liu via libcxx-commits]

================
@@ -381,8 +382,9 @@ __bitset<_N_words, _Size>::to_ullong(true_type, true_type) const {
   unsigned long long __r = __first_[0];
   _LIBCPP_DIAGNOSTIC_PUSH
   _LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wshift-count-overflow")
-  for (size_t __i = 1; __i < sizeof(unsigned long long) / sizeof(__storage_type); ++__i)
-    __r |= static_cast<unsigned long long>(__first_[__i]) << (sizeof(__storage_type) * CHAR_BIT);
+  size_t __n_words = std::min<size_t>(_N_words, sizeof(unsigned long long) / sizeof(__storage_type));
----------------
winner245 wrote:

I agree that we should make `__n_words` a constant expression. Regarding the suggested change:

```cpp
const size_t __ull_words = sizeof(unsigned long long) / sizeof(__storage_type);
const size_t __n_words   = _N_words < __ull_words ? _N_words : __ull_words;
```

I am not sure if your intention of replacing `std::min` by the conditional expression is to avoid including the `<__algorithm/min.h>` header? I don't mind using a conditional expression instead of calling `std::min`, although this approach does introduce an extra variable, `__ull_words`. However, I would like to point out that `std::min` is currently used in several places within `bitset`, relying on the transitive inclusion of the `<__algorithm/min.h>` header.

https://github.com/llvm/llvm-project/pull/121348