[PATCH] D79809: [YAMLVFSWriter] Fix for delimiters

[Jan Korous via Phabricator via llvm-commits]

jkorous marked 3 inline comments as done.
jkorous added inline comments.


================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:2043
 
-    if (!first_entry_is_directory)
+    bool IsCurrentDirEmpty = true;
+    if (!Entry.IsDirectory) {
----------------
JDevlieghere wrote:
> So this is set to true whenever we start a new directory, and is set to false whenever we write a file entry. Would it make sense to make this variable a member and toggle it inside `startDirectory ` and `writeEntry` respectively?
Maybe... I'd assume that if we just reset it at the start of `write()` method it should work.

That said - I kind of prefer to keep the whole lifecycle of the variable visible in this method as it's imo easier to understand than having to check two other methods (no matter how trivial).
Also, although I wouldn't expect any complications by doing that (can't imagine any potential misuse of the class interface or possibility to corrupt instance state) but I'd still prefer to land this patch as it is and address this maybe later.


================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:2057
       } else {
+        bool HaveWePoppedDirStack = false;
         while (!DirStack.empty() && !containedIn(DirStack.back(), Dir)) {
----------------
JDevlieghere wrote:
> Not a fan of this name. What about `IsDirPoppedFromStack`?
Sounds good


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

https://reviews.llvm.org/D79809