[PATCH] D53465: [VFS] Add support for "no_push" to VFS recursive iterators.

[Volodymyr Sapsai via Phabricator via llvm-commits]

vsapsai added a comment.

I am gravitating towards having a separate unit test(s) for `no_push` functionality. Maybe I'm wrong but I expect it to be smaller and easier to understand, though there'll be some boilerplate.

Implementation looks reasonable but I'm not sure about a few edge cases:

- call `no_push` before starting iteration;
- call `no_push` after we finished iteration (here I mean when we are pointing to the last directory, before the iterator becomes end iterator);
- call `no_push` for a directory that has no children and we wouldn't descend into anyway.

I think in these cases VFS iterator should behave as `llvm::sys::fs::recursive_directory_iterator` unless it is doing something strange. Not sure all these cases deserve a test, depends on the fact if they execute different paths in implementation.



================
Comment at: llvm/unittests/Support/VirtualFileSystemTest.cpp:484-486
+  EXPECT_EQ(1, Counts[4]); // e
+  EXPECT_EQ(0, Counts[5]); // e
+  EXPECT_EQ(1, Counts[6]); // g
----------------
Is `Counts[5]` supposed to be `// f`?


Repository:
  rL LLVM

https://reviews.llvm.org/D53465