[PATCH] D78961: [WIP][FileCollector] Add doc comments

[Jan Korous via Phabricator via llvm-commits]

jkorous created this revision.
jkorous added a reviewer: JDevlieghere.
Herald added subscribers: llvm-commits, dexonsmith, hiraditya.
Herald added a project: LLVM.

I would like to use `FileCollector` and figured out I might as well add things that weren't obvious to me as comments/names.

One thing that I still don't get is the function of `OverlayRoot`. It looks like it's exposing this prefix from `VFSWriter` but I am not sure why.


https://reviews.llvm.org/D78961

Files:
  llvm/include/llvm/Support/FileCollector.h
  llvm/lib/Support/FileCollector.cpp


Index: llvm/lib/Support/FileCollector.cpp
===================================================================
--- llvm/lib/Support/FileCollector.cpp
+++ llvm/lib/Support/FileCollector.cpp
@@ -33,9 +33,8 @@
   return true;
 }
 
-FileCollector::FileCollector(std::string Root, std::string OverlayRoot)
-    : Root(std::move(Root)), OverlayRoot(std::move(OverlayRoot)) {
-}
+FileCollector::FileCollector(std::string StorageDir, std::string OverlayRoot)
+    : StorageDir(std::move(StorageDir)), OverlayRoot(std::move(OverlayRoot)) { }
 
 bool FileCollector::getRealPath(StringRef SrcPath,
                                 SmallVectorImpl<char> &Result) {
@@ -90,7 +89,7 @@
   if (!getRealPath(AbsoluteSrc, CopyFrom))
     CopyFrom = VirtualPath;
 
-  SmallString<256> DstPath = StringRef(Root);
+  SmallString<256> DstPath = StringRef(StorageDir);
   sys::path::append(DstPath, sys::path::relative_path(CopyFrom));
 
   // Always map a canonical src path to its real path into the YAML, by doing
@@ -123,7 +122,7 @@
 }
 
 std::error_code FileCollector::copyFiles(bool StopOnError) {
-  auto Err = sys::fs::create_directories(Root, true);
+  auto Err = sys::fs::create_directories(StorageDir, true);
   if (Err) {
     return Err;
   }
Index: llvm/include/llvm/Support/FileCollector.h
===================================================================
--- llvm/include/llvm/Support/FileCollector.h
+++ llvm/include/llvm/Support/FileCollector.h
@@ -19,11 +19,29 @@
 
 namespace llvm {
 
-/// Collects files into a directory and generates a mapping that can be used by
-/// the VFS.
+/// Collects files into a directory and generates a mapping for RedirectingFileSystem.
+///
+/// For given existing files we (at some point) create:
+/// - a copy of the file inside StorageDir
+/// - a record in RedirectingFileSystem mapping that maps:
+///   current real path -> path to the copy in StorageDir
+///
+/// That intent is that later when the mapping is used by RedirectingFileSystem
+/// it simulates the state of FS that we collected.
+///
+/// We generate file copies and mapping lazily - see writeMapping and copyFiles.
+/// We don't try to capture the state of the file at the exact time when it's accessed.
+/// Files might get changed, deleted ... we record only the "final" state.
+///
+/// In order to preserve the relative topology of files we use their real paths
+/// as relative paths inside of the StorageDir.
 class FileCollector {
 public:
-  FileCollector(std::string Root, std::string OverlayRoot);
+  /// \p StorageDir directory where collected files are going to be stored.
+  /// \p OverlayRoot expected root of all the files collected.
+  /// TODO What does it actually do apart from YAMLVFSWriter asserting on this?
+  /// TODO Is it meant to save space in YAML mapping?
+  FileCollector(std::string StorageDir, std::string OverlayRoot);
 
   void addFile(const Twine &file);
 
@@ -37,8 +55,7 @@
   /// removed after it was added to the mapping.
   std::error_code copyFiles(bool StopOnError = true);
 
-  /// Create a VFS that collects all the paths that might be looked at by the
-  /// file system accesses.
+  /// Create a VFS that uses \p Collector to collect files accessed via \p BaseFS.
   static IntrusiveRefCntPtr<vfs::FileSystem>
   createCollectorVFS(IntrusiveRefCntPtr<vfs::FileSystem> BaseFS,
                      std::shared_ptr<FileCollector> Collector);
@@ -63,11 +80,11 @@
   /// Synchronizes adding files.
   std::mutex Mutex;
 
-  /// The root directory where files are copied.
-  std::string Root;
+  /// The directory where collected files are copied to in copyFiles().
+  const std::string StorageDir;
 
   /// The root directory where the VFS overlay lives.
-  std::string OverlayRoot;
+  const std::string OverlayRoot;
 
   /// Tracks already seen files so they can be skipped.
   StringSet<> Seen;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78961.260424.patch
Type: text/x-patch
Size: 3841 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200427/bf508186/attachment.bin>