[libc-commits] [PATCH] D130227: [libc] Add a convenience class and function for integer to string conversion.

[Michael Jones via Phabricator via libc-commits]

michaelrj added a comment.

overall LGTM, with a few comments



================
Comment at: libc/src/__support/integer_to_string.h:33-34
+  //
+  // NOTE: We need the ceil operation only when sizeof(T) is 1. So, we
+  // special case below instead of actually using a ceil operation.
+  //
----------------
for integer division you can just add (divisor - 1) to the initial number to get ceil, so in this case it would be `((sizeof(T) * 5) + 1) / 2)`


================
Comment at: libc/test/src/__support/integer_to_string_test.cpp:20
+TEST(LlvmLibcIntegerToStringTest, UINT8) {
+  EXPECT_TRUE(integer_to_string(uint8_t(0)).str().equals(StringView("0")));
+  EXPECT_TRUE(integer_to_string(uint8_t(1)).str().equals(StringView("1")));
----------------
would it be better to implement an equality checker for string views?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130227