[VFS] Combine VFSFromYamlDirIterImpl and OverlayFSDirIterImpl into a single implementation (NFC)

[nathawes]

Summary

As a fixme notes, both of these directory iterator implementations are conceptually similar and duplicate the functionality of returning and uniquing entries across two or more directories. This patch addresses the fixme by extracting their common functionality into a single class 'CombiningDirIterImpl'.

This also drops the 'Redirecting' prefix from RedirectingDirEntry and RedirectingFileEntry to save horizontal space. There's no loss of clarity as they already have to be prefixed with 'RedirectingFileSystem::' whenever they're referenced anyway.

rdar://problem/72485443

[dexonsmith]

LGTM! Thanks for splitting it out.

[JDevlieghere]

llvm/include/llvm/Support/VirtualFileSystem.h

661

Nit: most of the implementation uses EC as the variable name for an error_code.

[nathawes]

• Rename error_code to EC in shouldFallBackToExternalFS() to match existing conventions in the file.
• Update a missed RedirectingFileEntry reference in lldb to its new name (FileEntry)

llvm/include/llvm/Support/VirtualFileSystem.h

struct YAMLVFSEntry {
   YAMLVFSEntry(T1 &&VPath, T2 &&RPath, bool IsDirectory = false)
       : VPath(std::forward<T1>(VPath)), RPath(std::forward<T2>(RPath)),
         IsDirectory(IsDirectory) {}
   std::string VPath;
   std::string RPath;
   bool IsDirectory = false;
 };
 
-class VFSFromYamlDirIterImpl;
+class RedirectingFSDirIterImpl;
 class RedirectingFileSystemParser;
 
 /// A virtual file system parsed from a YAML file.
 ///
 /// Currently, this class allows creating virtual directories and mapping
 /// virtual file paths to existing external files, available in \c ExternalFS.
 ///
 /// The basic structure of the parsed file is:
public:
   public:
     Entry(EntryKind K, StringRef Name) : Kind(K), Name(Name) {}
     virtual ~Entry() = default;
 
     StringRef getName() const { return Name; }
     EntryKind getKind() const { return Kind; }
   };
 
