From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2181) id 91F113858401; Sun, 21 Jan 2024 22:25:46 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 91F113858401 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1705875946; bh=nBlobMAeKUvwOOhu9PrsAwtB9TSRzU1q7yJ8Pab94hU=; h=From:To:Subject:Date:From; b=RG+eBaxn4jz2VDugyDUVD5/KVTeEFUZZMRbosqmlpODF0TUgyJKV7SA6/CXgvVD/M cnSwnnrpe5jahwwHdq2aI8GlEwM0vArt+NexXqGjfLwgh0/lB5cq8PJapQxC3Xakhv C+DSfDb+ZuLX/qK16rn9rVmKz2t7fMXKXhtyvszQ= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jonathan Wakely To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org Subject: [gcc r14-8321] libstdc++: Fix std::format for floating-point chrono::time_point [PR113500] X-Act-Checkin: gcc X-Git-Author: Jonathan Wakely X-Git-Refname: refs/heads/master X-Git-Oldrev: fba15da8fa518bfb8d3e061795bc3ca2dea01d27 X-Git-Newrev: 7431fcea6b72beb54abb1932c254ac0e76bd0bde Message-Id: <20240121222546.91F113858401@sourceware.org> Date: Sun, 21 Jan 2024 22:25:46 +0000 (GMT) List-Id: https://gcc.gnu.org/g:7431fcea6b72beb54abb1932c254ac0e76bd0bde commit r14-8321-g7431fcea6b72beb54abb1932c254ac0e76bd0bde Author: Jonathan Wakely Date: Sun Jan 21 18:16:14 2024 +0000 libstdc++: Fix std::format for floating-point chrono::time_point [PR113500] Currently trying to use std::format with certain specializations of std::chrono::time_point is ill-formed, due to one member function of the __formatter_chrono type which tries to write a time_point to an ostream. For sys_time or sys_time with a period greater than days there is no operator<< that can be used. That operator<< is only needed when using an empty chrono-specs in the format string, like "{}", but the ill-formed expression gives an error even if not actually used. This means it's not possible to format some other specializations of chrono::time_point, even when using a non-empty chrono-specs. This fixes it by avoiding using 'os << t' for all chrono::time_point specializations, and instead using std::format("{:L%F %T}", t). So that we continue to reject std::format("{}", sys_time{1.0s}) a check for empty chrono-specs is added to the formatter, C> specialization. While testing this I noticed that the output for %S with a floating-point duration was incorrect, as the subseconds part was being appended to the seconds without a decimal point, and without the correct number of leading zeros. libstdc++-v3/ChangeLog: PR libstdc++/113500 * include/bits/chrono_io.h (__formatter_chrono::_M_S): Fix printing of subseconds with floating-point rep. (__formatter_chrono::_M_format_to_ostream): Do not write time_point specializations directly to the ostream. (formatter, C>::parse): Do not allow an empty chrono-spec if the type fails to meet the constraints for writing to an ostream with operator<<. * testsuite/std/time/clock/file/io.cc: Check formatting non-integral times with empty chrono-specs. * testsuite/std/time/clock/gps/io.cc: Likewise. * testsuite/std/time/clock/utc/io.cc: Likewise. * testsuite/std/time/hh_mm_ss/io.cc: Likewise. Diff: --- libstdc++-v3/include/bits/chrono_io.h | 71 +++++++++++++++++------- libstdc++-v3/testsuite/std/time/clock/file/io.cc | 17 ++++++ libstdc++-v3/testsuite/std/time/clock/gps/io.cc | 27 +++++++++ libstdc++-v3/testsuite/std/time/clock/utc/io.cc | 4 ++ libstdc++-v3/testsuite/std/time/hh_mm_ss/io.cc | 28 +++++++++- 5 files changed, 124 insertions(+), 23 deletions(-) diff --git a/libstdc++-v3/include/bits/chrono_io.h b/libstdc++-v3/include/bits/chrono_io.h index 7b5876b24e6..82f2d39ec44 100644 --- a/libstdc++-v3/include/bits/chrono_io.h +++ b/libstdc++-v3/include/bits/chrono_io.h @@ -681,6 +681,7 @@ namespace __format return __fc.locale(); } + // Format for empty chrono-specs, e.g. "{}" (C++20 [time.format] p6). // TODO: consider moving body of every operator<< into this function // and use std::format("{}", t) to implement those operators. That // would avoid std::format("{}", t) calling operator<< which calls @@ -702,6 +703,22 @@ namespace __format if constexpr (__is_specialization_of<_Tp, __utc_leap_second>) __os << __t._M_date << ' ' << __t._M_time; + else if constexpr (chrono::__is_time_point_v<_Tp>) + { + // Need to be careful here because not all specializations + // of chrono::sys_time can be written to an ostream. + // For the specializations of time_point that can be + // formatted with an empty chrono-specs, either it's a + // sys_time with period greater or equal to days: + if constexpr (is_convertible_v<_Tp, chrono::sys_days>) + __os << _S_date(__t); + else // Or it's formatted as "{:L%F %T}": + { + auto __days = chrono::floor(__t); + __os << chrono::year_month_day(__days) << ' ' + << chrono::hh_mm_ss(__t - __days); + } + } else { if constexpr (chrono::__is_duration_v<_Tp>) @@ -1122,39 +1139,43 @@ namespace __format 'S', 'O'); } - __out = __format::__write(std::move(__out), - _S_two_digits(__hms.seconds().count())); - if constexpr (__hms.fractional_width != 0) + if constexpr (__hms.fractional_width == 0) + __out = __format::__write(std::move(__out), + _S_two_digits(__hms.seconds().count())); + else { locale __loc = _M_locale(__ctx); + auto __s = __hms.seconds(); auto __ss = __hms.subseconds(); using rep = typename decltype(__ss)::rep; if constexpr (is_floating_point_v) { + chrono::duration __fs = __s + __ss; __out = std::format_to(std::move(__out), __loc, - _GLIBCXX_WIDEN("{:.{}Lg}"), - __ss.count(), - __hms.fractional_width); - } - else if constexpr (is_integral_v) - { - const auto& __np - = use_facet>(__loc); - __out = std::format_to(std::move(__out), - _GLIBCXX_WIDEN("{}{:0{}}"), - __np.decimal_point(), - __ss.count(), + _GLIBCXX_WIDEN("{:#0{}.{}Lf}"), + __fs.count(), + 3 + __hms.fractional_width, __hms.fractional_width); } else { const auto& __np = use_facet>(__loc); + __out = __format::__write(std::move(__out), + _S_two_digits(__s.count())); *__out++ = __np.decimal_point(); - auto __str = std::format(_S_empty_spec, __ss.count()); - __out = std::format_to(_GLIBCXX_WIDEN("{:0>{}s}"), - __str, - __hms.fractional_width); + if constexpr (is_integral_v) + __out = std::format_to(std::move(__out), + _GLIBCXX_WIDEN("{:0{}}"), + __ss.count(), + __hms.fractional_width); + else + { + auto __str = std::format(_S_empty_spec, __ss.count()); + __out = std::format_to(_GLIBCXX_WIDEN("{:0>{}s}"), + __str, + __hms.fractional_width); + } } } return __out; @@ -1911,7 +1932,13 @@ namespace __format template constexpr typename _ParseContext::iterator parse(_ParseContext& __pc) - { return _M_f._M_parse(__pc, __format::_ZonedDateTime); } + { + auto __next = _M_f._M_parse(__pc, __format::_ZonedDateTime); + if constexpr (!__stream_insertable) + if (_M_f._M_spec._M_chrono_specs.empty()) + __format::__invalid_chrono_spec(); // chrono-specs can't be empty + return __next; + } template typename _FormatContext::iterator @@ -1920,6 +1947,10 @@ namespace __format { return _M_f._M_format(__t, __fc); } private: + static constexpr bool __stream_insertable + = requires (basic_ostream<_CharT>& __os, + chrono::sys_time<_Duration> __t) { __os << __t; }; + __format::__formatter_chrono<_CharT> _M_f; }; diff --git a/libstdc++-v3/testsuite/std/time/clock/file/io.cc b/libstdc++-v3/testsuite/std/time/clock/file/io.cc index 39eb0dbcf51..3eee6bf5cc2 100644 --- a/libstdc++-v3/testsuite/std/time/clock/file/io.cc +++ b/libstdc++-v3/testsuite/std/time/clock/file/io.cc @@ -17,6 +17,23 @@ test_ostream() VERIFY( ss1.str() == ss2.str() ); } +void +test_format() +{ + using namespace std::chrono; + auto t = file_clock::now(); + + auto s = std::format("{}", t); + std::ostringstream ss; + ss << t; + VERIFY( s == ss.str() ); + + // PR libstdc++/113500 + auto ft = clock_cast(sys_days(2024y/January/21)) + 0ms + 2.5s; + s = std::format("{}", ft); + VERIFY( s == "2024-01-17 00:00:02.500"); +} + void test_parse() { diff --git a/libstdc++-v3/testsuite/std/time/clock/gps/io.cc b/libstdc++-v3/testsuite/std/time/clock/gps/io.cc index 6f4544fcd97..5d15713c69d 100644 --- a/libstdc++-v3/testsuite/std/time/clock/gps/io.cc +++ b/libstdc++-v3/testsuite/std/time/clock/gps/io.cc @@ -3,6 +3,7 @@ #include #include +#include #include void @@ -16,6 +17,32 @@ test_ostream() auto s = format("{0:%F %T %Z} == {1:%F %T %Z}", st, gt); VERIFY( s == "2000-01-01 00:00:00 UTC == 2000-01-01 00:00:13 GPS" ); + + std::ostringstream ss; + ss << gt; + VERIFY( ss.str() == "2000-01-01 00:00:13" ); + + gps_time> gtf = gt; + ss.str(""); + ss.clear(); + ss << (gps_time>(gt) + 20ms); + VERIFY( ss.str() == "2000-01-01 00:00:13.020" ); +} + +void +test_format() +{ + using std::format; + using namespace std::chrono; + + auto st = sys_days{2000y/January/1}; + auto gt = clock_cast(st); + auto s = std::format("{}", gt); + VERIFY( s == "2000-01-01 00:00:13" ); + + // PR libstdc++/113500 + s = std::format("{}", gt + 150ms + 10.5s); + VERIFY( s == "2000-01-01 00:00:35.650" ); } void diff --git a/libstdc++-v3/testsuite/std/time/clock/utc/io.cc b/libstdc++-v3/testsuite/std/time/clock/utc/io.cc index 58e358f3dbf..55c53dc4057 100644 --- a/libstdc++-v3/testsuite/std/time/clock/utc/io.cc +++ b/libstdc++-v3/testsuite/std/time/clock/utc/io.cc @@ -112,6 +112,10 @@ test_format() s = std::format("{:%T}", leap + 1s); VERIFY( s == "00:00:00" ); + + // PR libstdc++/113500 + s = std::format("{}", leap + 100ms + 2.5s); + VERIFY( s == "2017-01-01 00:00:01.600"); } void diff --git a/libstdc++-v3/testsuite/std/time/hh_mm_ss/io.cc b/libstdc++-v3/testsuite/std/time/hh_mm_ss/io.cc index 8eb80422e6e..d651dfbcdc8 100644 --- a/libstdc++-v3/testsuite/std/time/hh_mm_ss/io.cc +++ b/libstdc++-v3/testsuite/std/time/hh_mm_ss/io.cc @@ -35,9 +35,31 @@ test_ostream() VERIFY( out.str() == "18:15:45.123" ); } - ostringstream out; - out << hh_mm_ss{65745s}; - VERIFY( out.str() == "18:15:45" ); + { + ostringstream out; + out << hh_mm_ss{65745s}; + VERIFY( out.str() == "18:15:45" ); + } + + { + ostringstream out; + out << hh_mm_ss{0.020s}; + // hh_mm_ss>::fractional_width == 0 so no subseconds: + VERIFY( out.str() == "00:00:00" ); + } + + { + ostringstream out; + out << hh_mm_ss>{0.020s}; + // hh_mm_ss>::fractional_width == 9: + VERIFY( out.str() == "00:00:00.020000000" ); + } + + { + ostringstream out; + out << hh_mm_ss{65745s + 20ms}; + VERIFY( out.str() == "18:15:45.020" ); + } } void