[PATCH] D25416: Add Chrono.h - std::chrono support header

[Zachary Turner via llvm-commits]

zturner added inline comments.


================
Comment at: include/llvm/Support/Chrono.h:26-27
+/// implementation-defined).
+using TimePoint = std::chrono::time_point<std::chrono::system_clock,
+                                          std::chrono::nanoseconds>;
+
----------------
labath wrote:
> zturner wrote:
> > I'm not sure if it's appropriate to have a generic name like `TimePoint` (which indicates it should be used for any and all time calculations) be forced to a specific resolution.  A couple of possibilities would be:
> > 
> > 1. Call it `TimePointNano`
> > 2. making it an alias template (e.g. `template<typename R> using TimePoint = time_point<system_clock, T>`)
> I wanted to make the usage as frictionless as possible (and TimeValue already used nanoseconds precision and noone seemed to be bothered by that), but I do see your point. How about we go for the second option, but we have a default template parameter, so people don't have to specify precision unless they actually need to. I have two counter-proposals:
> - `template<typename D = nanoseconds> using TimePoint = time_point<system_clock, D>`, with the usage being: `TimePoint<std::chrono::microseconds>`
> - `template<typename R = nano> using TimePoint = time_point<system_clock, duration<XXX, R>>`, with the usage being `TimePoint<std::micro>`
> 
> I'd tend towards the second option as it is shorter if you don't import std::chrono namespace, but I don't care too much, as I don't anticipate much usage with a custom precision.
I agree, it might not ever be used, but who knows.  The second usage looks fine to me.


================
Comment at: include/llvm/Support/Chrono.h:37
+/// Convert a std::time_t to a TimePoint
+inline TimePoint toTimePoint(std::time_t T) {
+  return std::chrono::system_clock::from_time_t(T);
----------------
labath wrote:
> zturner wrote:
> > Same.
> I suppose we could return a time_point with second precision, as that is all that's stored in a time_t anyway. I don't think we need templates, because of implicit conversions.
Ok, as long as there are implicit conversions then no worries.  Can you add a unit test that exercises one of the implicit conversions?


https://reviews.llvm.org/D25416