labath added inline comments.

================
Comment at: include/lldb/Host/TimeValue.h:37-38
   explicit TimeValue(uint32_t seconds, uint64_t nanos = 0);
+  TimeValue(std::chrono::time_point<std::chrono::system_clock,
+                                    std::chrono::nanoseconds>
+                point)
----------------
labath wrote:
> zturner wrote:
> > Is there any reason to explicitly prefer a nanosecond clock here as opposed 
> > to `std::chrono::system_clock::duration`?  For any system which doesn't 
> > have nanosecond resolution, using a nanosecond clock is pointless, and if 
> > some theoretical system had finer resolution, then using a nanosecond clock 
> > would be limiting.  So how about just 
> > `std::chrono::time_point<std::chrono::system_clock>` and let the final 
> > argument be deduced?
> If you let this argument be less precise you will get conversion errors if 
> someone tries to pass in a time point which has higher precision (seconds are 
> implicitly convertible to nanoseconds, but not the other way). And struct 
> timespec already (theoretically) has nanosecond precision, so we would have 
> to make an explicit choice to lose the precision  at some level.
> 
> `chrono::system_clock::time_point` has the precision at which the host system 
> clock hands out timestamps, but that doesn't mean it's impossible to get 
> higher precision timestamps, particularly if we start dealing with remote 
> systems.
I've hit one more annoying issue `system_clock::to_time_t` will not accept a 
time point with precision greater than native system clock precision. Sort of 
makes sense, although it's pretty pointless since time_t has only second 
precision on all reasonable platforms. This means, if we use nanosecond 
precision everywhere, we would have to cast, or roll our own version of 
`to_time_t`. If we use native system clock precision, we lose the ability to 
represent nanoseconds in some cases (and there are filesystems storing 
timestamps with nanosecond precision), cannot represent `struct timespec` nor 
the current `TimeValue`s correctly, and our time functions would behave 
differently depending on the host os. I'd go with the first option as it seems 
to have less drawbacks.

I'll try to come up with a coherent proposal on the llvm side. Since it seems 
we'll be putting these things there, I feel I should start by porting the usage 
in llvm anyway.


https://reviews.llvm.org/D25392



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to