[libcxx-commits] [PATCH] D128215: [libc++] Reduces std::to_chars instantiations.

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

Is there a reason to not just instantiate it for that largest integral type? Or `size_t` and the largest integral type to avoid using multiple registers for smaller types?



================
Comment at: libcxx/include/charconv:492
+  static_assert(!is_same<__type, void>::value || sizeof(_Tp) > sizeof(int64_t), "unsupported integral type used in to_chars");
+  return __to_chars_itoa(__first, __last, static_cast<__type>(__value), is_signed<_Tp>());
 }
----------------



================
Comment at: libcxx/include/charconv:501
+
+  using __type = __make_32_64_or_128_bit_t<_Tp>;
+  static_assert(!is_same<__type, void>::value || sizeof(_Tp) > sizeof(int64_t), "unsupported integral type used in to_chars");
----------------
Why _Uglify it? You can just use `type`. Otherwise I would use `_Type` to make it clear that it's a type.


================
Comment at: libcxx/include/charconv:503
+  static_assert(!is_same<__type, void>::value || sizeof(_Tp) > sizeof(int64_t), "unsupported integral type used in to_chars");
+  return __to_chars_integral(__first, __last, static_cast<__type>(__value), __base, is_signed<_Tp>());
 }
----------------



================
Comment at: libcxx/include/type_traits:1024-1045
+template <class _Tp>
+using __make_32_64_or_128_bit_t = typename conditional<
+    is_signed<_Tp>::value,
+    typename conditional<
+        sizeof(_Tp) <= sizeof(int32_t), int32_t,
+        typename conditional<sizeof(_Tp) <= sizeof(int64_t), int64_t,
+#ifndef _LIBCPP_HAS_NO_INT128
----------------
Could you put this into it's own header?


================
Comment at: libcxx/include/type_traits:1025
+template <class _Tp>
+using __make_32_64_or_128_bit_t = typename conditional<
+    is_signed<_Tp>::value,
----------------
Mordante wrote:
> Note I'm not fond of this name, so I'm open to suggestions for a better name.
Maybe `__extend_to_32_64_or_128_bit_t`? It's also not exactly perfect, but I think it describes it a bit better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128215