[VFS] Add print/dump to the whole FileSystem hierarchy

[bnbarham]

Summary

For now most are implemented by printing out the name of the filesystem,
but this can be expanded in the future. Only OverlayFileSystem and
RedirectingFileSystem are properly implemented in this patch.

• OverlayFileSystem: Prints each filesystem in the order that any operations are actually run on them. Optionally prints recursively.
• RedirectingFileSystem: Prints out all mappings, as well as the ExternalFS. Most of this was already implemented other than the handling for the DirectoryRemap case and to actually print out the mapping.

Each FS should implement printImpl rather than print, where the
latter just fowards to the former. This is to avoid spreading the
default arguments through to the subclasses (where we may miss updating
in the future).

[bnbarham]

This and https://reviews.llvm.org/D121423 can be committed by themselves, but https://reviews.llvm.org/D121424, https://reviews.llvm.org/D121425, and https://reviews.llvm.org/D121426 need to be pushed together - they're split to make reviewing easier but tests will fail until all are in.

EDIT: And I've failed at my split already. overlays_range is in a later commit :(

[bnbarham]

Added overlays_range to this patch.

[dexonsmith]

This is awesome! Maybe eventually we'll want more details about some of these but this is enough to be useful.

A few comments inline, but besides the rename I suggested from dump to print, up to you whether to address now or later. It'd also be nice to unit test print() for OverlayFileSystem; in which case you might want the extra flags I suggest inline.

llvm/include/llvm/Support/VirtualFileSystem.h

310

Usually, the dump that takes a raw_ostream is called print(). Can we follow that here too?

I also think it'd be useful to have it take a second argument for whether to print recursively. A bit awkward with virtual functions, but you can make it work nicely with indirection:

  void print(raw_ostream &OS, bool PrintContents = true, bool PrintRecursively = false) {
    printImpl(OS, PrintRecursively);
  }
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
  LLVM_DUMP_METHOD void dump() const; // { print(dbgs(), /*PrintContents=*/true, /*PrintRecursively=*/true); }
#endif

protected:
  virtual void printImpl(raw_ostream &OS, bool, bool) {}

947–952

I'd suggest renaming both of these in a prep commit (you probably don't need pre-commit review for that... as long as tests pass):

• print()
• printEntry()

and reserve dump() for when there's no argument. I think this better matches usage elsewhere in LLVM.

llvm/lib/Support/VirtualFileSystem.cpp

155

I don't think you need to repeat LLVM_DUMP_METHOD here. Either header or source is sufficient.

482–487

I'd suggest (with the new flags I suggested):

void OverlayFileSystem::printImpl(raw_ostream &OS, bool PrintContents, bool PrintRecursively) const {
  OS << "OverlayFileSystem:";
  if (!PrintContents) {
    OS << "\n";
    return;
  }

  for (auto FS : enumerate(overlays_range())) {
    OS << FS.index() << ") ";
    FS.value()->print(OS, /*PrintContents=*/PrintRecursively, PrintRecursively);
    OS << "\n";
  }
}

... or something... maybe the flags aren't quite right, but the idea is that print(OS, true, false) would print:

OverlayFileSystem:
FS1

without printing the contents of FS1.

Also makes me feel like an Indent would be useful.

[bnbarham]

In D121421#3376132, @dexonsmith wrote:

This is awesome! Maybe eventually we'll want more details about some of these but this is enough to be useful.

I almost implemented a print for InMemoryFileSystem but figured we could do that later if it becomes useful. The things I added were mostly what I found useful while debugging the rest of the changes here.

A few comments inline, but besides the rename I suggested from dump to print, up to you whether to address now or later. It'd also be nice to unit test print() for OverlayFileSystem; in which case you might want the extra flags I suggest inline.

Thanks for the review! All great suggestions that I think would be worth getting in now - they shouldn't take too long.

[bnbarham]

Address review comments

[bnbarham]

Forgot to update dump (have a release build locally)

[dexonsmith]

LGTM, with a couple of suggestions inline.

llvm/include/llvm/Support/VirtualFileSystem.h

321

