Re: [PATCH] libstdc++: Ensure that std::from_chars is declared when supported

[Patrick Palka <ppalka@redhat.com>]

On Fri, 11 Mar 2022, Jonathan Wakely wrote:

> Patrick, I think this is right, but please take a look to double check.
> 
> I think we should fix the feature-test macro conditions for gcc-11 too,
> although it's a bit more complicated there. It should depend on IEEE
> float and double *and* uselocale. We don't need the other changes on the
> branch.

Don't we still depend on uselocale in GCC 12 for long double from_chars,
at least on targets where long double != binary64?

> 
> 
> -- >8 --
> 
> This adjusts the declarations in <charconv> to match when the definition
> is present. This solves the issue that std::from_chars is present on
> Solaris 11.3 (using fast_float) but was not declared in the header
> (because the declarations were guarded by _GLIBCXX_HAVE_USELOCALE).
> 
> Additionally, do not define __cpp_lib_to_chars unless both from_chars
> and to_chars are supported (which is only true for IEEE float and
> double). We might still provide from_chars (via strtold) but if to_chars
> isn't provided, we shouldn't define the feature test macro.
> 
> Finally, this simplifies some of the preprocessor checks in the bodies
> of std::from_chars in src/c++17/floating_from_chars.cc and hoists the
> repeated code for the strtod version into a new function template.
> 
> libstdc++-v3/ChangeLog:
> 
> 	* include/std/charconv (__cpp_lib_to_chars): Only define when
> 	both from_chars and to_chars are supported for floating-point
> 	types.
> 	(from_chars, to_chars): Adjust preprocessor conditions guarding
> 	declarations.
> 	* include/std/version (__cpp_lib_to_chars): Adjust condition to
> 	match <charconv> definition.
> 	* src/c++17/floating_from_chars.cc (from_chars_strtod): New
> 	function template.
> 	(from_chars): Simplify preprocessor checks and use
> 	from_chars_strtod when appropriate.
> ---
>  libstdc++-v3/include/std/charconv             |   8 +-
>  libstdc++-v3/include/std/version              |   3 +-
>  libstdc++-v3/src/c++17/floating_from_chars.cc | 120 ++++++------------
>  3 files changed, 45 insertions(+), 86 deletions(-)
> 
> diff --git a/libstdc++-v3/include/std/charconv b/libstdc++-v3/include/std/charconv
> index a3f8c7718b2..2ce9c7d4cb9 100644
> --- a/libstdc++-v3/include/std/charconv
> +++ b/libstdc++-v3/include/std/charconv
> @@ -43,7 +43,8 @@
>  #include <bits/error_constants.h> // for std::errc
>  #include <ext/numeric_traits.h>
>  
> -#if _GLIBCXX_HAVE_USELOCALE
> +#if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64 \
> +    && __SIZE_WIDTH__ >= 32

So ISTM we need to also check

  _GLIBCXX_HAVE_USELOCALE || (__LDBL_MANT_DIG__ == __DBL_MANT_DIG__)

or something like that :/

