From b349c651ff16240b8cc4225db76479154c941c28 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely <jwakely@redhat.com> Date: Wed, 18 Sep 2024 17:20:29 +0100 Subject: [PATCH] libstdc++: Fix formatting of chrono::duration with character rep [PR116755] Implement Peter Dimov's suggestion for resolving LWG 4118, which is to use +d.count() so that character types are promoted to an integer type before formatting them. This didn't have unanimous consensus in the committee as Howard Hinnant proposed that we should format the rep consistently with std::format("{}", d.count()) instead. That ends up being more complicated, because it makes std::formattable a precondition of operator<< which was not previously the case, and it means that ios_base::fmtflags from the stream would be ignored because std::format doesn't use them. libstdc++-v3/ChangeLog: PR libstdc++/116755 * include/bits/chrono_io.h (operator<<): Use +d.count() for duration inserter. (__formatter_chrono::_M_format): Likewise for %Q format. * testsuite/20_util/duration/io.cc: Test durations with character types as reps. --- libstdc++-v3/include/bits/chrono_io.h | 9 ++- libstdc++-v3/testsuite/20_util/duration/io.cc | 66 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/libstdc++-v3/include/bits/chrono_io.h b/libstdc++-v3/include/bits/chrono_io.h index 362bb5aa9e98..a337007266e9 100644 --- a/libstdc++-v3/include/bits/chrono_io.h +++ b/libstdc++-v3/include/bits/chrono_io.h @@ -150,7 +150,9 @@ namespace __detail __s.flags(__os.flags()); __s.imbue(__os.getloc()); __s.precision(__os.precision()); - __s << __d.count(); + // _GLIBCXX_RESOLVE_LIB_DEFECTS + // 4118. How should duration formatters format custom rep types? + __s << +__d.count(); __detail::__fmt_units_suffix<period, _CharT>(_Out(__s)); __os << std::move(__s).str(); return __os; @@ -635,8 +637,10 @@ namespace __format case 'Q': // %Q The duration's numeric value. if constexpr (chrono::__is_duration_v<_Tp>) + // _GLIBCXX_RESOLVE_LIB_DEFECTS + // 4118. How should duration formatters format custom rep? __out = std::format_to(__print_sign(), _S_empty_spec, - __t.count()); + +__t.count()); else __throw_format_error("chrono format error: argument is " "not a duration"); @@ -1703,6 +1707,7 @@ namespace __format /// @endcond template<typename _Rep, typename _Period, typename _CharT> + requires __format::__formattable_impl<_Rep, _CharT> struct formatter<chrono::duration<_Rep, _Period>, _CharT> { constexpr typename basic_format_parse_context<_CharT>::iterator diff --git a/libstdc++-v3/testsuite/20_util/duration/io.cc b/libstdc++-v3/testsuite/20_util/duration/io.cc index 57020f4f9537..383fb60afe2a 100644 --- a/libstdc++-v3/testsuite/20_util/duration/io.cc +++ b/libstdc++-v3/testsuite/20_util/duration/io.cc @@ -23,6 +23,24 @@ test01() VERIFY( s == "3[2]s" ); std::getline(ss, s); VERIFY( s == "9[2/3]s" ); + + // LWG 4118. How should duration formatters format custom rep types? + ss.str(""); + ss << duration<char>(121) << ' '; + ss << duration<wchar_t>(122) << ' '; + ss << duration<signed char>(123) << ' '; + ss << duration<unsigned char>(124) << ' '; + ss << duration<char8_t>(125) << ' '; + ss << duration<char16_t>(126) << ' '; + ss << duration<char32_t>(127) << ' '; + VERIFY( ss.str() == "121s 122s 123s 124s 125s 126s 127s " ); + + ss.str(""); + ss << std::hex << std::uppercase << duration<const char>(0x1A) << ' '; + ss << std::hex << std::uppercase << duration<const wchar_t>(0x2A) << ' '; + ss << std::hex << std::uppercase << duration<signed char>(0x3A) << ' '; + ss << std::scientific << duration<const double>(4.5) << ' '; + VERIFY( ss.str() == "1As 2As 3As 4.500000E+00s " ); } void @@ -44,6 +62,24 @@ test02() VERIFY( s == L"3[2]s" ); std::getline(ss, s); VERIFY( s == L"9[2/3]s" ); + + // LWG 4118. How should duration formatters format custom rep types? + ss.str(L""); + ss << duration<char>(121) << ' '; + ss << duration<wchar_t>(122) << ' '; + ss << duration<signed char>(123) << ' '; + ss << duration<unsigned char>(124) << ' '; + ss << duration<char8_t>(125) << ' '; + ss << duration<char16_t>(126) << ' '; + ss << duration<char32_t>(127) << ' '; + VERIFY( ss.str() == L"121s 122s 123s 124s 125s 126s 127s " ); + + ss.str(L""); + ss << std::hex << std::uppercase << duration<const char>(0x1A) << ' '; + ss << std::hex << std::uppercase << duration<const wchar_t>(0x2A) << ' '; + ss << std::hex << std::uppercase << duration<signed char>(0x3A) << ' '; + ss << std::scientific << duration<const double>(4.5) << ' '; + VERIFY( ss.str() == L"1As 2As 3As 4.500000E+00s " ); #endif } @@ -114,6 +150,36 @@ test_format() VERIFY( s == expected ); s = std::format("{:%Q%q}", minsec); VERIFY( s == expected ); + + // LWG 4118. How should duration formatters format custom rep types? + s = std::format("{}", std::chrono::duration<char>(100)); + VERIFY( s == "100s" ); + s = std::format("{:%Q}", std::chrono::duration<char>(101)); + VERIFY( s == "101" ); +#ifdef _GLIBCXX_USE_WCHAR_T + ws = std::format(L"{}", std::chrono::duration<char>(102)); + VERIFY( ws == L"102s" ); + ws = std::format(L"{}", std::chrono::duration<wchar_t>(103)); + VERIFY( ws == L"103s" ); +#endif + s = std::format("{}", std::chrono::duration<signed char>(50)); + VERIFY( s == "50s" ); + s = std::format("{:%Q}", std::chrono::duration<signed char>(51)); + VERIFY( s == "51" ); + s = std::format("{}", std::chrono::duration<unsigned char>(52)); + VERIFY( s == "52s" ); + s = std::format("{:%Q}", std::chrono::duration<unsigned char>(53)); + VERIFY( s == "53" ); + +#if __cplusplus > 202002L + static_assert( ! std::formattable<std::chrono::duration<wchar_t>, char> ); + static_assert( ! std::formattable<std::chrono::duration<char8_t>, char> ); + static_assert( ! std::formattable<std::chrono::duration<char16_t>, char> ); + static_assert( ! std::formattable<std::chrono::duration<char32_t>, char> ); + static_assert( ! std::formattable<std::chrono::duration<char8_t>, wchar_t> ); + static_assert( ! std::formattable<std::chrono::duration<char16_t>, wchar_t> ); + static_assert( ! std::formattable<std::chrono::duration<char32_t>, wchar_t> ); +#endif } void -- GitLab