Maybe worth an explanation in the commit message why print()'s virtual behaviour is split out to printImpl(); up to you though.

402

For this const/non-const pair you might skip the blank link between them.

[JDevlieghere]

Very cool!

[bnbarham]

Final review comments and small addition

Added "UseExternalName(s)" to the RedirectingFileSystem output as I keep putting that in our small
code snippets but realised it wasn't actually in there :).

[bnbarham]

Slight description update

[bnbarham]

Remove RedirectingFileSystem::dump (got re-added in the rebase)

llvm/include/llvm/Support/VirtualFileSystem.h

public:
   ///
   /// /absolute/path   => /absolute/path
   /// relative/../path => <current-directory>/relative/../path
   ///
   /// \param Path A path that is modified to be an absolute path.
   /// \returns success if \a path has been made absolute, otherwise a
   ///          platform-specific error_code.
   virtual std::error_code makeAbsolute(SmallVectorImpl<char> &Path) const;
+
+  enum class PrintType { Summary, Contents, RecursiveContents };
+  void print(raw_ostream &OS, PrintType Type = PrintType::Contents,
+             unsigned IndentLevel = 0) const {
+    printImpl(OS, Type, IndentLevel);
+  }
+
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+  LLVM_DUMP_METHOD void dump() const;
+#endif
+
+protected:
+  virtual void printImpl(raw_ostream &OS, PrintType Type,
+                         unsigned IndentLevel) const {
+    printIndent(OS, IndentLevel);
+    OS << "FileSystem\n";
+  }
+
+  void printIndent(raw_ostream &OS, unsigned IndentLevel) const {
+    for (unsigned i = 0; i < IndentLevel; ++i)
+      OS << "  ";
+  }
 };
 
 /// Gets an \p vfs::FileSystem for the 'real' file system, as seen by
 /// the operating system.
 /// The working directory is linked to the process's working directory.
 /// (This is usually thread-hostile).
 IntrusiveRefCntPtr<FileSystem> getRealFileSystem();
 
public:
   std::error_code isLocal(const Twine &Path, bool &Result) override;
   std::error_code getRealPath(const Twine &Path,
                               SmallVectorImpl<char> &Output) const override;
 
   using iterator = FileSystemList::reverse_iterator;
   using const_iterator = FileSystemList::const_reverse_iterator;
   using reverse_iterator = FileSystemList::iterator;
   using const_reverse_iterator = FileSystemList::const_iterator;
+  using range = iterator_range<iterator>;
+  using const_range = iterator_range<const_iterator>;
 
   /// Get an iterator pointing to the most recently added file system.
   iterator overlays_begin() { return FSList.rbegin(); }
   const_iterator overlays_begin() const { return FSList.rbegin(); }
 
   /// Get an iterator pointing one-past the least recently added file system.
   iterator overlays_end() { return FSList.rend(); }
   const_iterator overlays_end() const { return FSList.rend(); }
 
   /// Get an iterator pointing to the least recently added file system.
   reverse_iterator overlays_rbegin() { return FSList.begin(); }
   const_reverse_iterator overlays_rbegin() const { return FSList.begin(); }
 
   /// Get an iterator pointing one-past the most recently added file system.
   reverse_iterator overlays_rend() { return FSList.end(); }
   const_reverse_iterator overlays_rend() const { return FSList.end(); }
+
+  range overlays_range() { return llvm::reverse(FSList); }
+  const_range overlays_range() const { return llvm::reverse(FSList); }
+
+protected:
+  void printImpl(raw_ostream &OS, PrintType Type,
+                 unsigned IndentLevel) const override;
 };
 
 /// By default, this delegates all calls to the underlying file system. This
 /// is useful when derived file systems want to override some calls and still
 /// proxy other calls.
 class ProxyFileSystem : public FileSystem {
 public:
   explicit ProxyFileSystem(IntrusiveRefCntPtr<FileSystem> FS)
public:
   /// working directory is not set, this returns errc::operation_not_permitted.
   ///
   /// This doesn't resolve symlinks as they are not supported in in-memory file
   /// system.
   std::error_code getRealPath(const Twine &Path,
                               SmallVectorImpl<char> &Output) const override;
   std::error_code isLocal(const Twine &Path, bool &Result) override;
   std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
+
+protected:
+  void printImpl(raw_ostream &OS, PrintType Type,
+                 unsigned IndentLevel) const override;
 };
 
 /// Get a globally unique ID for a virtual file or directory.
 llvm::sys::fs::UniqueID getNextVirtualUniqueID();
 
 /// Gets a \p FileSystem for a virtual file system described in YAML
 /// format.
 std::unique_ptr<FileSystem>