>  # define __cpp_lib_to_chars 201611L
>  #endif
>  
> @@ -686,7 +687,7 @@ namespace __detail
>    operator^=(chars_format& __lhs, chars_format __rhs) noexcept
>    { return __lhs = __lhs ^ __rhs; }
>  
> -#if _GLIBCXX_HAVE_USELOCALE
> +#if defined __cpp_lib_to_chars || _GLIBCXX_HAVE_USELOCALE
>    from_chars_result
>    from_chars(const char* __first, const char* __last, float& __value,
>  	     chars_format __fmt = chars_format::general) noexcept;
> @@ -700,8 +701,7 @@ namespace __detail
>  	     chars_format __fmt = chars_format::general) noexcept;
>  #endif
>  
> -#if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64 \
> -    && __SIZE_WIDTH__ >= 32
> +#if defined __cpp_lib_to_chars
>    // Floating-point std::to_chars
>  
>    // Overloads for float.
> diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version
> index 461e65b5fab..d730a7ea3c7 100644
> --- a/libstdc++-v3/include/std/version
> +++ b/libstdc++-v3/include/std/version
> @@ -171,7 +171,8 @@
>  #endif
>  #define __cpp_lib_shared_ptr_weak_type 201606L
>  #define __cpp_lib_string_view 201803L
> -#if _GLIBCXX_HAVE_USELOCALE
> +#if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64 \
> +    && __SIZE_WIDTH__ >= 32
>  # define __cpp_lib_to_chars 201611L
>  #endif
>  #define __cpp_lib_unordered_map_try_emplace 201411L
> diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc b/libstdc++-v3/src/c++17/floating_from_chars.cc
> index ba0426b3344..4aa2483bc28 100644
> --- a/libstdc++-v3/src/c++17/floating_from_chars.cc
> +++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
> @@ -65,6 +65,7 @@ extern "C" __ieee128 __strtoieee128(const char*, char**);
>      && __SIZE_WIDTH__ >= 32
>  # define USE_LIB_FAST_FLOAT 1
>  # if __LDBL_MANT_DIG__ == __DBL_MANT_DIG__
> +// No need to use strtold.
>  #  undef USE_STRTOD_FOR_FROM_CHARS
>  # endif
>  #endif
> @@ -420,6 +421,33 @@ namespace
>      return true;
>    }
>  #endif
> +
> +  template<typename T>
> +  from_chars_result
> +  from_chars_strtod(const char* first, const char* last, T& value,
> +		    chars_format fmt) noexcept
> +  {
> +    errc ec = errc::invalid_argument;
> +#if _GLIBCXX_USE_CXX11_ABI
> +    buffer_resource mr;
> +    pmr::string buf(&mr);
> +#else
> +    string buf;
> +    if (!reserve_string(buf))
> +      return make_result(first, 0, {}, ec);
> +#endif
> +    size_t len = 0;
> +    __try
> +      {
> +	if (const char* pat = pattern(first, last, fmt, buf)) [[likely]]
> +	  len = from_chars_impl(pat, value, ec);
> +      }
> +    __catch (const std::bad_alloc&)
> +      {
> +	fmt = chars_format{};
> +      }
> +    return make_result(first, len, fmt, ec);
> +  }
>  #endif // USE_STRTOD_FOR_FROM_CHARS
>  
>  #if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64
> @@ -793,35 +821,15 @@ from_chars_result
>  from_chars(const char* first, const char* last, float& value,
>  	   chars_format fmt) noexcept
>  {
> -#if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64
> +#if USE_LIB_FAST_FLOAT

>    if (fmt == chars_format::hex)
>      return __floating_from_chars_hex(first, last, value);
>    else
>      {
> -      static_assert(USE_LIB_FAST_FLOAT);
>        return fast_float::from_chars(first, last, value, fmt);
>      }
>  #else
> -  errc ec = errc::invalid_argument;
> -#if _GLIBCXX_USE_CXX11_ABI
> -  buffer_resource mr;
> -  pmr::string buf(&mr);
> -#else
> -  string buf;
> -  if (!reserve_string(buf))
> -    return make_result(first, 0, {}, ec);
> -#endif
> -  size_t len = 0;
> -  __try
> -    {
> -      if (const char* pat = pattern(first, last, fmt, buf)) [[likely]]
> -	len = from_chars_impl(pat, value, ec);
> -    }
> -  __catch (const std::bad_alloc&)
> -    {
> -      fmt = chars_format{};
> -    }
> -  return make_result(first, len, fmt, ec);
> +  return from_chars_strtod(first, last, value, fmt);

I think __floating_from_chars_hex should work fine on 16 bit targets,
so I suppose we could use it in the !USE_LIB_FAST_FLOAT branch as well.

>  #endif
>  }
>  
> @@ -829,35 +837,15 @@ from_chars_result
>  from_chars(const char* first, const char* last, double& value,
>  	   chars_format fmt) noexcept
>  {
> -#if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64
> +#if USE_LIB_FAST_FLOAT
>    if (fmt == chars_format::hex)
>      return __floating_from_chars_hex(first, last, value);
>    else
>      {
> -      static_assert(USE_LIB_FAST_FLOAT);
>        return fast_float::from_chars(first, last, value, fmt);
>      }
>  #else
> -  errc ec = errc::invalid_argument;
> -#if _GLIBCXX_USE_CXX11_ABI
> -  buffer_resource mr;
> -  pmr::string buf(&mr);
> -#else
> -  string buf;
> -  if (!reserve_string(buf))
> -    return make_result(first, 0, {}, ec);
> -#endif
> -  size_t len = 0;
> -  __try
> -    {
> -      if (const char* pat = pattern(first, last, fmt, buf)) [[likely]]
> -	len = from_chars_impl(pat, value, ec);
> -    }
> -  __catch (const std::bad_alloc&)
> -    {
> -      fmt = chars_format{};
> -    }
> -  return make_result(first, len, fmt, ec);
> +  return from_chars_strtod(first, last, value, fmt);
>  #endif
>  }
>  
> @@ -865,41 +853,23 @@ from_chars_result
>  from_chars(const char* first, const char* last, long double& value,
>  	   chars_format fmt) noexcept
>  {
> -#if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64 \
> -  && ! USE_STRTOD_FOR_FROM_CHARS
> +#if ! USE_STRTOD_FOR_FROM_CHARS
> +  // Either long double is the same as double, or we can't use strtold.
> +  // In the latter case, this might give an incorrect result (e.g. values
> +  // out of range of double give an error, even if they fit in long double).
>    double dbl_value;
>    from_chars_result result;
>    if (fmt == chars_format::hex)
>      result = __floating_from_chars_hex(first, last, dbl_value);
>    else
>      {
> -      static_assert(USE_LIB_FAST_FLOAT);
>        result = fast_float::from_chars(first, last, dbl_value, fmt);
>      }
>    if (result.ec == errc{})
>      value = dbl_value;
>    return result;
>  #else
> -  errc ec = errc::invalid_argument;
> -#if _GLIBCXX_USE_CXX11_ABI
> -  buffer_resource mr;
> -  pmr::string buf(&mr);
> -#else
> -  string buf;
> -  if (!reserve_string(buf))
> -    return make_result(first, 0, {}, ec);
> -#endif
> -  size_t len = 0;
> -  __try
> -    {
> -      if (const char* pat = pattern(first, last, fmt, buf)) [[likely]]
> -	len = from_chars_impl(pat, value, ec);
> -    }
> -  __catch (const std::bad_alloc&)
> -    {
> -      fmt = chars_format{};
> -    }
> -  return make_result(first, len, fmt, ec);
> +  return from_chars_strtod(first, last, value, fmt);
>  #endif
>  }
>  
> @@ -918,20 +888,8 @@ from_chars_result
>  from_chars(const char* first, const char* last, __ieee128& value,
>  	   chars_format fmt) noexcept
>  {
> -  buffer_resource mr;
> -  pmr::string buf(&mr);
> -  size_t len = 0;
> -  errc ec = errc::invalid_argument;
> -  __try
> -    {
> -      if (const char* pat = pattern(first, last, fmt, buf)) [[likely]]
> -	len = from_chars_impl(pat, value, ec);
> -    }
> -  __catch (const std::bad_alloc&)
> -    {
> -      fmt = chars_format{};
> -    }
> -  return make_result(first, len, fmt, ec);
> +  // fast_float doesn't support IEEE binary128 format, but we can use strtold.
> +  return from_chars_strtod(first, last, value, fmt);
>  }
>  #endif
>  
> -- 
> 2.34.1
> 
>