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

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

[Jonathan Wakely]

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.


-- >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
 # 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);
 #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

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

[Patrick Palka]

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
> 
> 

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

[Jonathan Wakely]

On Mon, 14 Mar 2022 at 14:17, Patrick Palka via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> 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?

Not after this patch:

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).

If uselocale isn't available, this defines the long double overload in
terms of the double one, even if that doesn't always give the right
answers. That greatly simplifies the preprocessor conditions for when
it's supported. If the float and double forms are present, so is the
long double one.

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

[Patrick Palka]

On Mon, 14 Mar 2022, Jonathan Wakely wrote:

> On Mon, 14 Mar 2022 at 14:17, Patrick Palka via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > 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?
> 
> Not after this patch:
> 
> 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).
> 
> If uselocale isn't available, this defines the long double overload in
> terms of the double one, even if that doesn't always give the right
> answers. That greatly simplifies the preprocessor conditions for when
> it's supported. If the float and double forms are present, so is the
> long double one.

Ah sorry, I had overlooked that part of the patch.  Makes sense and LGTM!

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

[Jonathan Wakely]

On Tue, 15 Mar 2022 at 14:12, Patrick Palka wrote:
>
> On Mon, 14 Mar 2022, Jonathan Wakely wrote:
>
> > On Mon, 14 Mar 2022 at 14:17, Patrick Palka via Libstdc++
> > <libstdc++@gcc.gnu.org> wrote:
> > >
> > > 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?
> >
> > Not after this patch:
> >
> > 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).
> >
> > If uselocale isn't available, this defines the long double overload in
> > terms of the double one, even if that doesn't always give the right
> > answers. That greatly simplifies the preprocessor conditions for when
> > it's supported. If the float and double forms are present, so is the
> > long double one.
>
> Ah sorry, I had overlooked that part of the patch.  Makes sense and LGTM!

Pushed to trunk now.

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

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

On Mon, 14 Mar 2022 at 14:15, Patrick Palka wrote:
> 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.

Good point, and even for SIZE_WIDTH >= 32, it might be faster to use
your __floating_from_chars_hex function than to change locale twice
and allocate memory and call strto{f,d,ld}. It certainly avoids the
non-standard errc::out_of_memory result.

So maybe something like the attached semi-tested patch.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3408 bytes --]

commit 57b9e2227c7b14f04f2593ec41e177631b6f3fc2
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Mar 16 16:46:07 2022

    libstdc++: Prefer our own __floating_from_chars_hex if supported
    
    This adjust the floating-point overloads of std::from_chars so that they
    prefer to use __floating_from_chars_hex if possible, instead of using
    strtof/strtod/strtold. This avoids changing the per-thread C locale and
    allocating memory for a null-terminated string when we can use the
    hex-specific function instead.
    
    libstdc++-v3/ChangeLog:
    
            * src/c++17/floating_from_chars.cc (from_chars): Always use
            __floating_from_chars_hex for hex format if possible.

diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc b/libstdc++-v3/src/c++17/floating_from_chars.cc
index 4aa2483bc28..ce30752ee95 100644
--- a/libstdc++-v3/src/c++17/floating_from_chars.cc
+++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
@@ -821,13 +821,12 @@ from_chars_result
 from_chars(const char* first, const char* last, float& value,
 	   chars_format fmt) noexcept
 {
-#if USE_LIB_FAST_FLOAT
+#if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64
   if (fmt == chars_format::hex)
     return __floating_from_chars_hex(first, last, value);
-  else
-    {
-      return fast_float::from_chars(first, last, value, fmt);
-    }
+#endif
+#if USE_LIB_FAST_FLOAT
+  return fast_float::from_chars(first, last, value, fmt);
 #else
   return from_chars_strtod(first, last, value, fmt);
 #endif
@@ -837,13 +836,12 @@ from_chars_result
 from_chars(const char* first, const char* last, double& value,
 	   chars_format fmt) noexcept
 {
-#if USE_LIB_FAST_FLOAT
+#if _GLIBCXX_FLOAT_IS_IEEE_BINARY32 && _GLIBCXX_DOUBLE_IS_IEEE_BINARY64
   if (fmt == chars_format::hex)
     return __floating_from_chars_hex(first, last, value);
-  else
-    {
-      return fast_float::from_chars(first, last, value, fmt);
-    }
+#endif
+#if USE_LIB_FAST_FLOAT
+  return fast_float::from_chars(first, last, value, fmt);
 #else
   return from_chars_strtod(first, last, value, fmt);
 #endif
@@ -853,18 +851,12 @@ from_chars_result
 from_chars(const char* first, const char* last, long double& value,
 	   chars_format fmt) noexcept
 {
-#if ! USE_STRTOD_FOR_FROM_CHARS
+#if ! USE_STRTOD_FOR_FROM_CHARS || __LDBL_MANT_DIG__ == __DBL_MANT_DIG__
   // 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
-    {
-      result = fast_float::from_chars(first, last, dbl_value, fmt);
-    }
+  from_chars_result result = std::from_chars(first, last, dbl_value, fmt);
   if (result.ec == errc{})
     value = dbl_value;
   return result;
@@ -888,7 +880,8 @@ from_chars_result
 from_chars(const char* first, const char* last, __ieee128& value,
 	   chars_format fmt) noexcept
 {
-  // fast_float doesn't support IEEE binary128 format, but we can use strtold.
+  // fast_float doesn't support IEEE binary128 format, and neither does our
+  // __floating_from_chars_hex, but we can use __strtoieee128 on this target.
   return from_chars_strtod(first, last, value, fmt);
 }
 #endif