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



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

Reply via email to