[libc-commits] [PATCH] D131943: [libc][NFC] Make IntegerToString simpler to use at call-sites.

[Michael Jones via Phabricator via libc-commits]

michaelrj added a comment.

Overall LGTM with some nits.



================
Comment at: libc/src/__support/integer_to_string.h:27
+//   auto str = IntegerToString::hex(a, hexbuf,
+//                                   true /* generate upper case characters */);
+//
----------------
This should be `false` for uppercase.


================
Comment at: libc/src/__support/integer_to_string.h:153
+                                            cpp::MutableArrayRef<char> buffer) {
+    return convert<10>(val, buffer, false);
   }
----------------
instead of specifying false here, you could leave this blank since `lowercase` has a default value already. Same below.


================
Comment at: libc/src/__support/threads/linux/thread.cpp:279
     THREAD_NAME_PATH_PREFIX.size() +
-    IntegerToString<int>::BUFSIZE + // Size of tid
-    1 +                             // For '/' character
+    IntegerToString::bufsize<10, int>() + // Size of tid
+    1 +                                   // For '/' character
----------------
you could use `dec_bufsize<int>` here


================
Comment at: libc/src/stdio/printf_core/int_converter.h:68
 
-  static constexpr size_t BUFSIZE = IntegerToString<uintmax_t, 8>::BUFSIZE;
-  char buff[BUFSIZE];
-  cpp::MutableArrayRef<char> bufref(buff, BUFSIZE);
-  auto str = num_to_strview(num, bufref, to_conv.conv_name);
+  char buf[IntegerToString::bufsize<8, intmax_t>()];
+  auto str = num_to_strview(num, buf, to_conv.conv_name);
----------------
you could use `oct_bufsize<intmax_t>` here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131943