[libcxx-commits] [PATCH] D97705: [libc++] Improve std::to_chars for base != 10.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked 2 inline comments as done.
Mordante added inline comments.


================
Comment at: libcxx/include/charconv:508
+          typename enable_if<(sizeof(_Tp) < sizeof(unsigned)), int>::type = 0>
+_LIBCPP_AVAILABILITY_TO_CHARS _LIBCPP_HIDE_FROM_ABI to_chars_result
+__to_chars_integral(char* __first, char* __last, _Tp __value) {
----------------
ldionne wrote:
> I don't think we need `_LIBCPP_AVAILABILITY_TO_CHARS` here, since this is all implemented in the headers. Or did I miss a dependency on the dylib?
Good catch!


================
Comment at: libcxx/include/charconv:546
 {
-  if (__base == 10)
+  if (__base == 10) [[likely]]
     return __to_chars_itoa(__first, __last, __value, false_type());
----------------
philnik wrote:
> Does this `[[likely]]` make any difference?
Yes I've measured in the benchmarks that it makes improvements. However I plan to refactor this file to prepare for https://wg21.link/p2291r3 "Add Constexpr Modifiers to Functions to_chars and from_chars for Integral Types in <charconv>" and 128-bit support. When the base 10 implementation moves to the header this attribute may have less benefit. But that requires another round of profiling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97705