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

[Mark de Wever via Phabricator via libcxx-commits]

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


================
Comment at: libcxx/include/charconv:489
+template <unsigned _Base, typename _Tp,
+          typename enable_if<(sizeof(_Tp) >= sizeof(unsigned)), int>::type = 0>
+_LIBCPP_AVAILABILITY_TO_CHARS _LIBCPP_INLINE_VISIBILITY int
----------------
Quuxplusone wrote:
> Mordante wrote:
> > Quuxplusone wrote:
> > > `__enable_if_t<sizeof(_Tp) >= sizeof(unsigned)>* = nullptr` is the more traditional libc++ism. (`enable_if_t` if this is C++14-and-later.)
> > This is the same as other places in this file, so I prefer to keep it.
> > Note the `enable_if` specifies the type as `int` so `nullptr` isn't an option.
> > This is the same as other places in this file, so I prefer to keep it.
> 
> OK.
> 
> > Note the enable_if specifies the type as int so nullptr isn't an option.
> 
> FWIW: The existing `enable_if` comes out to `int = 0`, but in my suggested "more traditional libc++ism" rewrite, the `__enable_if_t` would come out to `void* = nullptr` //instead//. I don't see a problem there, so either I'm missing something or you were. (The rewrite //would// change the mangling, but this is hide-from-abi so we shouldn't care.)
I prefer to keep them in this style to keep the file uniform. It indeed shouldn't be an ABI break, but I rather keep them as-is, just in case there is an issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97705