Re: [PATCH] libstdc++: Format std::float16_t and std::bfloat16_t using respective std::to_chars overloads [PR126731]
Tomasz Kaminski <[email protected]>
| Newsgroups | gmane.comp.gcc.patches,gmane.comp.gcc.libstdc++.devel |
|---|---|
| Message-ID | <CAKvuMXCnVbvFNZYBY_b9j+R8LK0hjwWWk7d+dBT-tL9TKLp_Gw@mail.gmail.com> |
On Thu, Aug 20, 2026 at 3:51 PM Tomasz Kaminski <[email protected]> wrote: > > > On Thu, Aug 20, 2026 at 2:36 PM Jonathan Wakely <[email protected]> > wrote: > >> On Thu, 13 Aug 2026 at 10:36 +0200, Tomasz Kamiński wrote: >> >This patch removes the partial specialization for float16_t and >> bfloat16_t >> >that formatted the values by casting to float. In consequence an implicit >> >specialization of generic floating-point specialization is used, and >> >to_chars overloads for their types are used. >> >As explained in r13-3591-g0ae26533b3e268 (that adds corresponding >> to_chars >> >overload), the shortest string (produced when no precision is specified) >> >is shorter for above, than float. >> > >> >As formatter specializations were defined under the same condition as >> >corresponding to_chars overloads, the __formattable_float constrain on >> >formatter floating-point specialization makes it enable in same cases. >> > >> >This reverts parts of r14-3305-g6cf214b4fc97f5 (revereted partially >> >> "reverted" >> >> >in r14-3329-g27d0cfcb2b33de), leading to (necessary for correctness) >> >increase of the number template specializations. It also depends on >> >preserving values of above types (instead of float) in basic_format_arg >> >introduced r16-616-g9c9a7316adb996. >> >> Ah, so I added these specializations in r14-3305-g6cf214b4fc97f5 to >> support printing extended floating-point types in C++20, but that >> turned out to not work. So I adjusted the conditions for enabling them >> in r14-3329-g27d0cfcb2b33de, so that they are only enabled for C++23. >> But really we should have just removed the specializations again, >> because the __formattable_float specialization formats them properly >> since r16-616-g9c9a7316adb996. >> >> >The specializations for float32_t and float64_t are left unchanged, >> >as their to_chars overloads are also implemented in terms of casting >> >to float/double respectivelly. >> > >> >libstdc++-v3/ChangeLog: >> > >> > PR libstdc++/126731 >> > * include/std/format (std::formatter<_Float16, _CharT>) >> > (std::formatter<__format::__bflt16_t, _CharT>): Remove >> > partial specializations. >> > * testsuite/std/format/formatter/ext_float.cc: Test output >> > for closest value to 1/10. >> >--- >> >As far as I can tell this case will not be affected by P3505R4 >> >"Fix the default floating-point representation in std::format", >> >as float(0.1f16) is 1.998p-4, while 0.1 f is 1.99999ap-4, so >> >only later could use 0.1 as shortest string. >> > >> >Testing on x86_64-linux. *format* passed in all standard modes, >> >-m32, debug and assertions. OK for trunk and 16? >> >(We cannot backport it to eariel standards due dependency described >> >above). >> >> Yes OK for trunk and gcc-16. >> >> If I understand correctly, I think we could remove them in gcc-14 and >> gcc-15, because they're redundant (the __formattable_float >> specialization can format them). But we would still get incorrect >> results, because basic_format_arg would convert them to float. > > We will get inconsistent results, depending on how you format them: > * if (b)float16 is passed as argument to format call, then ye we will > convert > them to float, and get correct result > I meant incorrect output in this case. > * if you call formatter<(b)float16> direclty (for example becaue you are > writting > formatter for a class with a member), you will get correct output > >> So the extra specializations in gcc-14 and gcc-15 are not doing any harm, >> and >> it doesn't seem worth backporting it. >> > They are keeping us consistently wrong; if we remove them, we will be > inconsistent. > and wrong in the usual case. > >> >> >> > libstdc++-v3/include/std/format | 44 ------------------- >> > .../std/format/formatter/ext_float.cc | 22 ++++++---- >> > 2 files changed, 13 insertions(+), 53 deletions(-) >> > >> >diff --git a/libstdc++-v3/include/std/format >> b/libstdc++-v3/include/std/format >> >index 729cb89ec60..8d530e883c3 100644 >> >--- a/libstdc++-v3/include/std/format >> >+++ b/libstdc++-v3/include/std/format >> >@@ -3003,28 +3003,6 @@ namespace __format >> > }; >> > #endif >> > >> >-#if defined(__STDCPP_FLOAT16_T__) && >> defined(_GLIBCXX_FLOAT_IS_IEEE_BINARY32) >> >- // Reuse __formatter_fp<C>::format<float, Out> for _Float16. >> >- template<__format::__char _CharT> >> >- struct formatter<_Float16, _CharT> >> >- { >> >- formatter() = default; >> >- >> >- [[__gnu__::__always_inline__]] >> >- constexpr typename basic_format_parse_context<_CharT>::iterator >> >- parse(basic_format_parse_context<_CharT>& __pc) >> >- { return _M_f.parse(__pc); } >> >- >> >- template<typename _Out> >> >- typename basic_format_context<_Out, _CharT>::iterator >> >- format(_Float16 __u, basic_format_context<_Out, _CharT>& __fc) >> const >> >- { return _M_f.format((float)__u, __fc); } >> >- >> >- private: >> >- __format::__formatter_fp<_CharT> _M_f; >> >- }; >> >-#endif >> >- >> > #if defined(__FLT32_DIG__) && defined(_GLIBCXX_FLOAT_IS_IEEE_BINARY32) >> > // Reuse __formatter_fp<C>::format<float, Out> for _Float32. >> > template<__format::__char _CharT> >> >@@ -3114,28 +3092,6 @@ namespace __format >> > }; >> > #endif >> > >> >-#if defined(__STDCPP_BFLOAT16_T__) && >> defined(_GLIBCXX_FLOAT_IS_IEEE_BINARY32) >> >- // Reuse __formatter_fp<C>::format<float, Out> for bfloat16_t. >> >- template<__format::__char _CharT> >> >- struct formatter<__format::__bflt16_t, _CharT> >> >- { >> >- formatter() = default; >> >- >> >- [[__gnu__::__always_inline__]] >> >- constexpr typename basic_format_parse_context<_CharT>::iterator >> >- parse(basic_format_parse_context<_CharT>& __pc) >> >- { return _M_f.parse(__pc); } >> >- >> >- template<typename _Out> >> >- typename basic_format_context<_Out, _CharT>::iterator >> >- format(__gnu_cxx::__bfloat16_t __u, >> >- basic_format_context<_Out, _CharT>& __fc) const >> >- { return _M_f.format((float)__u, __fc); } >> >- >> >- private: >> >- __format::__formatter_fp<_CharT> _M_f; >> >- }; >> >-#endif >> > #endif // __cpp_lib_to_chars >> > >> > /** Format a pointer. >> >diff --git a/libstdc++-v3/testsuite/std/format/formatter/ext_float.cc >> b/libstdc++-v3/testsuite/std/format/formatter/ext_float.cc >> >index 82e696fa797..7b231b07006 100644 >> >--- a/libstdc++-v3/testsuite/std/format/formatter/ext_float.cc >> >+++ b/libstdc++-v3/testsuite/std/format/formatter/ext_float.cc >> >@@ -4,10 +4,14 @@ >> > #include <testsuite_hooks.h> >> > >> > template<typename T> >> >-bool format_float() >> >+void >> >+verify_output() >> > { >> >- auto s = std::format("{:#} != {:<+7.3f}", (T)-0.0, (T)0.5); >> >- return s == "-0. != +0.500 "; >> >+ auto s = std::format("{:#} != {:<+7.3f}", T(-0.0), T(0.5)); >> >+ VERIFY( s == "-0. != +0.500 "); >> >+ >> >+ s = std::format("{}", T(1)/T(10)); >> >+ VERIFY( s == "0.1"); >> > } >> > >> > #if __cplusplus > 202002L >> >@@ -23,7 +27,7 @@ test_float16() >> > { >> > #if __FLT16_DIG__ >> > if constexpr (formattable<_Float16>) >> >- VERIFY( format_float<_Float16>() ); >> >+ verify_output<_Float16>(); >> > else >> > std::puts("Cannot format _Float16 on this target"); >> > #endif >> >@@ -34,7 +38,7 @@ test_float32() >> > { >> > #if __FLT32_DIG__ >> > if constexpr (formattable<_Float32>) >> >- VERIFY( format_float<_Float32>() ); >> >+ verify_output<_Float32>(); >> > else >> > std::puts("Cannot format _Float32 on this target"); >> > #endif >> >@@ -45,7 +49,7 @@ test_float64() >> > { >> > #if __FLT64_DIG__ >> > if constexpr (formattable<_Float64>) >> >- VERIFY( format_float<_Float64>() ); >> >+ verify_output<_Float64>(); >> > else >> > std::puts("Cannot format _Float64 on this target"); >> > #endif >> >@@ -56,13 +60,13 @@ test_float128() >> > { >> > #ifdef __SIZEOF_FLOAT128__ >> > if constexpr (formattable<__float128>) >> >- VERIFY( format_float<__float128>() ); >> >+ verify_output<__float128>(); >> > else >> > std::puts("Cannot format __float128 on this target"); >> > #endif >> > #if __FLT128_DIG__ >> > if constexpr (formattable<_Float128>) >> >- VERIFY( format_float<_Float128>() ); >> >+ verify_output<_Float128>(); >> > else >> > std::puts("Cannot format _Float128 on this target"); >> > #endif >> >@@ -75,7 +79,7 @@ test_bfloat16() >> > using bfloat16_t = decltype(0.0bf16); >> > >> > if constexpr (formattable<bfloat16_t>) >> >- VERIFY( format_float<bfloat16_t>() ); >> >+ verify_output<bfloat16_t>(); >> > else >> > std::puts("Cannot format bfloat16_t on this target"); >> > #endif >> >-- >> >2.55.0 >> > >> > >> >>