public:
 
   /// Sets the redirection kind to \c Fallthrough if true or \c RedirectOnly
   /// otherwise. Will removed in the future, use \c setRedirection instead.
   void setFallthrough(bool Fallthrough);
 
   void setRedirection(RedirectingFileSystem::RedirectKind Kind);
 
   std::vector<llvm::StringRef> getRoots() const;
 
-  void print(raw_ostream &OS) const;
-  void printEntry(raw_ostream &OS, Entry *E, int NumSpaces = 0) const;
+  void printEntry(raw_ostream &OS, Entry *E, unsigned IndentLevel = 0) const;
 
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
-  LLVM_DUMP_METHOD void dump() const;
-#endif
+protected:
+  void printImpl(raw_ostream &OS, PrintType Type,
+                 unsigned IndentLevel) const override;
 };
 
 /// Collect all pairs of <virtual path, real path> entries from the
 /// \p YAMLFilePath. This is used by the module dependency collector to forward
 /// the entries into the reproducer output VFS YAML file.
 void collectVFSFromYAML(
     std::unique_ptr<llvm::MemoryBuffer> Buffer,
     llvm::SourceMgr::DiagHandlerTy DiagHandler, StringRef YAMLFilePath,

llvm/lib/Support/VirtualFileSystem.cpp

std::error_code FileSystem::isLocal(const Twine &Path, bool &Result) {
   return errc::operation_not_permitted;
 }
 
 bool FileSystem::exists(const Twine &Path) {
   auto Status = status(Path);
   return Status && Status->exists();
 }
 
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+void FileSystem::dump() const { print(dbgs(), PrintType::RecursiveContents); }
+#endif
+
 #ifndef NDEBUG
 static bool isTraversalComponent(StringRef Component) {
   return Component.equals("..") || Component.equals(".");
 }
 
 static bool pathHasTraversal(StringRef Path) {
   using namespace llvm::sys;
 
public:
   directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;
 
   llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override;
   std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
   std::error_code isLocal(const Twine &Path, bool &Result) override;
   std::error_code getRealPath(const Twine &Path,
                               SmallVectorImpl<char> &Output) const override;
 
+protected:
+  void printImpl(raw_ostream &OS, PrintType Type,
+                 unsigned IndentLevel) const override;
+
 private:
   // If this FS has its own working dir, use it to make Path absolute.
   // The returned twine is safe to use as long as both Storage and Path live.
   Twine adjustPath(const Twine &Path, SmallVectorImpl<char> &Storage) const {
     if (!WD)
       return Path;
     Path.toVector(Storage);
     sys::fs::make_absolute(WD->Resolved, Storage);

 
 std::error_code
 RealFileSystem::getRealPath(const Twine &Path,
                             SmallVectorImpl<char> &Output) const {
   SmallString<256> Storage;
   return llvm::sys::fs::real_path(adjustPath(Path, Storage), Output);
 }
 
+void RealFileSystem::printImpl(raw_ostream &OS, PrintType Type,
+                               unsigned IndentLevel) const {
+  printIndent(OS, IndentLevel);
+  OS << "RealFileSystem using ";
+  if (WD)
+    OS << "own";
+  else
+    OS << "process";
+  OS << " CWD\n";
+}
+
 IntrusiveRefCntPtr<FileSystem> vfs::getRealFileSystem() {
   static IntrusiveRefCntPtr<FileSystem> FS(new RealFileSystem(true));
   return FS;
 }
 
 std::unique_ptr<FileSystem> vfs::createPhysicalFileSystem() {
   return std::make_unique<RealFileSystem>(false);
 }

 OverlayFileSystem::getRealPath(const Twine &Path,
                                SmallVectorImpl<char> &Output) const {
   for (const auto &FS : FSList)
     if (FS->exists(Path))
       return FS->getRealPath(Path, Output);
   return errc::no_such_file_or_directory;
 }
 
+void OverlayFileSystem::printImpl(raw_ostream &OS, PrintType Type,
+                                  unsigned IndentLevel) const {
+  printIndent(OS, IndentLevel);
+  OS << "OverlayFileSystem\n";
+  if (Type == PrintType::Summary)
+    return;
+
+  if (Type == PrintType::Contents)
+    Type = PrintType::Summary;
+  for (auto FS : overlays_range())
+    FS->print(OS, Type, IndentLevel + 1);
+}
+
 llvm::vfs::detail::DirIterImpl::~DirIterImpl() = default;
 
 namespace {
 
 /// Combines and deduplicates directory entries across multiple file systems.
 class CombiningDirIterImpl : public llvm::vfs::detail::DirIterImpl {
   using FileSystemPtr = llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>;
 
InMemoryFileSystem::getRealPath(const Twine &Path,
   return {};
 }
 
 std::error_code InMemoryFileSystem::isLocal(const Twine &Path, bool &Result) {
   Result = false;
   return {};
 }
 
+void InMemoryFileSystem::printImpl(raw_ostream &OS, PrintType PrintContents,
+                                   unsigned IndentLevel) const {
+  printIndent(OS, IndentLevel);
+  OS << "InMemoryFileSystem\n";
+}
+
 } // namespace vfs
 } // namespace llvm
 
 //===-----------------------------------------------------------------------===/
 // RedirectingFileSystem implementation
 //===-----------------------------------------------------------------------===/
 
 namespace {

 
 std::vector<StringRef> RedirectingFileSystem::getRoots() const {
   std::vector<StringRef> R;
   for (const auto &Root : Roots)
     R.push_back(Root->getName());
   return R;
 }
 
-void RedirectingFileSystem::print(raw_ostream &OS) const {
+void RedirectingFileSystem::printImpl(raw_ostream &OS, PrintType Type,
+                                      unsigned IndentLevel) const {
+  printIndent(OS, IndentLevel);
+  OS << "RedirectingFileSystem (UseExternalNames: "
+     << (UseExternalNames ? "true" : "false") << ")\n";
+  if (Type == PrintType::Summary)
+    return;
+
   for (const auto &Root : Roots)
-    printEntry(OS, Root.get());
+    printEntry(OS, Root.get(), IndentLevel);
+
+  printIndent(OS, IndentLevel);
+  OS << "ExternalFS:\n";
+  ExternalFS->print(OS, Type == PrintType::Contents ? PrintType::Summary : Type,
+                    IndentLevel + 1);
 }
 
 void RedirectingFileSystem::printEntry(raw_ostream &OS,
                                        RedirectingFileSystem::Entry *E,
-                                       int NumSpaces) const {
-  StringRef Name = E->getName();
-  for (int i = 0, e = NumSpaces; i < e; ++i)
-    OS << " ";
-  OS << "'" << Name.str().c_str() << "'"
-     << "\n";
-
-  if (E->getKind() == RedirectingFileSystem::EK_Directory) {
-    auto *DE = dyn_cast<RedirectingFileSystem::DirectoryEntry>(E);
-    assert(DE && "Should be a directory");
+                                       unsigned IndentLevel) const {
+  printIndent(OS, IndentLevel);
+  OS << "'" << E->getName() << "'";
+
+  switch (E->getKind()) {
+  case EK_Directory: {
+    auto *DE = cast<RedirectingFileSystem::DirectoryEntry>(E);
 
+    OS << "\n";
     for (std::unique_ptr<Entry> &SubEntry :
          llvm::make_range(DE->contents_begin(), DE->contents_end()))
-      printEntry(OS, SubEntry.get(), NumSpaces + 2);
+      printEntry(OS, SubEntry.get(), IndentLevel + 1);
+    break;
+  }
+  case EK_DirectoryRemap:
+  case EK_File: {
+    auto *RE = cast<RedirectingFileSystem::RemapEntry>(E);
+    OS << " -> '" << RE->getExternalContentsPath() << "'";
+    switch (RE->getUseName()) {
+    case NK_NotSet:
+      break;
+    case NK_External:
+      OS << " (UseExternalName: true)";
+      break;
+    case NK_Virtual:
+      OS << " (UseExternalName: false)";
+      break;
+    }
+    OS << "\n";
+    break;
+  }
   }
 }
-
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
-void RedirectingFileSystem::dump() const { print(dbgs()); }
-#endif
 
 /// A helper class to hold the common YAML parsing state.
 class llvm::vfs::RedirectingFileSystemParser {
   yaml::Stream &Stream;
 
   void error(yaml::Node *N, const Twine &Msg) { Stream.printError(N, Msg); }
 
   // false on error

llvm/unittests/Support/VirtualFileSystemTest.cpp

public:
   }
 
   void addSymlink(StringRef Path) {
     vfs::Status S(Path, UniqueID(FSID, FileID++),
                   std::chrono::system_clock::now(), 0, 0, 0,
                   sys::fs::file_type::symlink_file, sys::fs::all_all);
     addEntry(Path, S);
   }
+
+protected:
+  void printImpl(raw_ostream &OS, PrintType Type,
+                 unsigned IndentLevel) const override {
+    printIndent(OS, IndentLevel);
+    OS << "DummyFileSystem (";
+    switch (Type) {
+    case vfs::FileSystem::PrintType::Summary:
+      OS << "Summary";
+      break;
+    case vfs::FileSystem::PrintType::Contents:
+      OS << "Contents";
+      break;
+    case vfs::FileSystem::PrintType::RecursiveContents:
+      OS << "RecursiveContents";
+      break;
+    }
+    OS << ")\n";
+  }
 };
 
 class ErrorDummyFileSystem : public DummyFileSystem {
   std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
     return llvm::errc::no_such_file_or_directory;
   }
 };
 
// Make sure we get the top-most entry
     for (; !EC && I != E; I.increment(EC))
       if (I->path() == "/hiddenByMid")
         break;
     ASSERT_NE(E, I);
     EXPECT_EQ(sys::fs::file_type::regular_file, I->type());
   }
 }
 
+TEST(OverlayFileSystemTest, PrintOutput) {
+  auto Dummy = makeIntrusiveRefCnt<DummyFileSystem>();
+  auto Overlay1 = makeIntrusiveRefCnt<vfs::OverlayFileSystem>(Dummy);
+  Overlay1->pushOverlay(Dummy);
+  auto Overlay2 = makeIntrusiveRefCnt<vfs::OverlayFileSystem>(Overlay1);
+  Overlay2->pushOverlay(Dummy);
+
+  SmallString<0> Output;
+  raw_svector_ostream OuputStream{Output};
+
+  Overlay2->print(OuputStream, vfs::FileSystem::PrintType::Summary);
+  ASSERT_EQ("OverlayFileSystem\n", Output);
+
+  Output.clear();
+  Overlay2->print(OuputStream, vfs::FileSystem::PrintType::Contents);
+  ASSERT_EQ("OverlayFileSystem\n"
+            "  DummyFileSystem (Summary)\n"
+            "  OverlayFileSystem\n",
+            Output);
+
+  Output.clear();
+  Overlay2->print(OuputStream, vfs::FileSystem::PrintType::RecursiveContents);
+  ASSERT_EQ("OverlayFileSystem\n"
+            "  DummyFileSystem (RecursiveContents)\n"
+            "  OverlayFileSystem\n"
+            "    DummyFileSystem (RecursiveContents)\n"
+            "    DummyFileSystem (RecursiveContents)\n",
+            Output);
+}
+
 TEST(ProxyFileSystemTest, Basic) {
   IntrusiveRefCntPtr<vfs::InMemoryFileSystem> Base(
       new vfs::InMemoryFileSystem());
   vfs::ProxyFileSystem PFS(Base);
 
   Base->addFile("/a", 0, MemoryBuffer::getMemBuffer("test"));
 
   auto Stat = PFS.status("/a");
TEST(VFSFromRemappedFilesTest, LastMappingWins) {
 
   auto BufferKeepA = RemappedFSKeepName->getBufferForFile("//root/a");
   auto BufferExternalA = RemappedFSExternalName->getBufferForFile("//root/a");
   ASSERT_TRUE(BufferKeepA);
   ASSERT_TRUE(BufferExternalA);
   EXPECT_EQ("contents of c", (*BufferKeepA)->getBuffer());
   EXPECT_EQ("contents of c", (*BufferExternalA)->getBuffer());
 }
+
+TEST(RedirectingFileSystemTest, PrintOutput) {
+  auto Buffer =
+      MemoryBuffer::getMemBuffer("{\n"
+                                 "  'version': 0,\n"
+                                 "  'roots': [\n"
+                                 "    {\n"
+                                 "      'type': 'directory-remap',\n"
+                                 "      'name': '/dremap',\n"
+                                 "      'external-contents': '/a',\n"
+                                 "    },"
+                                 "    {\n"
+                                 "      'type': 'directory',\n"
+                                 "      'name': '/vdir',\n"
+                                 "      'contents': ["
+                                 "        {\n"
+                                 "          'type': 'directory-remap',\n"
+                                 "          'name': 'dremap',\n"
+                                 "          'external-contents': '/b'\n"
+                                 "          'use-external-name': 'true'\n"
+                                 "        },\n"
+                                 "        {\n"
+                                 "          'type': 'file',\n"
+                                 "          'name': 'vfile',\n"
+                                 "          'external-contents': '/c'\n"
+                                 "          'use-external-name': 'false'\n"
+                                 "        }]\n"
+                                 "    }]\n"
+                                 "}");
+
+  auto Dummy = makeIntrusiveRefCnt<DummyFileSystem>();
+  auto Redirecting = vfs::RedirectingFileSystem::create(
+      std::move(Buffer), nullptr, "", nullptr, Dummy);
+
+  SmallString<0> Output;
+  raw_svector_ostream OuputStream{Output};
+
+  Redirecting->print(OuputStream, vfs::FileSystem::PrintType::Summary);
+  ASSERT_EQ("RedirectingFileSystem (UseExternalNames: true)\n", Output);
+
+  Output.clear();
+  Redirecting->print(OuputStream, vfs::FileSystem::PrintType::Contents);
+  ASSERT_EQ("RedirectingFileSystem (UseExternalNames: true)\n"
+            "'/'\n"
+            "  'dremap' -> '/a'\n"
+            "  'vdir'\n"
+            "    'dremap' -> '/b' (UseExternalName: true)\n"
+            "    'vfile' -> '/c' (UseExternalName: false)\n"
+            "ExternalFS:\n"
+            "  DummyFileSystem (Summary)\n",
+            Output);
+
+  Output.clear();
+  Redirecting->print(OuputStream, vfs::FileSystem::PrintType::Contents, 1);
+  ASSERT_EQ("  RedirectingFileSystem (UseExternalNames: true)\n"
+            "  '/'\n"
+            "    'dremap' -> '/a'\n"
+            "    'vdir'\n"
+            "      'dremap' -> '/b' (UseExternalName: true)\n"
+            "      'vfile' -> '/c' (UseExternalName: false)\n"
+            "  ExternalFS:\n"
+            "    DummyFileSystem (Summary)\n",
+            Output);
+
+  Output.clear();
+  Redirecting->print(OuputStream,
+                     vfs::FileSystem::PrintType::RecursiveContents);
+  ASSERT_EQ("RedirectingFileSystem (UseExternalNames: true)\n"
+            "'/'\n"
+            "  'dremap' -> '/a'\n"
+            "  'vdir'\n"
+            "    'dremap' -> '/b' (UseExternalName: true)\n"
+            "    'vfile' -> '/c' (UseExternalName: false)\n"
+            "ExternalFS:\n"
+            "  DummyFileSystem (RecursiveContents)\n",
+            Output);
+}