[Lldb-commits] [PATCH] D25392: Remove TimeValue usage from FileSpec.h

[Pavel Labath via lldb-commits]

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