[libc-commits] [PATCH] D110643: [libc] Add support for 128 bit ints in limits.h

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/test/utils/CPP/limits_test.cpp:15
+TEST(LlvmLibcLimitsTest, LimitsFollowSpec) {
+  ASSERT_GE(__llvm_libc::cpp::NumericLimits<int>::max(), 32767);
+  ASSERT_LE(__llvm_libc::cpp::NumericLimits<int>::min(), -32767);
----------------
Instead of literals, we should use the macros like `INT_MAX` etc. Then, you can make these to be equality checks. The test might seem trivial at that point - we are probably only testing for copy-paste errors. But, the macros make it more robust against platform differences.


================
Comment at: libc/utils/CPP/Limits.h:55
 };
+template <> class NumericLimits<__uint128_t> {
+public:
----------------
For completeness, should we add the limits of the signed `__int128_t` type?


================
Comment at: libc/utils/CPP/Limits.h:58
+  static constexpr __uint128_t max() {
+    return (((__uint128_t(1) << 127) - 1) << 1) + 1;
+  }
----------------
Can this be `~__uint128_t(0)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110643