[libcxx-commits] [PATCH] D145590: [libc++][charconv] Granularizes the header.

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__charconv/to_chars_integral.h:317-334
+template <typename _Tp, typename enable_if<is_integral<_Tp>::value, int>::type = 0>
+inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI to_chars_result
+to_chars(char* __first, char* __last, _Tp __value)
+{
+  using _Type = __make_32_64_or_128_bit_t<_Tp>;
+  static_assert(!is_same<_Type, void>::value, "unsupported integral type used in to_chars");
+  return std::__to_chars_itoa(__first, __last, static_cast<_Type>(__value), is_signed<_Tp>());
----------------
I think we should put these and the floating point overloads in a `__fwd/to_chars.h` header to avoid having a different overload set depending on what file is included. A linker error is a lot better than a program that compiler but is subtly wrong.


================
Comment at: libcxx/include/__charconv/traits.h:1
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
----------------
FWIW I'd be fine with formatting the files in this patch.


================
Comment at: libcxx/include/module.modulemap.in:688-690
+      module from_chars_integral {
+        private header "__charconv/from_chars_integral.h"
+      }
----------------
Why do you deviate from the normal style here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145590