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

[Pavel Labath via lldb-commits]

labath added inline comments.


================
Comment at: include/lldb/Host/FileSystem.h:69-71
+  static std::chrono::time_point<std::chrono::system_clock,
+                                 std::chrono::nanoseconds>
+  GetModificationTime(const FileSpec &file_spec);
----------------
labath wrote:
> zturner wrote:
> > I wonder if it would be worth defining some typedefs in LLVM's `Chrono.h` that Mehdi is adding to make things like this less verbose.  For example:
> > 
> > ```
> > llvm::nanosecond_time_point
> > llvm::microsecond_time_point
> > llvm::nanosecond_duration
> > llvm::microsecond_duration
> > ```
> > 
> > Now, with all that aside, I'm not sure we actually need this function here (or many of the other functions for that matter).  LLVM has `llvm::support::fs::status()` which will return you a `file_status` object which contains the modification time.  I wonder if we should use that instead.  It currently uses an `llvm::TimeValue`, but the conversion to `chrono` could happen at that level presumably.
> That's a good point. I can check how easy it would be to do that. I'd like to avoid refactoring the filesystem code just to get the time value conversion done though.
I've changed this to use llvm::sys::fs::status() underneath (which now uses chrono). I am still keeping this function, as it has a somewhat different interface (takes FileSpec, returns an empty time point in case of an error), and I don't want to update all callers now.


https://reviews.llvm.org/D25392