[VFS] Fix root directory handling in YAMLVFSWriter

[jkorous]

Summary

[VFS] Fix root directory handling in YAMLVFSWriter

• refactor JSONWriter::write
• handle directories uniformly - don't skip writeEntry only for the first Entry
• handle entries without a parent directory
• assert every entry have either a parent dir or is root directory

The testcase previously vioated assert in JSONWriter::containedPart about a previous directory element path being non-empty.

[jkorous]

llvm/lib/Support/VirtualFileSystem.cpp

2043

Not happy with the name - open to suggestions.

2060

Hmm, maybe I should just move the assert to GetParentOrIfRootDir so it's not duplicated.

llvm/lib/Support/VirtualFileSystem.cpp

void JSONWriter::write(ArrayRef<YAMLVFSEntry> Entries,
   if (IsOverlayRelative.hasValue()) {
     UseOverlayRelative = IsOverlayRelative.getValue();
     OS << "  'overlay-relative': '" << (UseOverlayRelative ? "true" : "false")
        << "',\n";
   }
   OS << "  'roots': [\n";
 
   if (!Entries.empty()) {
-    const YAMLVFSEntry &Entry = Entries.front();
-    bool first_entry_is_directory = Entry.IsDirectory;
-    StringRef Dir =
-        first_entry_is_directory ? Entry.VPath : path::parent_path(Entry.VPath);
-    startDirectory(Dir);
-
+    auto HandleEntry = [this, UseOverlayRelative,
+                        &OverlayDir](const YAMLVFSEntry &Entry) {
       StringRef RPath = Entry.RPath;
       if (UseOverlayRelative) {
         unsigned OverlayDirLen = OverlayDir.size();
         assert(RPath.substr(0, OverlayDirLen) == OverlayDir &&
                "Overlay dir must be contained in RPath");
         RPath = RPath.slice(OverlayDirLen, RPath.size());
       }
 
-    if (!first_entry_is_directory)
+      if (!Entry.IsDirectory)
         writeEntry(path::filename(Entry.VPath), RPath);
+    };
+
+    auto GetParentOrIfRootDir = [](StringRef Path) -> StringRef {
+      auto MaybeResult = path::parent_path(Path);
+      if (!MaybeResult.empty())
+        return MaybeResult;
+
+      MaybeResult = path::root_path(Path);
+      if (MaybeResult == Path)
+        return MaybeResult;
+
+      return "";
+    };
+
+    bool CurrentDirElementHasNoItems = true;
 
+    { // Handle the first element.
+      const auto &Entry = Entries.front();
+      auto NewDir = GetParentOrIfRootDir(Entry.VPath);
+      assert(!NewDir.empty() && "VPath doesn't have parent directory and is "
+                                "not a root directory either.");
+      startDirectory(NewDir);
+      HandleEntry(Entry);
+      CurrentDirElementHasNoItems = Entry.IsDirectory;
+    }
+
+    // Handle the rest of the elements with delimiters and indentation.
     for (const auto &Entry : Entries.slice(1)) {
-      StringRef Dir =
-          Entry.IsDirectory ? Entry.VPath : path::parent_path(Entry.VPath);
+      auto Dir = GetParentOrIfRootDir(Entry.VPath);
+      assert(!Dir.empty() && "VPath doesn't have parent directory and is not a "
+                             "root directory either.");
+
       if (Dir == DirStack.back()) {
-        if (!first_entry_is_directory) {
+        // Add delimiter only if there's an item in the current dir already.
+        if (!CurrentDirElementHasNoItems)
           OS << ",\n";
-          first_entry_is_directory = false;
-        }
       } else {
         while (!DirStack.empty() && !containedIn(DirStack.back(), Dir)) {
           OS << "\n";
           endDirectory();
         }
         OS << ",\n";
         startDirectory(Dir);
+        CurrentDirElementHasNoItems = Entry.IsDirectory;
       }
-      StringRef RPath = Entry.RPath;
-      if (UseOverlayRelative) {
-        unsigned OverlayDirLen = OverlayDir.size();
-        assert(RPath.substr(0, OverlayDirLen) == OverlayDir &&
-               "Overlay dir must be contained in RPath");
-        RPath = RPath.slice(OverlayDirLen, RPath.size());
-      }
-      writeEntry(path::filename(Entry.VPath), RPath);
+
+      HandleEntry(Entry);
     }
 
     while (!DirStack.empty()) {
       OS << "\n";
       endDirectory();
     }
     OS << "\n";
   }

llvm/unittests/Support/VirtualFileSystemTest.cpp

 
 TEST_F(VFSFromYAMLTest, YAMLVFSWriterTest) {
   ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
   ScopedDir _a(TestDirectory + "/a");
   ScopedFile _ab(TestDirectory + "/a/b", "");
   ScopedDir _c(TestDirectory + "/c");
   ScopedFile _cd(TestDirectory + "/c/d", "");
   ScopedDir _e(TestDirectory + "/e");
-  ScopedDir _ef(TestDirectory + "/e/f");
+  ScopedFile _ef(TestDirectory + "/e/f", "");
   ScopedDir _g(TestDirectory + "/g");
   ScopedFile _h(TestDirectory + "/h", "");
 
   vfs::YAMLVFSWriter VFSWriter;
   VFSWriter.addDirectoryMapping(_a.Path, "//root/a");
   VFSWriter.addFileMapping(_ab.Path, "//root/a/b");
   VFSWriter.addFileMapping(_cd.Path, "//root/c/d");
   VFSWriter.addDirectoryMapping(_e.Path, "//root/e");
-  VFSWriter.addDirectoryMapping(_ef.Path, "//root/e/f");
+  VFSWriter.addFileMapping(_ef.Path, "//root/e/f");
   VFSWriter.addFileMapping(_g.Path, "//root/g");
-  VFSWriter.addDirectoryMapping(_h.Path, "//root/h");
+  VFSWriter.addFileMapping(_h.Path, "//root/h");
 
   std::string Buffer;
   raw_string_ostream OS(Buffer);
   VFSWriter.write(OS);
   OS.flush();
 
   IntrusiveRefCntPtr<ErrorDummyFileSystem> Lower(new ErrorDummyFileSystem());
   Lower->addDirectory("//root/");
TEST_F(VFSFromYAMLTest, YAMLVFSWriterTest) {
   EXPECT_TRUE(FS->exists(_ab.Path));
   EXPECT_TRUE(FS->exists(_c.Path));
   EXPECT_TRUE(FS->exists(_cd.Path));
   EXPECT_TRUE(FS->exists(_e.Path));
   EXPECT_TRUE(FS->exists(_ef.Path));
   EXPECT_TRUE(FS->exists(_g.Path));
   EXPECT_TRUE(FS->exists(_h.Path));
 }
+
+TEST(YAMLVFSWriterTest, HandleRootAsVPath) {
+  llvm::vfs::YAMLVFSWriter W;
+  W.addFileMapping("/", "/tmp");
+  W.addFileMapping("/foo", "/tmp/bar");
+  std::string Dump;
+  llvm::raw_string_ostream ToBeIgnored(Dump);
+
+  EXPECT_NO_FATAL_FAILURE(W.write(ToBeIgnored));
+}