[PATCH] D97061: [llvm-cov] Cache file status information

Vedant Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 2 11:56:56 PST 2021


vsk added a comment.

This looks nice.



================
Comment at: llvm/tools/llvm-cov/CodeCoverage.cpp:269
+                                        StringRef FilePath2) {
+  auto file_status1 = getFileStatus(FilePath1);
+  auto file_status2 = getFileStatus(FilePath1);
----------------
Please keep the naming convention consistent (e.g. 'Status1' might work better here).


================
Comment at: llvm/tools/llvm-cov/CodeCoverage.cpp:288
       return *Files.second;
+
   auto Buffer = MemoryBuffer::getFile(SourceFile);
----------------
Please leave out unrelated whitespace changes.


================
Comment at: llvm/tools/llvm-cov/CodeCoverage.cpp:260
+
+  Optional<sys::fs::file_status> result;
+  sys::fs::file_status status;
----------------
The `result` variable shouldn't be necessary - the Optional should be implicitly converted wherever `status` is used.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97061/new/

https://reviews.llvm.org/D97061



More information about the llvm-commits mailing list