[libstdc++] use strtold for from_chars even without locale

[Patrick Palka]

On Fri, 5 May 2023, Jonathan Wakely wrote:

> 
> 
> On Fri, 5 May 2023 at 10:43, Florian Weimer wrote:
>       * Jonathan Wakely via Libstdc:
> 
>       > We could use strtod for a single-threaded target (i.e.
>       > !defined(_GLIBCXX_HAS_GTHREADS) by changing the global locale using
>       > setlocale, instead of changing the per-thread locale using uselocale.
> 
>       This is not generally safe because the call to setlocale is still
>       observable to applications in principle because a previous pointer
>       returned from setlocale they have store could be invalidated.
> 
> 
> Ah yes, good point, thanks. I think that's a non-starter then. I still think using RAII makes the from_chars_impl function easier to read, so here's a version of that patch without the single-threaded
> conditions.
> 
> commit 4dc5b8864ec527e699d35880fbc706157113f92b
> Author: Jonathan Wakely <jwakely@redhat.com>
> Date:   Thu May 4 15:22:07 2023
> 
>     libstdc++: Use RAII types in strtod-based std::from_chars implementation
>     
>     This adds auto_locale and auto_ferounding types to use RAII for changing
>     and restoring the local and floating-point environment when using strtod
>     to implement std::from_chars.
>     
>     The destructors for the RAII objects run slightly later than the
>     previous statements that restored the locale/fenv, but the differences
>     are just some trivial assignments and an isinf call.
>     
>     libstdc++-v3/ChangeLog:
>     
>             * src/c++17/floating_from_chars.cc [USE_STRTOD_FOR_FROM_CHARS]
>             (auto_locale, auto_ferounding): New class types.
>             (from_chars_impl): Use auto_locale and auto_ferounding.
> 
> diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc b/libstdc++-v3/src/c++17/floating_from_chars.cc
> index 78b9d92cdc0..7b3bdf445e3 100644
> --- a/libstdc++-v3/src/c++17/floating_from_chars.cc
> +++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
> @@ -597,6 +597,69 @@ namespace
>      return buf.c_str();
>    }
>  
> +  // RAII type to change and restore the locale.
> +  struct auto_locale
> +  {
> +#if _GLIBCXX_HAVE_USELOCALE
> +    // When we have uselocale we can change the current thread's locale.
> +    locale_t loc;
> +    locale_t orig;

It's not a big deal, but we could consider making these members const
too, like in auto_ferounding.

LGTM.  I noticed sprintf_ld from floating_to_chars.cc could benefit from
auto_ferounding as well.

> +
> +    auto_locale()
> +    : loc(::newlocale(LC_ALL_MASK, "C", (locale_t)0))
> +    {
> +      if (loc)
> +	orig = ::uselocale(loc);
> +      else
> +	ec = errc{errno};
> +    }
> +
> +    ~auto_locale()
> +    {
> +      if (loc)
> +	{
> +	  ::uselocale(orig);
> +	  ::freelocale(loc);
> +	}
> +    }
> +#else
> +    // Otherwise, we can't change the locale and so strtod can't be used.
> +    auto_locale() = delete;
> +#endif
> +
> +    explicit operator bool() const noexcept { return ec == errc{}; }
> +
> +    errc ec{};
> +
> +    auto_locale(const auto_locale&) = delete;
> +    auto_locale& operator=(const auto_locale&) = delete;
> +  };
> +
> +  // RAII type to change and restore the floating-point environment.
> +  struct auto_ferounding
> +  {
> +#if _GLIBCXX_USE_C99_FENV_TR1 && defined(FE_TONEAREST)
> +    const int rounding = std::fegetround();
> +
> +    auto_ferounding()
> +    {
> +      if (rounding != FE_TONEAREST)
> +	std::fesetround(FE_TONEAREST);
> +    }
> +
> +    ~auto_ferounding()
> +    {
> +      if (rounding != FE_TONEAREST)
> +	std::fesetround(rounding);
> +    }
> +#else
> +    auto_ferounding() = default;
> +#endif
> +
> +    auto_ferounding(const auto_ferounding&) = delete;
> +    auto_ferounding& operator=(const auto_ferounding&) = delete;
> +  };
> +
>    // Convert the NTBS `str` to a floating-point value of type `T`.
>    // If `str` cannot be converted, `value` is unchanged and `0` is returned.
>    // Otherwise, let N be the number of characters consumed from `str`.
> @@ -607,16 +670,11 @@ namespace
>    ptrdiff_t
>    from_chars_impl(const char* str, T& value, errc& ec) noexcept
>    {
> -    if (locale_t loc = ::newlocale(LC_ALL_MASK, "C", (locale_t)0)) [[likely]]
> +    auto_locale loc;
> +
> +    if (loc)
>        {
> -	locale_t orig = ::uselocale(loc);
> -
> -#if _GLIBCXX_USE_C99_FENV_TR1 && defined(FE_TONEAREST)
> -	const int rounding = std::fegetround();
> -	if (rounding != FE_TONEAREST)
> -	  std::fesetround(FE_TONEAREST);
> -#endif
> -
> +	auto_ferounding rounding;
>  	const int save_errno = errno;
>  	errno = 0;
>  	char* endptr;
> @@ -647,14 +705,6 @@ namespace
>  #endif
>  	const int conv_errno = std::__exchange(errno, save_errno);
>  
> -#if _GLIBCXX_USE_C99_FENV_TR1 && defined(FE_TONEAREST)
> -	if (rounding != FE_TONEAREST)
> -	  std::fesetround(rounding);
> -#endif
> -
> -	::uselocale(orig);
> -	::freelocale(loc);
> -
>  	const ptrdiff_t n = endptr - str;
>  	if (conv_errno == ERANGE) [[unlikely]]
>  	  {
> @@ -675,8 +725,8 @@ namespace
>  	  }
>  	return n;
>        }
> -    else if (errno == ENOMEM)
> -      ec = errc::not_enough_memory;
> +    else
> +      ec = loc.ec;
>  
>      return 0;
>    }