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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked an inline comment as done.
Mordante 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>());
----------------
philnik wrote:
> Mordante wrote:
> > philnik wrote:
> > > 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.
> > I did this on purpose, when I use the floating point formatter, I can include that light header. I don't see a huge issue with this approach. This is only used in the library internally.
> Adding two forward declarations doesn't make this much heavier. My concern is that someone might rely on this being included transitively, and they get unexpected behaviour.
I tried it, but then Clang (rightfully) complains about duplicated default arguments. It also pulls in some headers for the floating-point case. So for now I keep it as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145590