From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 571AD389EC64 for ; Mon, 14 Mar 2022 14:15:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 571AD389EC64 Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-582-xxPYBWxHP0qdCNHbX24jmA-1; Mon, 14 Mar 2022 10:15:53 -0400 X-MC-Unique: xxPYBWxHP0qdCNHbX24jmA-1 Received: by mail-qt1-f198.google.com with SMTP id 6-20020ac85746000000b002e1a6575540so10272214qtx.6 for ; Mon, 14 Mar 2022 07:15:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:mime-version; bh=A7v5uyFNaBB2g28DCn3C6PtLevXAV0awo9jUTc4vs2s=; b=dzP5VIBCK3hmJOEjCqJ+yO1mN5VORzf6g6TBDi2UYxIS+H5swO0wZYDv2C4ldH00cJ 7F8Tw2gxt+EoJ7eGAlMkeZcxq8tFxmEPspT894PZblA8bvP7+SxaMBjYWeQMY7PXa5yR 6mTABn9oGVfa9EKhf7Bzj5wArx1gzHFWW33fR6PxkoUygRyZQXSp62fMBhUP3lv1agDF XeUGy3lDH5zfscXCjCeSUbn1lK3chfxhQUmQ6XqbPjCS9ucG/TUUjT5HUvHbK09gSZab +3hL/40IlSkSyYFP1dNN0NVWgl8D3M/0RVcokcfwG/8on9KmlOG8WPSqtxOXFg5Pb89+ oIAw== X-Gm-Message-State: AOAM530XvItlEyKvtDZLL5y+Y9BNiJ7zYV49y5XTWrFqNWc3zQGjLxSx zigZQP13LOnKxr9fYWyWrJSEph13dtl3jk96bWzxDhfFVOdQHEBQFshSuT0/ioZvVkRkiw1hHff YA2VyLaL28YDfak4= X-Received: by 2002:a05:620a:2449:b0:67d:16cc:8505 with SMTP id h9-20020a05620a244900b0067d16cc8505mr15510438qkn.203.1647267353223; Mon, 14 Mar 2022 07:15:53 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzW4vRZM5rH6IoMczIRYbEk4VAlKPkRUk9FZiah/8ylz0ZkQ57zSamqJEQX37HfM0+Yp2vk3g== X-Received: by 2002:a05:620a:2449:b0:67d:16cc:8505 with SMTP id h9-20020a05620a244900b0067d16cc8505mr15510400qkn.203.1647267352829; Mon, 14 Mar 2022 07:15:52 -0700 (PDT) Received: from [192.168.1.130] (ool-18e40894.dyn.optonline.net. [24.228.8.148]) by smtp.gmail.com with ESMTPSA id g1-20020ae9e101000000b0067d4bfffc57sm6708888qkm.117.2022.03.14.07.15.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Mar 2022 07:15:52 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Mon, 14 Mar 2022 10:15:51 -0400 (EDT) To: Jonathan Wakely cc: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org, ppalka@redhat.com Subject: Re: [PATCH] libstdc++: Ensure that std::from_chars is declared when supported In-Reply-To: <20220311183244.1861699-1-jwakely@redhat.com> Message-ID: References: <20220311183244.1861699-1-jwakely@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-14.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Mar 2022 14:15:58 -0000 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 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 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 // for std::errc > #include > > -#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 > + 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 > >