-  class RedirectingDirectoryEntry : public Entry {
+  class DirectoryEntry : public Entry {
     std::vector<std::unique_ptr<Entry>> Contents;
     Status S;
 
   public:
-    RedirectingDirectoryEntry(StringRef Name,
-                              std::vector<std::unique_ptr<Entry>> Contents,
-                              Status S)
+    /// Constructs a directory entry with explicitly specified contents.
+    DirectoryEntry(StringRef Name, std::vector<std::unique_ptr<Entry>> Contents,
         : Entry(EK_Directory, Name), Contents(std::move(Contents)),
           S(std::move(S)) {}
-    RedirectingDirectoryEntry(StringRef Name, Status S)
+
+    /// Constructs an empty directory entry.
+    DirectoryEntry(StringRef Name, Status S)
         : Entry(EK_Directory, Name), S(std::move(S)) {}
 
     Status getStatus() { return S; }
 
     void addContent(std::unique_ptr<Entry> Content) {
       Contents.push_back(std::move(Content));
     }
 
     Entry *getLastContent() const { return Contents.back().get(); }
 
     using iterator = decltype(Contents)::iterator;
 
     iterator contents_begin() { return Contents.begin(); }
     iterator contents_end() { return Contents.end(); }
 
     static bool classof(const Entry *E) { return E->getKind() == EK_Directory; }
   };
 
-  class RedirectingFileEntry : public Entry {
+  class FileEntry : public Entry {
   public:
     enum NameKind { NK_NotSet, NK_External, NK_Virtual };
 
   private:
     std::string ExternalContentsPath;
     NameKind UseName;
 
   public:
-    RedirectingFileEntry(StringRef Name, StringRef ExternalContentsPath,
-                         NameKind UseName)
+    FileEntry(StringRef Name, StringRef ExternalContentsPath, NameKind UseName)
         : Entry(EK_File, Name), ExternalContentsPath(ExternalContentsPath),
           UseName(UseName) {}
 
     StringRef getExternalContentsPath() const { return ExternalContentsPath; }
 
     /// whether to use the external path as the name for this file.
     bool useExternalName(bool GlobalUseExternalName) const {
       return UseName == NK_NotSet ? GlobalUseExternalName
                                   : (UseName == NK_External);
     }
 
     NameKind getUseName() const { return UseName; }
 
     static bool classof(const Entry *E) { return E->getKind() == EK_File; }
   };
 
 private:
-  friend class VFSFromYamlDirIterImpl;
+  friend class RedirectingFSDirIterImpl;
   friend class RedirectingFileSystemParser;
 
   bool shouldUseExternalFS() const {
     return ExternalFSValidWD && IsFallthrough;
   }
 
+  /// Whether to fall back to the external file system when an operation fails
+  /// with the given error code.
+  bool shouldFallBackToExternalFS(std::error_code Error) const;
+
   // In a RedirectingFileSystem, keys can be specified in Posix or Windows
   // style (or even a mixture of both), so this comparison helper allows
   // slashes (representing a root) to match backslashes (and vice versa).  Note
   // that, other than the root, path components should not contain slashes or
   // backslashes.
   bool pathComponentMatches(llvm::StringRef lhs, llvm::StringRef rhs) const {
     if ((CaseSensitive ? lhs.equals(rhs) : lhs.equals_lower(rhs)))
       return true;

llvm/lib/Support/VirtualFileSystem.cpp

if (FS->exists(Path))
       return FS->getRealPath(Path, Output);
   return errc::no_such_file_or_directory;
 }
 
 llvm::vfs::detail::DirIterImpl::~DirIterImpl() = default;
 
 namespace {
 
-class OverlayFSDirIterImpl : public llvm::vfs::detail::DirIterImpl {
-  OverlayFileSystem &Overlays;
-  std::string Path;
-  OverlayFileSystem::iterator CurrentFS;
+/// Combines and deduplicates directory entries across multiple file systems.
+class CombiningDirIterImpl : public llvm::vfs::detail::DirIterImpl {
+  using FileSystemPtr = llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>;
+
+  /// File systems to check for entries in. Processed in reverse order.
+  SmallVector<FileSystemPtr, 8> FSList;
+  /// The directory iterator for the current filesystem.
   directory_iterator CurrentDirIter;
+  /// The path of the directory to iterate the entries of.
+  std::string DirPath;
+  /// The set of names already returned as entries.
   llvm::StringSet<> SeenNames;
 
+  /// Sets \c CurrentDirIter to an iterator of \c DirPath in the next file
+  /// system in the list, or leaves it as is (at its end position) if we've
+  /// already gone through them all.
   std::error_code incrementFS() {
-    assert(CurrentFS != Overlays.overlays_end() && "incrementing past end");
-    ++CurrentFS;
-    for (auto E = Overlays.overlays_end(); CurrentFS != E; ++CurrentFS) {
+    while (!FSList.empty()) {
       std::error_code EC;
-      CurrentDirIter = (*CurrentFS)->dir_begin(Path, EC);
+      CurrentDirIter = FSList.back()->dir_begin(DirPath, EC);
+      FSList.pop_back();
       if (EC && EC != errc::no_such_file_or_directory)
         return EC;
       if (CurrentDirIter != directory_iterator())
         break; // found
     }
     return {};
   }
 
while (true) {
       StringRef Name = llvm::sys::path::filename(CurrentEntry.path());
       if (SeenNames.insert(Name).second)
         return EC; // name not seen before
     }
     llvm_unreachable("returned above");
   }
 
 public:
-  OverlayFSDirIterImpl(const Twine &Path, OverlayFileSystem &FS,
+  CombiningDirIterImpl(ArrayRef<FileSystemPtr> FileSystems, std::string Dir,
                        std::error_code &EC)
-      : Overlays(FS), Path(Path.str()), CurrentFS(Overlays.overlays_begin()) {
-    CurrentDirIter = (*CurrentFS)->dir_begin(Path, EC);
+      : FSList(FileSystems.begin(), FileSystems.end()),
+        DirPath(std::move(Dir)) {
+    if (!FSList.empty()) {
+      CurrentDirIter = FSList.back()->dir_begin(DirPath, EC);
+      FSList.pop_back();
+      if (!EC || EC == errc::no_such_file_or_directory)
+        EC = incrementImpl(true);
+    }
+  }
+
+  CombiningDirIterImpl(directory_iterator FirstIter, FileSystemPtr Fallback,
+                       std::string FallbackDir, std::error_code &EC)
+      : FSList({Fallback}), CurrentDirIter(FirstIter),
+        DirPath(std::move(FallbackDir)) {
+    if (!EC || EC == errc::no_such_file_or_directory)
       EC = incrementImpl(true);
   }
 
   std::error_code increment() override { return incrementImpl(false); }
 };
 
 } // namespace
 
 directory_iterator OverlayFileSystem::dir_begin(const Twine &Dir,
                                                 std::error_code &EC) {
   return directory_iterator(
-      std::make_shared<OverlayFSDirIterImpl>(Dir, *this, EC));
+      std::make_shared<CombiningDirIterImpl>(FSList, Dir.str(), EC));
 }
 
 void ProxyFileSystem::anchor() {}
 
 namespace llvm {
 namespace vfs {
 
 namespace detail {
RedirectingFileSystem::RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> FS)
   if (ExternalFS)
     if (auto ExternalWorkingDirectory =
             ExternalFS->getCurrentWorkingDirectory()) {
       WorkingDirectory = *ExternalWorkingDirectory;
       ExternalFSValidWD = true;
     }
 }
 
-// FIXME: reuse implementation common with OverlayFSDirIterImpl as these
-// iterators are conceptually similar.
-class llvm::vfs::VFSFromYamlDirIterImpl
+/// Directory iterator implementation for \c RedirectingFileSystem's
+/// directory entries.
+class llvm::vfs::RedirectingFSDirIterImpl
     : public llvm::vfs::detail::DirIterImpl {
   std::string Dir;
-  RedirectingFileSystem::RedirectingDirectoryEntry::iterator Current, End;
-
-  // To handle 'fallthrough' mode we need to iterate at first through
-  // RedirectingDirectoryEntry and then through ExternalFS. These operations are
-  // done sequentially, we just need to keep a track of what kind of iteration
-  // we are currently performing.
-
-  /// Flag telling if we should iterate through ExternalFS or stop at the last
-  /// RedirectingDirectoryEntry::iterator.
-  bool IterateExternalFS;
-  /// Flag telling if we have switched to iterating through ExternalFS.
-  bool IsExternalFSCurrent = false;
-  FileSystem &ExternalFS;
-  directory_iterator ExternalDirIter;
-  llvm::StringSet<> SeenNames;
+  RedirectingFileSystem::DirectoryEntry::iterator Current, End;
 
-  /// To combine multiple iterations, different methods are responsible for
-  /// different iteration steps.
-  /// @{
-
-  /// Responsible for dispatching between RedirectingDirectoryEntry iteration
-  /// and ExternalFS iteration.
-  std::error_code incrementImpl(bool IsFirstTime);
-  /// Responsible for RedirectingDirectoryEntry iteration.
-  std::error_code incrementContent(bool IsFirstTime);
-  /// Responsible for ExternalFS iteration.
-  std::error_code incrementExternal();
-  /// @}
+  std::error_code incrementImpl(bool IsFirstTime) {
+    assert((IsFirstTime || Current != End) && "cannot iterate past end");
+    if (!IsFirstTime)
+      ++Current;
+    if (Current != End) {
+      SmallString<128> PathStr(Dir);
+      llvm::sys::path::append(PathStr, (*Current)->getName());
+      sys::fs::file_type Type = sys::fs::file_type::type_unknown;
+      switch ((*Current)->getKind()) {
+      case RedirectingFileSystem::EK_Directory:
+        Type = sys::fs::file_type::directory_file;
+        break;
+      case RedirectingFileSystem::EK_File:
+        Type = sys::fs::file_type::regular_file;
+        break;
+      }
+      CurrentEntry = directory_entry(std::string(PathStr.str()), Type);
+    } else {
+      CurrentEntry = directory_entry();
+    }
+    return {};
+  };
 
 public:
-  VFSFromYamlDirIterImpl(
-      const Twine &Path,
-      RedirectingFileSystem::RedirectingDirectoryEntry::iterator Begin,
-      RedirectingFileSystem::RedirectingDirectoryEntry::iterator End,
-      bool IterateExternalFS, FileSystem &ExternalFS, std::error_code &EC);
+  RedirectingFSDirIterImpl(
+      const Twine &Path, RedirectingFileSystem::DirectoryEntry::iterator Begin,
+      RedirectingFileSystem::DirectoryEntry::iterator End, std::error_code &EC)
+      : Dir(Path.str()), Current(Begin), End(End) {
+    EC = incrementImpl(/*IsFirstTime=*/true);
+  }
 
-  std::error_code increment() override;
+  std::error_code increment() override {
+    return incrementImpl(/*IsFirstTime=*/false);
+  }
 };
 
 llvm::ErrorOr<std::string>
 RedirectingFileSystem::getCurrentWorkingDirectory() const {
   return WorkingDirectory;
 }
 
 std::error_code
std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl<char> &Path) const {
   return {};
 }
 
 directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
                                                     std::error_code &EC) {
   ErrorOr<RedirectingFileSystem::Entry *> E = lookupPath(Dir);
   if (!E) {
     EC = E.getError();
-    if (shouldUseExternalFS() && EC == errc::no_such_file_or_directory)
+    if (shouldFallBackToExternalFS(EC))
       return ExternalFS->dir_begin(Dir, EC);
     return {};
   }
   ErrorOr<Status> S = status(Dir, *E);
   if (!S) {
     EC = S.getError();
     return {};
   }
   if (!S->isDirectory()) {
     EC = std::error_code(static_cast<int>(errc::not_a_directory),
                          std::system_category());
     return {};
   }
 
-  auto *D = cast<RedirectingFileSystem::RedirectingDirectoryEntry>(*E);
-  return directory_iterator(std::make_shared<VFSFromYamlDirIterImpl>(
-      Dir, D->contents_begin(), D->contents_end(),
-      /*IterateExternalFS=*/shouldUseExternalFS(), *ExternalFS, EC));
+  auto *D = cast<RedirectingFileSystem::DirectoryEntry>(*E);
+  auto DirIter = directory_iterator(std::make_shared<RedirectingFSDirIterImpl>(
+      Dir, D->contents_begin(), D->contents_end(), EC));
+
+  if (!shouldUseExternalFS())
+    return DirIter;
+  return directory_iterator(std::make_shared<CombiningDirIterImpl>(
+      DirIter, ExternalFS, Dir.str(), EC));
 }
 
 void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) {
   ExternalContentsPrefixDir = PrefixDir.str();
 }
 
 StringRef RedirectingFileSystem::getExternalContentsPrefixDir() const {
   return ExternalContentsPrefixDir;
void RedirectingFileSystem::dumpEntry(raw_ostream &OS,
                                       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::RedirectingDirectoryEntry>(E);
+    auto *DE = dyn_cast<RedirectingFileSystem::DirectoryEntry>(E);
     assert(DE && "Should be a directory");
 
     for (std::unique_ptr<Entry> &SubEntry :
          llvm::make_range(DE->contents_begin(), DE->contents_end()))
       dumpEntry(OS, SubEntry.get(), NumSpaces + 2);
   }
 }
 
lookupOrCreateEntry(RedirectingFileSystem *FS, StringRef Name,
     if (!ParentEntry) { // Look for a existent root
       for (const auto &Root : FS->Roots) {
         if (Name.equals(Root->getName())) {
           ParentEntry = Root.get();
           return ParentEntry;
         }
       }
     } else { // Advance to the next component
-      auto *DE = dyn_cast<RedirectingFileSystem::RedirectingDirectoryEntry>(
-          ParentEntry);
+      auto *DE = dyn_cast<RedirectingFileSystem::DirectoryEntry>(ParentEntry);
       for (std::unique_ptr<RedirectingFileSystem::Entry> &Content :
            llvm::make_range(DE->contents_begin(), DE->contents_end())) {
         auto *DirContent =
-            dyn_cast<RedirectingFileSystem::RedirectingDirectoryEntry>(
-                Content.get());
+            dyn_cast<RedirectingFileSystem::DirectoryEntry>(Content.get());
         if (DirContent && Name.equals(Content->getName()))
           return DirContent;
       }
     }
 
     // ... or create a new one
     std::unique_ptr<RedirectingFileSystem::Entry> E =
-        std::make_unique<RedirectingFileSystem::RedirectingDirectoryEntry>(
+        std::make_unique<RedirectingFileSystem::DirectoryEntry>(
             Name, Status("", getNextVirtualUniqueID(),
                          std::chrono::system_clock::now(), 0, 0, 0,
                          file_type::directory_file, sys::fs::all_all));
 
     if (!ParentEntry) { // Add a new root to the overlay
       FS->Roots.push_back(std::move(E));
       ParentEntry = FS->Roots.back().get();
       return ParentEntry;
     }
 
-    auto *DE =
-        cast<RedirectingFileSystem::RedirectingDirectoryEntry>(ParentEntry);
+    auto *DE = cast<RedirectingFileSystem::DirectoryEntry>(ParentEntry);
     DE->addContent(std::move(E));
     return DE->getLastContent();
   }
 
 private:
   void uniqueOverlayTree(RedirectingFileSystem *FS,
                          RedirectingFileSystem::Entry *SrcE,
                          RedirectingFileSystem::Entry *NewParentE = nullptr) {
     StringRef Name = SrcE->getName();
     switch (SrcE->getKind()) {
     case RedirectingFileSystem::EK_Directory: {
-      auto *DE = cast<RedirectingFileSystem::RedirectingDirectoryEntry>(SrcE);
+      auto *DE = cast<RedirectingFileSystem::DirectoryEntry>(SrcE);
       // Empty directories could be present in the YAML as a way to
       // describe a file for a current directory after some of its subdir
       // is parsed. This only leads to redundant walks, ignore it.
       if (!Name.empty())
         NewParentE = lookupOrCreateEntry(FS, Name, NewParentE);
       for (std::unique_ptr<RedirectingFileSystem::Entry> &SubEntry :
            llvm::make_range(DE->contents_begin(), DE->contents_end()))
         uniqueOverlayTree(FS, SubEntry.get(), NewParentE);
       break;
     }
     case RedirectingFileSystem::EK_File: {
       assert(NewParentE && "Parent entry must exist");
-      auto *FE = cast<RedirectingFileSystem::RedirectingFileEntry>(SrcE);
-      auto *DE =
-          cast<RedirectingFileSystem::RedirectingDirectoryEntry>(NewParentE);
-      DE->addContent(
-          std::make_unique<RedirectingFileSystem::RedirectingFileEntry>(
-              Name, FE->getExternalContentsPath(), FE->getUseName()));
+      auto *FE = cast<RedirectingFileSystem::FileEntry>(SrcE);
+      auto *DE = cast<RedirectingFileSystem::DirectoryEntry>(NewParentE);
+      DE->addContent(std::make_unique<RedirectingFileSystem::FileEntry>(
       break;
     }
     }
   }
 
   std::unique_ptr<RedirectingFileSystem::Entry>
   parseEntry(yaml::Node *N, RedirectingFileSystem *FS, bool IsRootEntry) {
     auto *M = dyn_cast<yaml::MappingNode>(N);
parseEntry(yaml::Node *N, RedirectingFileSystem *FS, bool IsRootEntry) {
     DenseMap<StringRef, KeyStatus> Keys(std::begin(Fields), std::end(Fields));
 
     bool HasContents = false; // external or otherwise
     std::vector<std::unique_ptr<RedirectingFileSystem::Entry>>
         EntryArrayContents;
     SmallString<256> ExternalContentsPath;
     SmallString<256> Name;
     yaml::Node *NameValueNode = nullptr;
-    auto UseExternalName =
-        RedirectingFileSystem::RedirectingFileEntry::NK_NotSet;
+    auto UseExternalName = RedirectingFileSystem::FileEntry::NK_NotSet;
     RedirectingFileSystem::EntryKind Kind;
 
     for (auto &I : *M) {
       StringRef Key;
       // Reuse the buffer for key and value, since we don't look at key after
       // parsing value.
       SmallString<256> Buffer;
       if (!parseScalarString(I.getKey(), Key, Buffer))
for (auto &I : *M) {
         // Guarantee that old YAML files containing paths with ".." and "."
         // are properly canonicalized before read into the VFS.
         FullPath = canonicalize(FullPath);
         ExternalContentsPath = FullPath.str();
       } else if (Key == "use-external-name") {
         bool Val;
         if (!parseScalarBool(I.getValue(), Val))
           return nullptr;
-        UseExternalName =
-            Val ? RedirectingFileSystem::RedirectingFileEntry::NK_External
-                : RedirectingFileSystem::RedirectingFileEntry::NK_Virtual;
+        UseExternalName = Val ? RedirectingFileSystem::FileEntry::NK_External
+                              : RedirectingFileSystem::FileEntry::NK_Virtual;
       } else {
         llvm_unreachable("key missing from Keys");
       }
     }
 
     if (Stream.failed())
       return nullptr;
 
     // check for missing keys
     if (!HasContents) {
       error(N, "missing key 'contents' or 'external-contents'");
       return nullptr;
     }
     if (!checkMissingKeys(N, Keys))
       return nullptr;
 
     // check invalid configuration
     if (Kind == RedirectingFileSystem::EK_Directory &&
-        UseExternalName !=
-            RedirectingFileSystem::RedirectingFileEntry::NK_NotSet) {
+        UseExternalName != RedirectingFileSystem::FileEntry::NK_NotSet) {
       error(N, "'use-external-name' is not supported for directories");
       return nullptr;
     }
 
     sys::path::Style path_style = sys::path::Style::native;
     if (IsRootEntry) {
       // VFS root entries may be in either Posix or Windows style.  Figure out
       // which style we have, and use it consistently.
while (Trimmed.size() > RootPathLen &&
       Trimmed = Trimmed.slice(0, Trimmed.size() - 1);
 
     // Get the last component
     StringRef LastComponent = sys::path::filename(Trimmed, path_style);
 
     std::unique_ptr<RedirectingFileSystem::Entry> Result;
     switch (Kind) {
     case RedirectingFileSystem::EK_File:
-      Result = std::make_unique<RedirectingFileSystem::RedirectingFileEntry>(
+      Result = std::make_unique<RedirectingFileSystem::FileEntry>(
           LastComponent, std::move(ExternalContentsPath), UseExternalName);
       break;
     case RedirectingFileSystem::EK_Directory:
-      Result =
-          std::make_unique<RedirectingFileSystem::RedirectingDirectoryEntry>(
-              LastComponent, std::move(EntryArrayContents),
-              Status("", getNextVirtualUniqueID(),
-                     std::chrono::system_clock::now(), 0, 0, 0,
-                     file_type::directory_file, sys::fs::all_all));
+      Result = std::make_unique<RedirectingFileSystem::DirectoryEntry>(
+          Status("", getNextVirtualUniqueID(), std::chrono::system_clock::now(),
+                 0, 0, 0, file_type::directory_file, sys::fs::all_all));
       break;
     }
 
     StringRef Parent = sys::path::parent_path(Trimmed, path_style);
     if (Parent.empty())
       return Result;
 
     // if 'name' contains multiple components, create implicit directory entries
     for (sys::path::reverse_iterator I = sys::path::rbegin(Parent, path_style),
                                      E = sys::path::rend(Parent);
          I != E; ++I) {
       std::vector<std::unique_ptr<RedirectingFileSystem::Entry>> Entries;
       Entries.push_back(std::move(Result));
-      Result =
-          std::make_unique<RedirectingFileSystem::RedirectingDirectoryEntry>(
-              *I, std::move(Entries),
-              Status("", getNextVirtualUniqueID(),
-                     std::chrono::system_clock::now(), 0, 0, 0,
-                     file_type::directory_file, sys::fs::all_all));
+      Result = std::make_unique<RedirectingFileSystem::DirectoryEntry>(
+          Status("", getNextVirtualUniqueID(), std::chrono::system_clock::now(),
+                 0, 0, 0, file_type::directory_file, sys::fs::all_all));
     }
     return Result;
   }
 
 public:
   RedirectingFileSystemParser(yaml::Stream &S) : Stream(S) {}
 
   // false on error
for (auto &Mapping : llvm::reverse(RemappedFiles)) {
     assert(Parent && "File without a directory?");
     {
       auto EC = ExternalFS.makeAbsolute(To);
       (void)EC;
       assert(!EC && "Could not make absolute path");
     }
 
     // Add the file.
-    auto NewFile =
-        std::make_unique<RedirectingFileSystem::RedirectingFileEntry>(
-            llvm::sys::path::filename(From), To,
-            UseExternalNames
-                ? RedirectingFileSystem::RedirectingFileEntry::NK_External
-                : RedirectingFileSystem::RedirectingFileEntry::NK_Virtual);
+    auto NewFile = std::make_unique<RedirectingFileSystem::FileEntry>(
+        UseExternalNames ? RedirectingFileSystem::FileEntry::NK_External
+                         : RedirectingFileSystem::FileEntry::NK_Virtual);
     ToEntry = NewFile.get();
-    cast<RedirectingFileSystem::RedirectingDirectoryEntry>(Parent)->addContent(
+    cast<RedirectingFileSystem::DirectoryEntry>(Parent)->addContent(
         std::move(NewFile));
   }
 
   return FS;
 }
 
+bool RedirectingFileSystem::shouldFallBackToExternalFS(
+    std::error_code Error) const {
+  return shouldUseExternalFS() &&
+         Error == llvm::errc::no_such_file_or_directory;
+};
+
 ErrorOr<RedirectingFileSystem::Entry *>
 RedirectingFileSystem::lookupPath(const Twine &Path_) const {
   SmallString<256> Path;
   Path_.toVector(Path);
 
   // Handle relative paths
   if (std::error_code EC = makeAbsolute(Path))
     return EC;
if (!FromName.empty()) {
     ++Start;
 
     if (Start == End) {
       // Match!
       return From;
     }
   }
 
-  auto *DE = dyn_cast<RedirectingFileSystem::RedirectingDirectoryEntry>(From);
+  auto *DE = dyn_cast<RedirectingFileSystem::DirectoryEntry>(From);
   if (!DE)
     return make_error_code(llvm::errc::not_a_directory);
 
   for (const std::unique_ptr<RedirectingFileSystem::Entry> &DirEntry :
        llvm::make_range(DE->contents_begin(), DE->contents_end())) {
     ErrorOr<RedirectingFileSystem::Entry *> Result =
         lookupPath(Start, End, DirEntry.get());
     if (Result || Result.getError() != llvm::errc::no_such_file_or_directory)
if (!UseExternalNames)
     S = Status::copyWithNewName(S, Path);
   S.IsVFSMapped = true;
   return S;
 }
 
 ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path,
                                               RedirectingFileSystem::Entry *E) {
   assert(E != nullptr);
-  if (auto *F = dyn_cast<RedirectingFileSystem::RedirectingFileEntry>(E)) {
+  if (auto *F = dyn_cast<RedirectingFileSystem::FileEntry>(E)) {
     ErrorOr<Status> S = ExternalFS->status(F->getExternalContentsPath());
     assert(!S || S->getName() == F->getExternalContentsPath());
     if (S)
       return getRedirectedFileStatus(Path, F->useExternalName(UseExternalNames),
                                      *S);
     return S;
   } else { // directory
-    auto *DE = cast<RedirectingFileSystem::RedirectingDirectoryEntry>(E);
+    auto *DE = cast<RedirectingFileSystem::DirectoryEntry>(E);
     return Status::copyWithNewName(DE->getStatus(), Path);
   }
 }
 
 ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path) {
   ErrorOr<RedirectingFileSystem::Entry *> Result = lookupPath(Path);
   if (!Result) {
-    if (shouldUseExternalFS() &&
-        Result.getError() == llvm::errc::no_such_file_or_directory) {
+    if (shouldFallBackToExternalFS(Result.getError()))
       return ExternalFS->status(Path);
-    }
     return Result.getError();
   }
   return status(Path, *Result);
 }
 
 namespace {
 
 /// Provide a file wrapper with an overriden status.

 };
 
 } // namespace
 
 ErrorOr<std::unique_ptr<File>>
 RedirectingFileSystem::openFileForRead(const Twine &Path) {
   ErrorOr<RedirectingFileSystem::Entry *> E = lookupPath(Path);
   if (!E) {
-    if (shouldUseExternalFS() &&
-        E.getError() == llvm::errc::no_such_file_or_directory) {
+    if (shouldFallBackToExternalFS(E.getError()))
       return ExternalFS->openFileForRead(Path);
-    }
     return E.getError();
   }
 
-  auto *F = dyn_cast<RedirectingFileSystem::RedirectingFileEntry>(*E);
+  auto *F = dyn_cast<RedirectingFileSystem::FileEntry>(*E);
   if (!F) // FIXME: errc::not_a_file?
     return make_error_code(llvm::errc::invalid_argument);
 
   auto Result = ExternalFS->openFileForRead(F->getExternalContentsPath());
   if (!Result)
     return Result;
 
   auto ExternalStatus = (*Result)->status();
   if (!ExternalStatus)
     return ExternalStatus.getError();
 
   // FIXME: Update the status with the name and VFSMapped.
   Status S = getRedirectedFileStatus(Path, F->useExternalName(UseExternalNames),
                                      *ExternalStatus);
   return std::unique_ptr<File>(
       std::make_unique<FileWithFixedStatus>(std::move(*Result), S));
 }
 
 std::error_code
 RedirectingFileSystem::getRealPath(const Twine &Path,
                                    SmallVectorImpl<char> &Output) const {
   ErrorOr<RedirectingFileSystem::Entry *> Result = lookupPath(Path);
   if (!Result) {
-    if (shouldUseExternalFS() &&
-        Result.getError() == llvm::errc::no_such_file_or_directory) {
+    if (shouldFallBackToExternalFS(Result.getError()))
       return ExternalFS->getRealPath(Path, Output);
-    }
     return Result.getError();
   }
 
-  if (auto *F =
-          dyn_cast<RedirectingFileSystem::RedirectingFileEntry>(*Result)) {
+  if (auto *F = dyn_cast<RedirectingFileSystem::FileEntry>(*Result)) {
     return ExternalFS->getRealPath(F->getExternalContentsPath(), Output);
   }
   // Even if there is a directory entry, fall back to ExternalFS if allowed,
   // because directories don't have a single external contents path.
   return shouldUseExternalFS() ? ExternalFS->getRealPath(Path, Output)
                                : llvm::errc::invalid_argument;
 }
 
 std::unique_ptr<FileSystem>
 vfs::getVFSFromYAML(std::unique_ptr<MemoryBuffer> Buffer,
                     SourceMgr::DiagHandlerTy DiagHandler,
                     StringRef YAMLFilePath, void *DiagContext,
                     IntrusiveRefCntPtr<FileSystem> ExternalFS) {
   return RedirectingFileSystem::create(std::move(Buffer), DiagHandler,
                                        YAMLFilePath, DiagContext,
                                        std::move(ExternalFS));
 }
 
 static void getVFSEntries(RedirectingFileSystem::Entry *SrcE,
                           SmallVectorImpl<StringRef> &Path,
                           SmallVectorImpl<YAMLVFSEntry> &Entries) {
   auto Kind = SrcE->getKind();
   if (Kind == RedirectingFileSystem::EK_Directory) {
-    auto *DE = dyn_cast<RedirectingFileSystem::RedirectingDirectoryEntry>(SrcE);
+    auto *DE = dyn_cast<RedirectingFileSystem::DirectoryEntry>(SrcE);
     assert(DE && "Must be a directory");
     for (std::unique_ptr<RedirectingFileSystem::Entry> &SubEntry :
          llvm::make_range(DE->contents_begin(), DE->contents_end())) {
       Path.push_back(SubEntry->getName());
       getVFSEntries(SubEntry.get(), Path, Entries);
       Path.pop_back();
     }
     return;
   }
 
   assert(Kind == RedirectingFileSystem::EK_File && "Must be a EK_File");
-  auto *FE = dyn_cast<RedirectingFileSystem::RedirectingFileEntry>(SrcE);
+  auto *FE = dyn_cast<RedirectingFileSystem::FileEntry>(SrcE);
   assert(FE && "Must be a file");
   SmallString<128> VPath;
   for (auto &Comp : Path)
     llvm::sys::path::append(VPath, Comp);
   Entries.push_back(YAMLVFSEntry(VPath.c_str(), FE->getExternalContentsPath()));
 }
 
 void vfs::collectVFSFromYAML(std::unique_ptr<MemoryBuffer> Buffer,
void YAMLVFSWriter::write(llvm::raw_ostream &OS) {
   llvm::sort(Mappings, [](const YAMLVFSEntry &LHS, const YAMLVFSEntry &RHS) {
     return LHS.VPath < RHS.VPath;
   });
 
   JSONWriter(OS).write(Mappings, UseExternalNames, IsCaseSensitive,
                        IsOverlayRelative, OverlayDir);
 }
 
-VFSFromYamlDirIterImpl::VFSFromYamlDirIterImpl(
-    const Twine &_Path,
-    RedirectingFileSystem::RedirectingDirectoryEntry::iterator Begin,
-    RedirectingFileSystem::RedirectingDirectoryEntry::iterator End,
-    bool IterateExternalFS, FileSystem &ExternalFS, std::error_code &EC)
-    : Dir(_Path.str()), Current(Begin), End(End),
-      IterateExternalFS(IterateExternalFS), ExternalFS(ExternalFS) {
-  EC = incrementImpl(/*IsFirstTime=*/true);
-}
-
-std::error_code VFSFromYamlDirIterImpl::increment() {
-  return incrementImpl(/*IsFirstTime=*/false);
-}
-
-std::error_code VFSFromYamlDirIterImpl::incrementExternal() {
-  assert(!(IsExternalFSCurrent && ExternalDirIter == directory_iterator()) &&
-         "incrementing past end");
-  std::error_code EC;
-  if (IsExternalFSCurrent) {
-    ExternalDirIter.increment(EC);
-  } else if (IterateExternalFS) {
-    ExternalDirIter = ExternalFS.dir_begin(Dir, EC);
-    IsExternalFSCurrent = true;
-    if (EC && EC != errc::no_such_file_or_directory)
-      return EC;
-    EC = {};
-  }
-  if (EC || ExternalDirIter == directory_iterator()) {
-    CurrentEntry = directory_entry();
-  } else {
-    CurrentEntry = *ExternalDirIter;
-  }
-  return EC;
-}
-
-std::error_code VFSFromYamlDirIterImpl::incrementContent(bool IsFirstTime) {
-  assert((IsFirstTime || Current != End) && "cannot iterate past end");
-  if (!IsFirstTime)
-    ++Current;
-  while (Current != End) {
-    SmallString<128> PathStr(Dir);
-    llvm::sys::path::append(PathStr, (*Current)->getName());
-    sys::fs::file_type Type = sys::fs::file_type::type_unknown;
-    switch ((*Current)->getKind()) {
-    case RedirectingFileSystem::EK_Directory:
-      Type = sys::fs::file_type::directory_file;
-      break;
-    case RedirectingFileSystem::EK_File:
-      Type = sys::fs::file_type::regular_file;
-      break;
-    }
-    CurrentEntry = directory_entry(std::string(PathStr.str()), Type);
-    return {};
-  }
-  return incrementExternal();
-}
-
-std::error_code VFSFromYamlDirIterImpl::incrementImpl(bool IsFirstTime) {
-  while (true) {
-    std::error_code EC = IsExternalFSCurrent ? incrementExternal()
-                                             : incrementContent(IsFirstTime);
-    if (EC || CurrentEntry.path().empty())
-      return EC;
-    StringRef Name = llvm::sys::path::filename(CurrentEntry.path());
-    if (SeenNames.insert(Name).second)
-      return EC; // name not seen before
-  }
-  llvm_unreachable("returned above");
-}
-
 vfs::recursive_directory_iterator::recursive_directory_iterator(
     FileSystem &FS_, const Twine &Path, std::error_code &EC)
     : FS(&FS_) {
   directory_iterator I = FS->dir_begin(Path, EC);
   if (I != directory_iterator()) {
     State = std::make_shared<detail::RecDirIterState>();
     State->Stack.push(I);
   }