[libc-commits] [PATCH] D148759: [libc] Support constexpr uint initialization

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

Looks pretty good but I have some minor questions.



================
Comment at: libc/src/__support/FPUtil/x86_64/LongDoubleBits.h:52
   static constexpr UIntType MAX_NORMAL =
-      ((UIntType(MAX_EXPONENT) - 1)
+      ((UIntType(MAX_EXPONENT - 1))
        << (MantissaWidth<long double>::VALUE + 1)) |
----------------
Why was this change required?


================
Comment at: libc/src/__support/UInt.h:89
 
+  constexpr explicit operator uint16_t() const {
+    return uint16_t(uint64_t(*this));
----------------
Is this required for the functionality of this change?


================
Comment at: libc/src/__support/UInt.h:97
 
+  constexpr explicit operator bool() const { return (*this) == 1; }
+
----------------
Is this required for the functionality of this change? Also, wouldn't a more correct check be `*this != 0`?


================
Comment at: libc/src/__support/builtin_wrappers.h:88
 
+// This version is not because it's templated below if builtins are available.
+template <typename T>
----------------
Incomplete sentence?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148759/new/

https://reviews.llvm.org/D148759