[PATCH] D79809: [YAMLVFSWriter] Speculative fix for delimiters

[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.
jkorous marked an inline comment as done.
jkorous added inline comments.


================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:2061
         }
         OS << ",\n";
         startDirectory(Dir);
----------------
I feel this delimiter should also be conditional on `IsCurrentDirEmpty` but need to think about it a bit more.

I'll add a testcase for entries like this:
```
/a/
/a/b/
```


https://reviews.llvm.org/D79809

Files:
  llvm/lib/Support/VirtualFileSystem.cpp


Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -2027,10 +2027,10 @@
 
   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);
+
+    startDirectory(
+      Entry.IsDirectory ? Entry.VPath : path::parent_path(Entry.VPath)
+    );
 
     StringRef RPath = Entry.RPath;
     if (UseOverlayRelative) {
@@ -2040,16 +2040,18 @@
       RPath = RPath.slice(OverlayDirLen, RPath.size());
     }
 
-    if (!first_entry_is_directory)
+    bool IsCurrentDirEmpty = true;
+    if (!Entry.IsDirectory) {
       writeEntry(path::filename(Entry.VPath), RPath);
+      IsCurrentDirEmpty = false;
+    }
 
     for (const auto &Entry : Entries.slice(1)) {
       StringRef Dir =
           Entry.IsDirectory ? Entry.VPath : path::parent_path(Entry.VPath);
       if (Dir == DirStack.back()) {
-        if (!first_entry_is_directory) {
+        if (!IsCurrentDirEmpty) {
           OS << ",\n";
-          first_entry_is_directory = false;
         }
       } else {
         while (!DirStack.empty() && !containedIn(DirStack.back(), Dir)) {
@@ -2058,6 +2060,7 @@
         }
         OS << ",\n";
         startDirectory(Dir);
+        IsCurrentDirEmpty = true;
       }
       StringRef RPath = Entry.RPath;
       if (UseOverlayRelative) {
@@ -2066,8 +2069,10 @@
                "Overlay dir must be contained in RPath");
         RPath = RPath.slice(OverlayDirLen, RPath.size());
       }
-      if (!Entry.IsDirectory)
+      if (!Entry.IsDirectory) {
         writeEntry(path::filename(Entry.VPath), RPath);
+        IsCurrentDirEmpty = false;
+      }
     }
 
     while (!DirStack.empty()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79809.263523.patch
Type: text/x-patch
Size: 1944 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200512/910386b8/attachment-0001.bin>