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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
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
----------------
Mordante wrote:
> ldionne wrote:
> > Mordante wrote:
> > > philnik wrote:
> > > > Could you put this into it's own header?
> > > Yes I think that makes sense, but I first want to settle on a good name.
> > I would suggest this instead (the reformatting would make it easier to read IMO):
> > 
> > ```
> > template <bool _Cond, class _If, class _Else>
> > using __conditional_t = typename conditional<_Cond, _If, _Else>::type;
> > 
> > template <class _Tp>
> > using __make_32_64_or_128_bit_t = __copy_unsigned_t<_Tp,
> >   __conditional_t<sizeof(_Tp) <= sizeof(int32_t),   int32_t,
> >   __conditional_t<sizeof(_Tp) <= sizeof(int64_t),   int64_t,
> >   __conditional_t<sizeof(_Tp) <= sizeof(__int128_t), __int128_t,
> >   /* else */                                         void
> >   >>>
> > >;
> > ```
> > 
> > Here, `__copy_unsigned_t ` would be basically
> > 
> > ```
> > template <class _Tp, class _Up>
> > using __copy_unsigned_t = typename conditional<is_unsigned<_Tp>::value, make_unsigned_t<_Up>, _Up>;
> > ```
> After the rewrite to the suggestion above that's no longer an option; it uses `make_unsigned`. Do you have patches in progress to move that? If not I can make a followup patch to move `make_unsigned` and this new helper.
I don't have  a patch for that currently. You can just leave the new trait here and I'll move it once I get around to granularizing `<type_traits>` further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128215