[libc-commits] [PATCH] D131301: [libc] add int to string for extended bases

Tue Ly via Phabricator via libc-commits libc-commits at lists.llvm.org
Fri Aug 5 19:05:10 PDT 2022

lntue added inline comments.

Comment at: libc/src/__support/integer_to_string.h:82
+constexpr size_t floor_log_2(size_t num) {
+  size_t i = 0;
Look like a good candidate for `clz` builtin, unless it's only used for compile-time constants.  In that case, it should be ok to bump this to C++20 and use `consteval`.  Otherwise, you'll need to add `inline` for this.

Comment at: libc/src/__support/integer_to_string.h:109
+  static_assert(
+      2 <= Base <= 36,
+      "BaseIntegerToString can only have a base between 2 and 36 inclusive.");
Can we really do this?  On the surface it looks equivalent to `int(bool(2 <= Base)) <= 36` which is always `true` since `int(bool) is either 0 or 1.

Comment at: libc/src/__support/integer_to_string.h:150
+  constexpr explicit BaseIntegerToString() { ; }
+  constexpr explicit BaseIntegerToString(T val) { convert(val, Base, true); }
why is the `;` needed?

Comment at: libc/src/__support/integer_to_string.h:154
+  constexpr explicit BaseIntegerToString(T val, size_t conv_base) {
+    convert(val, conv_base, true);
+  }
`convert(val, conv_base, /*lowercase=*/ true)` might be a bit easier to read.

  rG LLVM Github Monorepo



More information about the libc-commits mailing list