Skip to content

Commit a5aca83

Browse files
committed
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<floating-point> 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<sys_time<D>, 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<chrono::sys_time<D>, 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. (cherry picked from commit 7431fce)
1 parent 4cdcfb4 commit a5aca83

File tree

5 files changed

+126
-23
lines changed

5 files changed

+126
-23
lines changed

libstdc++-v3/include/bits/chrono_io.h

+51-20
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,7 @@ namespace __format
687687
return __fc.locale();
688688
}
689689

690+
// Format for empty chrono-specs, e.g. "{}" (C++20 [time.format] p6).
690691
// TODO: consider moving body of every operator<< into this function
691692
// and use std::format("{}", t) to implement those operators. That
692693
// would avoid std::format("{}", t) calling operator<< which calls
@@ -708,6 +709,22 @@ namespace __format
708709

709710
if constexpr (__is_specialization_of<_Tp, __utc_leap_second>)
710711
__os << __t._M_date << ' ' << __t._M_time;
712+
else if constexpr (chrono::__is_time_point_v<_Tp>)
713+
{
714+
// Need to be careful here because not all specializations
715+
// of chrono::sys_time can be written to an ostream.
716+
// For the specializations of time_point that can be
717+
// formatted with an empty chrono-specs, either it's a
718+
// sys_time with period greater or equal to days:
719+
if constexpr (is_convertible_v<_Tp, chrono::sys_days>)
720+
__os << _S_date(__t);
721+
else // Or it's formatted as "{:L%F %T}":
722+
{
723+
auto __days = chrono::floor<chrono::days>(__t);
724+
__os << chrono::year_month_day(__days) << ' '
725+
<< chrono::hh_mm_ss(__t - __days);
726+
}
727+
}
711728
else
712729
{
713730
if constexpr (chrono::__is_duration_v<_Tp>)
@@ -1150,39 +1167,43 @@ namespace __format
11501167
'S', 'O');
11511168
}
11521169

1153-
__out = __format::__write(std::move(__out),
1154-
_S_two_digits(__hms.seconds().count()));
1155-
if constexpr (__hms.fractional_width != 0)
1170+
if constexpr (__hms.fractional_width == 0)
1171+
__out = __format::__write(std::move(__out),
1172+
_S_two_digits(__hms.seconds().count()));
1173+
else
11561174
{
11571175
locale __loc = _M_locale(__ctx);
1176+
auto __s = __hms.seconds();
11581177
auto __ss = __hms.subseconds();
11591178
using rep = typename decltype(__ss)::rep;
11601179
if constexpr (is_floating_point_v<rep>)
11611180
{
1181+
chrono::duration<rep> __fs = __s + __ss;
11621182
__out = std::format_to(std::move(__out), __loc,
1163-
_GLIBCXX_WIDEN("{:.{}Lg}"),
1164-
__ss.count(),
1165-
__hms.fractional_width);
1166-
}
1167-
else if constexpr (is_integral_v<rep>)
1168-
{
1169-
const auto& __np
1170-
= use_facet<numpunct<_CharT>>(__loc);
1171-
__out = std::format_to(std::move(__out),
1172-
_GLIBCXX_WIDEN("{}{:0{}}"),
1173-
__np.decimal_point(),
1174-
__ss.count(),
1183+
_GLIBCXX_WIDEN("{:#0{}.{}Lf}"),
1184+
__fs.count(),
1185+
3 + __hms.fractional_width,
11751186
__hms.fractional_width);
11761187
}
11771188
else
11781189
{
11791190
const auto& __np
11801191
= use_facet<numpunct<_CharT>>(__loc);
1192+
__out = __format::__write(std::move(__out),
1193+
_S_two_digits(__s.count()));
11811194
*__out++ = __np.decimal_point();
1182-
auto __str = std::format(_S_empty_spec, __ss.count());
1183-
__out = std::format_to(_GLIBCXX_WIDEN("{:0>{}s}"),
1184-
__str,
1185-
__hms.fractional_width);
1195+
if constexpr (is_integral_v<rep>)
1196+
__out = std::format_to(std::move(__out),
1197+
_GLIBCXX_WIDEN("{:0{}}"),
1198+
__ss.count(),
1199+
__hms.fractional_width);
1200+
else
1201+
{
1202+
auto __str = std::format(_S_empty_spec, __ss.count());
1203+
__out = std::format_to(_GLIBCXX_WIDEN("{:0>{}s}"),
1204+
__str,
1205+
__hms.fractional_width);
1206+
}
11861207
}
11871208
}
11881209
return __out;
@@ -1939,7 +1960,13 @@ namespace __format
19391960
template<typename _ParseContext>
19401961
constexpr typename _ParseContext::iterator
19411962
parse(_ParseContext& __pc)
1942-
{ return _M_f._M_parse(__pc, __format::_ZonedDateTime); }
1963+
{
1964+
auto __next = _M_f._M_parse(__pc, __format::_ZonedDateTime);
1965+
if constexpr (!__stream_insertable)
1966+
if (_M_f._M_spec._M_chrono_specs.empty())
1967+
__format::__invalid_chrono_spec(); // chrono-specs can't be empty
1968+
return __next;
1969+
}
19431970

