Re: [PATCH] libstdc++: Reduce integer std::to/from_chars symbol sizes

[Jonathan Wakely <jwakely@redhat.com>]

On Wed, 13 Sept 2023 at 23:52, Patrick Palka via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?

OK, thanks.

>
> -- >8 --
>
> For std::to_chars:
>
> The constrained alias __integer_to_chars_result_type seems unnecessary
> ever since r10-3080-g28f0075742ed58 got rid of the only public overload
> which used it.  Now only non-public overloads are constrained by it
> (through their return type) and these non-public overloads aren't used
> in a SFINAE context, so the constraints have no observable effect.  So
> this patch gets rid of this alias, which greatly reduces the symbol
> sizes of the affected functions (since the expanded alias is quite
> large).
>
> For std::from_chars:
>
> We can't get rid of the corresponding alias because it's constrains
> the public integer std::from_chars overload.  But we can avoid having
> the constraint bloat the mangled name by instead encoding it as a
> defaulted template parameter.  We use the non-type parameter form
>
>   enable_if_t<..., int> = 0
>
> instead of the type parameter form
>
>   typename = enable_if_t<...>
>
> because the type form can be circumvented by providing an explicit template
> argument to type parameter, e.g. 'std::from_chars<int, void>(...)', so the
> non-type form seems like the more robust choice.
>
> In passing, use __is_standard_integer in the constraint.
>
> libstdc++-v3/ChangeLog:
>
>         * include/std/charconv (__detail::__integer_to_chars_result_type):
>         Remove.
>         (__detail::__to_chars_16): Use to_chars_result as return type.
>         (__detail::__to_chars_10): Likewise.
>         (__detail::__to_chars_8): Likewise.
>         (__detail::__to_chars_2): Likewise.
>         (__detail::__to_chars_i): Likewise.
>         (__detail::__integer_from_chars_result_type): Pull out enable_if_t
>         condition into and replace with ...
>         (__detail::__integer_from_chars_enabled): ... this.  Use
>         __is_standard_integer instead of __is_signed_integer and
>         __is_unsigned_integer.
>         (from_chars): Encode constraint as a defaulted non-type template
>         parameter instead of within the return type.
> ---
>  libstdc++-v3/include/std/charconv | 33 ++++++++++---------------------
>  1 file changed, 10 insertions(+), 23 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/charconv b/libstdc++-v3/include/std/charconv
> index 01711d38576..ec25ae139ba 100644
> --- a/libstdc++-v3/include/std/charconv
> +++ b/libstdc++-v3/include/std/charconv
> @@ -79,17 +79,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>  namespace __detail
>  {
> -  template<typename _Tp>
> -    using __integer_to_chars_result_type
> -      = enable_if_t<__or_<__is_signed_integer<_Tp>,
> -                         __is_unsigned_integer<_Tp>,
> -#if defined __SIZEOF_INT128__ && defined __STRICT_ANSI__
> -                         is_same<_Tp, signed __int128>,
> -                         is_same<_Tp, unsigned __int128>,
> -#endif
> -                         is_same<char, remove_cv_t<_Tp>>>::value,
> -                   to_chars_result>;
> -
>    // Pick an unsigned type of suitable size. This is used to reduce the
>    // number of specializations of __to_chars_len, __to_chars etc. that
>    // get instantiated. For example, to_chars<char> and to_chars<short>
> @@ -162,7 +151,7 @@ namespace __detail
>      }
>
>    template<typename _Tp>
> -    constexpr __integer_to_chars_result_type<_Tp>
> +    constexpr to_chars_result
>      __to_chars_16(char* __first, char* __last, _Tp __val) noexcept
>      {
>        static_assert(__integer_to_chars_is_unsigned<_Tp>, "implementation bug");
> @@ -208,7 +197,7 @@ namespace __detail
>      }
>
>    template<typename _Tp>
> -    constexpr __integer_to_chars_result_type<_Tp>
> +    constexpr to_chars_result
>      __to_chars_10(char* __first, char* __last, _Tp __val) noexcept
>      {
>        static_assert(__integer_to_chars_is_unsigned<_Tp>, "implementation bug");
> @@ -231,7 +220,7 @@ namespace __detail
>      }
>
>    template<typename _Tp>
> -    constexpr __integer_to_chars_result_type<_Tp>
> +    constexpr to_chars_result
>      __to_chars_8(char* __first, char* __last, _Tp __val) noexcept
>      {
>        static_assert(__integer_to_chars_is_unsigned<_Tp>, "implementation bug");
> @@ -284,7 +273,7 @@ namespace __detail
>      }
>
>    template<typename _Tp>
> -    constexpr __integer_to_chars_result_type<_Tp>
> +    constexpr to_chars_result
>      __to_chars_2(char* __first, char* __last, _Tp __val) noexcept
>      {
>        static_assert(__integer_to_chars_is_unsigned<_Tp>, "implementation bug");
> @@ -320,7 +309,7 @@ namespace __detail
>  } // namespace __detail
>
>    template<typename _Tp>
> -    constexpr __detail::__integer_to_chars_result_type<_Tp>
> +    constexpr to_chars_result
>      __to_chars_i(char* __first, char* __last, _Tp __value, int __base = 10)
>      {
>        __glibcxx_assert(2 <= __base && __base <= 36);
> @@ -548,17 +537,15 @@ namespace __detail
>      }
>
>    template<typename _Tp>
> -    using __integer_from_chars_result_type
> -      = enable_if_t<__or_<__is_signed_integer<_Tp>,
> -                         __is_unsigned_integer<_Tp>,
> -                         is_same<char, remove_cv_t<_Tp>>>::value,
> -                   from_chars_result>;
> +    constexpr bool __integer_from_chars_enabled
> +      = __or_<__is_standard_integer<_Tp>, is_same<char, remove_cv_t<_Tp>>>::value;
>
>  } // namespace __detail
>
>    /// std::from_chars for integral types.
> -  template<typename _Tp>
> -    _GLIBCXX23_CONSTEXPR __detail::__integer_from_chars_result_type<_Tp>
> +  template<typename _Tp,
> +          enable_if_t<__detail::__integer_from_chars_enabled<_Tp>, int> = 0>
> +    _GLIBCXX23_CONSTEXPR from_chars_result
>      from_chars(const char* __first, const char* __last, _Tp& __value,
>                int __base = 10)
>      {
> --
> 2.42.0.158.g94e83dcf5b
>