[libcxx-commits] [PATCH] D134742: [libc++][chrono] Implements formatter duration.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/include/__chrono/convert_to_tm.h:43
+    //   day elapsed since midnight.
+    uint64_t __sec = chrono::duration_cast<chrono::seconds>(__value).count();
+    __sec %= 24 * 3600;
----------------
EricWF wrote:
> Does this not overflow for a lot of things?
> 
> Like `hours(duration_cast<hours>(seconds::max()) + hours(1))`?
> 
> Instead of reverting to raw arithmetic on ints, use chrono to get the values you want.
> 
> See https://godbolt.org/z/GYc7Tzn9W
> 
The value is intended to be time since midnight, but it indeed may overflow.

Unfortunately it's not as simple as you suggest. Casting to hours fails for other small units, for example atto seconds. D138826 addresses the issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134742/new/

https://reviews.llvm.org/D134742