19441971
template<typename _FormatContext>
19451972
typename _FormatContext::iterator
@@ -1948,6 +1975,10 @@ namespace __format
19481975
{ return _M_f._M_format(__t, __fc); }
19491976

19501977
private:
1978+
static constexpr bool __stream_insertable
1979+
= requires (basic_ostream<_CharT>& __os,
1980+
chrono::sys_time<_Duration> __t) { __os << __t; };
1981+
19511982
__format::__formatter_chrono<_CharT> _M_f;
19521983
};
19531984

libstdc++-v3/testsuite/std/time/clock/file/io.cc

+18
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,25 @@ test_ostream()
1818
VERIFY( ss1.str() == ss2.str() );
1919
}
2020

21+
void
22+
test_format()
23+
{
24+
using namespace std::chrono;
25+
auto t = file_clock::now();
26+
27+
auto s = std::format("{}", t);
28+
std::ostringstream ss;
29+
ss << t;
30+
VERIFY( s == ss.str() );
31+
32+
// PR libstdc++/113500
33+
auto ft = clock_cast<file_clock>(sys_days(2024y/January/21)) + 0ms + 2.5s;
34+
s = std::format("{}", ft);
35+
VERIFY( s == "2024-01-21 00:00:02.500");
36+
}
37+
2138
int main()
2239
{
2340
test_ostream();
41+
test_format();
2442
}

libstdc++-v3/testsuite/std/time/clock/gps/io.cc

+28
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include <chrono>
66
#include <format>
7+
#include <sstream>
78
#include <testsuite_hooks.h>
89

910
void
@@ -17,9 +18,36 @@ test01()
1718

1819
auto s = format("{0:%F %T %Z} == {1:%F %T %Z}", st, gt);
1920
VERIFY( s == "2000-01-01 00:00:00 UTC == 2000-01-01 00:00:13 GPS" );
21+
22+
std::ostringstream ss;
23+
ss << gt;
24+
VERIFY( ss.str() == "2000-01-01 00:00:13" );
25+
26+
gps_time<duration<float>> gtf = gt;
27+
ss.str("");
28+
ss.clear();
29+
ss << (gps_time<duration<long double>>(gt) + 20ms);
30+
VERIFY( ss.str() == "2000-01-01 00:00:13.020" );
31+
}
32+
33+
void
34+
test_format()
35+
{
36+
using std::format;
37+
using namespace std::chrono;
38+
39+
auto st = sys_days{2000y/January/1};
40+
auto gt = clock_cast<gps_clock>(st);
41+
auto s = std::format("{}", gt);
42+
VERIFY( s == "2000-01-01 00:00:13" );
43+
44+
// PR libstdc++/113500
45+
s = std::format("{}", gt + 150ms + 10.5s);
46+
VERIFY( s == "2000-01-01 00:00:23.650" );
2047
}
2148

2249
int main()
2350
{
2451
test01();
52+
test_format();
2553
}

libstdc++-v3/testsuite/std/time/clock/utc/io.cc

+4
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ test_format()
113113

114114
s = std::format("{:%T}", leap + 1s);
115115
VERIFY( s == "00:00:00" );
116+
117+
// PR libstdc++/113500
118+
s = std::format("{}", leap + 100ms + 2.5s);
119+
VERIFY( s == "2017-01-01 00:00:01.600");
116120
}
117121

118122
int main()

libstdc++-v3/testsuite/std/time/hh_mm_ss/io.cc

+25-3
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,31 @@ test_ostream()
3636
VERIFY( out.str() == "18:15:45.123" );
3737
}
3838

39-
ostringstream out;
40-
out << hh_mm_ss{65745s};
41-
VERIFY( out.str() == "18:15:45" );
39+
{
40+
ostringstream out;
41+
out << hh_mm_ss{65745s};
42+
VERIFY( out.str() == "18:15:45" );
43+
}
44+
45+
{
46+
ostringstream out;
47+
out << hh_mm_ss{0.020s};
48+
// hh_mm_ss<duration<long double>>::fractional_width == 0 so no subseconds:
49+
VERIFY( out.str() == "00:00:00" );
50+
}
51+
52+
{
53+
ostringstream out;
54+
out << hh_mm_ss<std::chrono::duration<long double, std::nano>>{0.020s};
55+
// hh_mm_ss<duration<long double, nano>>::fractional_width == 9:
56+
VERIFY( out.str() == "00:00:00.020000000" );
57+
}
58+
59+
{
60+
ostringstream out;
61+
out << hh_mm_ss{65745s + 20ms};
62+
VERIFY( out.str() == "18:15:45.020" );
63+
}
4264
}
4365

4466
void

0 commit comments

Comments
 (0)