[libc-commits] [PATCH] D83931: Update Test (EXPECT_EQ and friends) to accept __uint128_t and floating point types (float, double, long double).

[Siva Chandra via Phabricator via libc-commits]

sivachandra accepted this revision.
sivachandra added a comment.
This revision is now accepted and ready to land.

This is very well done. I have left nittiest of nits inline.



================
Comment at: libc/utils/UnitTest/Test.cpp:40
+          cpp::EnableIfType<cpp::IsIntegral<T>::Value, int> N = sizeof(T) * 2>
+std::string uintToHex(T X, bool LowerCase = false) {
+  std::string s(N, '0');
----------------
Nit: Do we need the flexibility in case? If not, just remove it?


================
Comment at: libc/utils/UnitTest/Test.cpp:45
+    unsigned char Mod = static_cast<unsigned char>(X) & 15;
+    *it = llvm::hexdigit(Mod, LowerCase);
+  }
----------------
Continuing the above comment: Always pass `true` with a comment attached saying "In upper case" or something similar?


================
Comment at: libc/utils/testutils/StreamWrapper.cpp:46
 template StreamWrapper &StreamWrapper::operator<<<std::string>(std::string t);
+template StreamWrapper &
+    StreamWrapper::operator<<<llvm::StringRef>(llvm::StringRef t);
----------------
Do we still need this for this change? May be, but just asking because I couldn't easily locate where.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83931