[libcxx-commits] [PATCH] D138826: [libc++][chrono] Fixes formatter duration.
Eric Fiselier via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Mar 6 12:18:44 PST 2023
EricWF requested changes to this revision.
EricWF added inline comments.
This revision now requires changes to proceed.
================
Comment at: libcxx/include/__chrono/convert_to_tm.h:88
+ // conversion errors. In that case the conversion to seconds works.
+ if constexpr (is_convertible_v<_ChronoT, chrono::hours>) {
+ auto __hour = chrono::floor<chrono::hours>(__value);
----------------
In what case can it be converted to seconds but not hours?
Also is there a test case for the case for the case where the type cannot be converted to hours?
================
Comment at: libcxx/test/std/time/time.syn/formatter.duration.pass.cpp:1089
+ check(SV("06:20:44.415"), SV("{:%T}"), std::chrono::milliseconds{0x0000'fff'ffff'ffffll}); // 45 bit signed value max
+ check(SV("01:53:03"), SV("{:%T}"), std::chrono::seconds{0x0000'0003'ffff'ffffll}); // 35 bit signed value max
+ check(SV("12:15:00"), SV("{:%T}"), std::chrono::minutes{0x0fff'ffff}); // 29 bit signed value max
----------------
Can't we just use `std::chrono::years::max`?
Do these test cases fail before your fixes? It looks to me that they're actively avoiding constructing test values that will overflow.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138826/new/
https://reviews.llvm.org/D138826
More information about the libcxx-commits
mailing list