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

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/__support/integer_to_string.h:82
 
+constexpr inline size_t floor_log_2(size_t num) {
+  size_t i = 0;
----------------
AFAICT, this is a private function so can be a static private member of the converter class.


================
Comment at: libc/src/__support/integer_to_string.h:166
+    convert(val, conv_base, lowercase);
+  }
+
----------------
Based on the use case you have in D131302, a better design seems to be to use a template function and not a template class?

```
class IntegerToString {
public:
  template <typename T, uint8_t BASE>
  constexpr size_t bufsize() {
    return ...; // The BUFSIZE formula
  }

  template <typename T, uint8_t BASE,  cpp::enable_if_t<2 <= BASE && BASE <= 10, int> = 0>
  static constexpr cpp:optional<cpp::StringView> convert(T val, cpp::MutableArrayRef<char> &buffer) {
    // If This function can actually be a constexpr, then the below "if" will be optimized out.
    if (buffer.size() < bufsize<T, BASE>())
      return cpp::optional<cpp::StringView>();

    // Perform conversion and write it to buffer

    return cpp::StringView(...); // A StringView into the |buffer|
  }

  template <typename T, uint8_t BASE, cpp::enable_if_t<10 < BASE && BASE <= 36, int> = 0>
  static constexpr cpp:optional<cpp::StringView> convert(T val, cpp::MutableArrayRef<char> &buffer, bool lowercase) {
    ...
  }

};


```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131301