[llvm] 502f14d - [VFS] Add a "redirecting-with" field to overlays

Keith Smiley via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 3 13:10:37 PST 2022


Author: Ben Barham
Date: 2022-02-03T13:10:23-08:00
New Revision: 502f14d6f2eee10d2993ed22d820f12cf52462d6

URL: https://github.com/llvm/llvm-project/commit/502f14d6f2eee10d2993ed22d820f12cf52462d6
DIFF: https://github.com/llvm/llvm-project/commit/502f14d6f2eee10d2993ed22d820f12cf52462d6.diff

LOG: [VFS] Add a "redirecting-with" field to overlays

Extend "fallthrough" to allow a third option: "fallback". Fallthrough
allows the original path to used if the redirected (or mapped) path
fails. Fallback is the reverse of this, ie. use the original path and
fallback to the mapped path otherwise.

While this result *can* be achieved today using multiple overlays, this
adds a much more intuitive option. As an example, take two directories
"A" and "B". We would like files from "A" to be used, unless they don't
exist, in which case the VFS should fallback to those in "B".

With the current fallthrough option this is possible by adding two
overlays: one mapping from A -> B and another mapping from B -> A. Since
the frontend *nests* the two RedirectingFileSystems, the result will
be that "A" is mapped to "B" and back to "A", unless it isn't in "A" in
which case it fallsthrough to "B" (or fails if it exists in neither).

Using "fallback" semantics allows a single overlay instead: one mapping
from "A" to "B" but only using that mapping if the operation in "A"
fails first.

"redirect-only" is used to represent the current "fallthrough: false"
case.

Differential Revision: https://reviews.llvm.org/D117937

Added: 
    clang/test/VFS/Inputs/redirect-and-fallthrough.yaml
    clang/test/VFS/Inputs/unknown-redirect.yaml
    clang/test/VFS/fallback.c

Modified: 
    clang/test/VFS/parse-errors.c
    llvm/include/llvm/Support/VirtualFileSystem.h
    llvm/lib/Support/VirtualFileSystem.cpp
    llvm/unittests/Support/VirtualFileSystemTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/test/VFS/Inputs/redirect-and-fallthrough.yaml b/clang/test/VFS/Inputs/redirect-and-fallthrough.yaml
new file mode 100644
index 000000000000..0b6ca03f28c5
--- /dev/null
+++ b/clang/test/VFS/Inputs/redirect-and-fallthrough.yaml
@@ -0,0 +1,11 @@
+{
+  'redirecting-with': 'fallthrough',
+  'fallthrough': true,
+  'roots': [
+    {
+      'type': 'directory-remap',
+      'name': '//root/a',
+      'external-contents': '//root/b'
+    }
+  ]
+}

diff  --git a/clang/test/VFS/Inputs/unknown-redirect.yaml b/clang/test/VFS/Inputs/unknown-redirect.yaml
new file mode 100644
index 000000000000..d836fefbe832
--- /dev/null
+++ b/clang/test/VFS/Inputs/unknown-redirect.yaml
@@ -0,0 +1,10 @@
+{
+  'redirecting-with': 'none',
+  'roots': [
+    {
+      'type': 'directory-remap',
+      'name': '//root/a',
+      'external-contents': '//root/b'
+    }
+  ]
+}

diff  --git a/clang/test/VFS/fallback.c b/clang/test/VFS/fallback.c
new file mode 100644
index 000000000000..11392bdc4e44
--- /dev/null
+++ b/clang/test/VFS/fallback.c
@@ -0,0 +1,86 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Test fallback directory remapping, ie. a directory "Base" which is used as
+// a fallback if files are missing from "UseFirst"
+
+// RUN: sed -e "s at EXTERNAL_DIR@%{/t:regex_replacement}/Both/Base at g" -e "s at NAME_DIR@%{/t:regex_replacement}/Both/UseFirst at g" %t/vfs/base.yaml > %t/vfs/both.yaml
+
+// RUN: cp -R %t/Both %t/UseFirstOnly
+// RUN: rm -rf %t/UseFirstOnly/Base
+// RUN: sed -e "s at EXTERNAL_DIR@%{/t:regex_replacement}/UseFirstOnly/Base at g" -e "s at NAME_DIR@%{/t:regex_replacement}/UseFirstOnly/UseFirst at g" %t/vfs/base.yaml > %t/vfs/use-first-only.yaml
+
+// RUN: cp -R %t/Both %t/BaseOnly
+// RUN: rm -rf %t/BaseOnly/UseFirst
+// RUN: sed -e "s at EXTERNAL_DIR@%{/t:regex_replacement}/BaseOnly/Base at g" -e "s at NAME_DIR@%{/t:regex_replacement}/BaseOnly/UseFirst at g" %t/vfs/base.yaml > %t/vfs/base-only.yaml
+
+// RUN: cp -R %t/Both %t/BFallback
+// RUN: rm %t/BFallback/UseFirst/B.h
+// RUN: sed -e "s at EXTERNAL_DIR@%{/t:regex_replacement}/BFallback/Base at g" -e "s at NAME_DIR@%{/t:regex_replacement}/BFallback/UseFirst at g" %t/vfs/base.yaml > %t/vfs/b-fallback.yaml
+
+// RUN: cp -R %t/Both %t/CFallback
+// RUN: rm %t/CFallback/UseFirst/C.h
+// RUN: sed -e "s at EXTERNAL_DIR@%{/t:regex_replacement}/CFallback/Base at g" -e "s at NAME_DIR@%{/t:regex_replacement}/CFallback/UseFirst at g" %t/vfs/base.yaml > %t/vfs/c-fallback.yaml
+
+// Both B.h and C.h are in both folders
+// RUN: %clang_cc1 -Werror -I %t/Both/UseFirst -ivfsoverlay %t/vfs/both.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=IN_UF %s
+
+// IN_UF: # 1 "{{.*(/|\\\\)UseFirst(/|\\\\)}}B.h"
+// IN_UF-NEXT: // B.h in UseFirst
+// IN_UF: # 1 "{{.*(/|\\\\)UseFirst(/|\\\\)}}C.h"
+// IN_UF-NEXT: // C.h in UseFirst
+
+// Base missing, so now they are only in UseFirst
+// RUN: %clang_cc1 -Werror -I %t/UseFirstOnly/UseFirst -ivfsoverlay %t/vfs/use-first-only.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=IN_UF %s
+
+// UseFirst missing, fallback to Base
+// RUN: %clang_cc1 -Werror -I %t/BaseOnly/UseFirst -ivfsoverlay %t/vfs/base-only.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=IN_BASE %s
+
+// IN_BASE: # 1 "{{.*(/|\\\\)Base(/|\\\\)}}B.h"
+// IN_BASE-NEXT: // B.h in Base
+// IN_BASE: # 1 "{{.*(/|\\\\)Base(/|\\\\)}}C.h"
+// IN_BASE-NEXT: // C.h in Base
+
+// B.h missing from UseFirst
+// RUN: %clang_cc1 -Werror -I %t/BFallback/UseFirst -ivfsoverlay %t/vfs/b-fallback.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=B_FALLBACK %s
+
+// B_FALLBACK: # 1 "{{.*(/|\\\\)Base(/|\\\\)}}B.h"
+// B_FALLBACK-NEXT: // B.h in Base
+// B_FALLBACK: # 1 "{{.*(/|\\\\)UseFirst(/|\\\\)}}C.h"
+// B_FALLBACK-NEXT: // C.h in UseFirst
+
+// C.h missing from UseFirst
+// RUN: %clang_cc1 -Werror -I %t/CFallback/UseFirst -ivfsoverlay %t/vfs/c-fallback.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=C_FALLBACK %s
+
+// C_FALLBACK: # 1 "{{.*(/|\\\\)UseFirst(/|\\\\)}}B.h"
+// C_FALLBACK-NEXT: // B.h in UseFirst
+// C_FALLBACK: # 1 "{{.*(/|\\\\)Base(/|\\\\)}}C.h"
+// C_FALLBACK-NEXT: // C.h in Base
+
+//--- main.c
+#include "B.h"
+
+//--- Both/UseFirst/B.h
+// B.h in UseFirst
+#include "C.h"
+
+//--- Both/UseFirst/C.h
+// C.h in UseFirst
+
+//--- Both/Base/B.h
+// B.h in Base
+#include "C.h"
+
+//--- Both/Base/C.h
+// C.h in Base
+
+//--- vfs/base.yaml
+{
+  'version' : 0,
+      'redirecting-with' : 'fallback',
+                           'roots' : [
+                             {'name' : 'NAME_DIR',
+                              'type' : 'directory-remap',
+                              'external-contents' : 'EXTERNAL_DIR'}
+                           ]
+}

diff  --git a/clang/test/VFS/parse-errors.c b/clang/test/VFS/parse-errors.c
index 7194efc65a5d..8aa208438bcc 100644
--- a/clang/test/VFS/parse-errors.c
+++ b/clang/test/VFS/parse-errors.c
@@ -12,3 +12,11 @@
 // RUN: not %clang_cc1 -ivfsoverlay %S/Inputs/unknown-value.yaml -fsyntax-only %s 2>&1 | FileCheck -check-prefix=CHECK-UNKNOWN-VALUE %s
 // CHECK-UNKNOWN-VALUE: expected boolean value
 // CHECK-UNKNOWN-VALUE: invalid virtual filesystem overlay file
+
+// RUN: not %clang_cc1 -ivfsoverlay %S/Inputs/unknown-redirect.yaml -fsyntax-only %s 2>&1 | FileCheck -check-prefix=CHECK-REDIRECT %s
+// CHECK-REDIRECT: expected valid redirect kind
+// CHECK-REDIRECT: invalid virtual filesystem overlay file
+
+// RUN: not %clang_cc1 -ivfsoverlay %S/Inputs/redirect-and-fallthrough.yaml -fsyntax-only %s 2>&1 | FileCheck -check-prefix=CHECK-EXCLUSIVE-KEYS %s
+// CHECK-EXCLUSIVE-KEYS: 'fallthrough' and 'redirecting-with' are mutually exclusive
+// CHECK-EXCLUSIVE-KEYS: invalid virtual filesystem overlay file

diff  --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h
index f5dde334b0a7..3cfb71aa497d 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -571,7 +571,10 @@ class RedirectingFileSystemParser;
 ///   'case-sensitive': <boolean, default=(true for Posix, false for Windows)>
 ///   'use-external-names': <boolean, default=true>
 ///   'overlay-relative': <boolean, default=false>
-///   'fallthrough': <boolean, default=true>
+///   'fallthrough': <boolean, default=true, deprecated - use 'redirecting-with'
+///                   instead>
+///   'redirecting-with': <string, one of 'fallthrough', 'fallback', or
+///                        'redirect-only', default='fallthrough'>
 ///
 /// Virtual directories that list their contents are represented as
 /// \verbatim
@@ -642,6 +645,20 @@ class RedirectingFileSystem : public vfs::FileSystem {
   enum EntryKind { EK_Directory, EK_DirectoryRemap, EK_File };
   enum NameKind { NK_NotSet, NK_External, NK_Virtual };
 
+  /// The type of redirection to perform.
+  enum class RedirectKind {
+    /// Lookup the redirected path first (ie. the one specified in
+    /// 'external-contents') and if that fails "fallthrough" to a lookup of the
+    /// originally provided path.
+    Fallthrough,
+    /// Lookup the provided path first and if that fails, "fallback" to a
+    /// lookup of the redirected path.
+    Fallback,
+    /// Only lookup the redirected path, do not lookup the originally provided
+    /// path.
+    RedirectOnly
+  };
+
   /// A single file or directory in the VFS.
   class Entry {
     EntryKind Kind;
@@ -776,17 +793,11 @@ class RedirectingFileSystem : public vfs::FileSystem {
   friend class RedirectingFSDirIterImpl;
   friend class RedirectingFileSystemParser;
 
-  bool shouldUseExternalFS() const { return IsFallthrough; }
-
   /// Canonicalize path by removing ".", "..", "./", components. This is
   /// a VFS request, do not bother about symlinks in the path components
   /// but canonicalize in order to perform the correct entry search.
   std::error_code makeCanonical(SmallVectorImpl<char> &Path) const;
 
-  /// Whether to fall back to the external file system when an operation fails
-  /// with the given error code on a path associated with the provided Entry.
-  bool shouldFallBackToExternalFS(std::error_code EC, Entry *E = nullptr) const;
-
   /// Get the File status, or error, from the underlying external file system.
   /// This returns the status with the originally requested name, while looking
   /// up the entry using the canonical path.
@@ -834,9 +845,9 @@ class RedirectingFileSystem : public vfs::FileSystem {
   /// names of files.  This global value is overridable on a per-file basis.
   bool UseExternalNames = true;
 
-  /// Whether to attempt a file lookup in external file system after it wasn't
-  /// found in VFS.
-  bool IsFallthrough = true;
+  /// Determines the lookups to perform, as well as their order. See
+  /// \c RedirectKind for details.
+  RedirectKind Redirection = RedirectKind::Fallthrough;
   /// @}
 
   RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> ExternalFS);
@@ -891,7 +902,7 @@ class RedirectingFileSystem : public vfs::FileSystem {
 
   StringRef getExternalContentsPrefixDir() const;
 
-  void setFallthrough(bool Fallthrough);
+  void setRedirection(RedirectingFileSystem::RedirectKind Kind);
 
   std::vector<llvm::StringRef> getRoots() const;
 

diff  --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index f15e301874c4..8573c3a294e3 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -467,28 +467,26 @@ namespace {
 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.
+  /// Iterators to combine, processed in reverse order.
+  SmallVector<directory_iterator, 8> IterList;
+  /// The iterator currently being traversed.
   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() {
-    while (!FSList.empty()) {
-      std::error_code EC;
-      CurrentDirIter = FSList.back()->dir_begin(DirPath, EC);
-      FSList.pop_back();
-      if (EC && EC != errc::no_such_file_or_directory)
-        return EC;
+  /// Sets \c CurrentDirIter to the next iterator in the list, or leaves it as
+  /// is (at its end position) if we've already gone through them all.
+  std::error_code incrementIter(bool IsFirstTime) {
+    while (!IterList.empty()) {
+      CurrentDirIter = IterList.back();
+      IterList.pop_back();
       if (CurrentDirIter != directory_iterator())
         break; // found
     }
+
+    if (IsFirstTime && CurrentDirIter == directory_iterator())
+      return std::error_code(static_cast<int>(errc::no_such_file_or_directory),
+                             std::system_category());
     return {};
   }
 
@@ -499,7 +497,7 @@ class CombiningDirIterImpl : public llvm::vfs::detail::DirIterImpl {
     if (!IsFirstTime)
       CurrentDirIter.increment(EC);
     if (!EC && CurrentDirIter == directory_iterator())
-      EC = incrementFS();
+      EC = incrementIter(IsFirstTime);
     return EC;
   }
 
@@ -520,23 +518,24 @@ class CombiningDirIterImpl : public llvm::vfs::detail::DirIterImpl {
 
 public:
   CombiningDirIterImpl(ArrayRef<FileSystemPtr> FileSystems, std::string Dir,
-                       std::error_code &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);
+                       std::error_code &EC) {
+    for (auto FS : FileSystems) {
+      std::error_code FEC;
+      directory_iterator Iter = FS->dir_begin(Dir, FEC);
+      if (FEC && FEC != errc::no_such_file_or_directory) {
+        EC = FEC;
+        return;
+      }
+      if (!FEC)
+        IterList.push_back(Iter);
     }
+    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);
+  CombiningDirIterImpl(ArrayRef<directory_iterator> DirIters,
+                       std::error_code &EC)
+      : IterList(DirIters.begin(), DirIters.end()) {
+    EC = incrementImpl(true);
   }
 
   std::error_code increment() override { return incrementImpl(false); }
@@ -546,8 +545,11 @@ class CombiningDirIterImpl : public llvm::vfs::detail::DirIterImpl {
 
 directory_iterator OverlayFileSystem::dir_begin(const Twine &Dir,
                                                 std::error_code &EC) {
-  return directory_iterator(
+  directory_iterator Combined = directory_iterator(
       std::make_shared<CombiningDirIterImpl>(FSList, Dir.str(), EC));
+  if (EC)
+    return {};
+  return Combined;
 }
 
 void ProxyFileSystem::anchor() {}
@@ -1079,6 +1081,14 @@ static llvm::SmallString<256> canonicalize(llvm::StringRef Path) {
   return result;
 }
 
+/// Whether the error and entry specify a file/directory that was not found.
+static bool isFileNotFound(std::error_code EC,
+                           RedirectingFileSystem::Entry *E = nullptr) {
+  if (E && !isa<RedirectingFileSystem::DirectoryRemapEntry>(E))
+    return false;
+  return EC == llvm::errc::no_such_file_or_directory;
+}
+
 } // anonymous namespace
 
 
@@ -1255,20 +1265,25 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
 
   ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
   if (!Result) {
-    EC = Result.getError();
-    if (shouldFallBackToExternalFS(EC))
+    if (Redirection != RedirectKind::RedirectOnly &&
+        isFileNotFound(Result.getError()))
       return ExternalFS->dir_begin(Path, EC);
+
+    EC = Result.getError();
     return {};
   }
 
   // Use status to make sure the path exists and refers to a directory.
   ErrorOr<Status> S = status(Path, Dir, *Result);
   if (!S) {
-    if (shouldFallBackToExternalFS(S.getError(), Result->E))
+    if (Redirection != RedirectKind::RedirectOnly &&
+        isFileNotFound(S.getError(), Result->E))
       return ExternalFS->dir_begin(Dir, EC);
+
     EC = S.getError();
     return {};
   }
+
   if (!S->isDirectory()) {
     EC = std::error_code(static_cast<int>(errc::not_a_directory),
                          std::system_category());
@@ -1277,27 +1292,67 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
 
   // Create the appropriate directory iterator based on whether we found a
   // DirectoryRemapEntry or DirectoryEntry.
-  directory_iterator DirIter;
+  directory_iterator RedirectIter;
+  std::error_code RedirectEC;
   if (auto ExtRedirect = Result->getExternalRedirect()) {
     auto RE = cast<RedirectingFileSystem::RemapEntry>(Result->E);
-    DirIter = ExternalFS->dir_begin(*ExtRedirect, EC);
+    RedirectIter = ExternalFS->dir_begin(*ExtRedirect, RedirectEC);
 
     if (!RE->useExternalName(UseExternalNames)) {
       // Update the paths in the results to use the virtual directory's path.
-      DirIter =
+      RedirectIter =
           directory_iterator(std::make_shared<RedirectingFSDirRemapIterImpl>(
-              std::string(Path), DirIter));
+              std::string(Path), RedirectIter));
     }
   } else {
     auto DE = cast<DirectoryEntry>(Result->E);
-    DirIter = directory_iterator(std::make_shared<RedirectingFSDirIterImpl>(
-        Path, DE->contents_begin(), DE->contents_end(), EC));
+    RedirectIter =
+        directory_iterator(std::make_shared<RedirectingFSDirIterImpl>(
+            Path, DE->contents_begin(), DE->contents_end(), RedirectEC));
+  }
+
+  if (RedirectEC) {
+    if (RedirectEC != errc::no_such_file_or_directory) {
+      EC = RedirectEC;
+      return {};
+    }
+    RedirectIter = {};
+  }
+
+  if (Redirection == RedirectKind::RedirectOnly) {
+    EC = RedirectEC;
+    return RedirectIter;
+  }
+
+  std::error_code ExternalEC;
+  directory_iterator ExternalIter = ExternalFS->dir_begin(Path, ExternalEC);
+  if (ExternalEC) {
+    if (ExternalEC != errc::no_such_file_or_directory) {
+      EC = ExternalEC;
+      return {};
+    }
+    ExternalIter = {};
+  }
+
+  SmallVector<directory_iterator, 2> Iters;
+  switch (Redirection) {
+  case RedirectKind::Fallthrough:
+    Iters.push_back(ExternalIter);
+    Iters.push_back(RedirectIter);
+    break;
+  case RedirectKind::Fallback:
+    Iters.push_back(RedirectIter);
+    Iters.push_back(ExternalIter);
+    break;
+  default:
+    llvm_unreachable("unhandled RedirectKind");
   }
 
-  if (!shouldUseExternalFS())
-    return DirIter;
-  return directory_iterator(std::make_shared<CombiningDirIterImpl>(
-      DirIter, ExternalFS, std::string(Path), EC));
+  directory_iterator Combined{
+      std::make_shared<CombiningDirIterImpl>(Iters, EC)};
+  if (EC)
+    return {};
+  return Combined;
 }
 
 void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) {
@@ -1308,8 +1363,9 @@ StringRef RedirectingFileSystem::getExternalContentsPrefixDir() const {
   return ExternalContentsPrefixDir;
 }
 
-void RedirectingFileSystem::setFallthrough(bool Fallthrough) {
-  IsFallthrough = Fallthrough;
+void RedirectingFileSystem::setRedirection(
+    RedirectingFileSystem::RedirectKind Kind) {
+  Redirection = Kind;
 }
 
 std::vector<StringRef> RedirectingFileSystem::getRoots() const {
@@ -1388,6 +1444,23 @@ class llvm::vfs::RedirectingFileSystemParser {
     return false;
   }
 
+  Optional<RedirectingFileSystem::RedirectKind>
+  parseRedirectKind(yaml::Node *N) {
+    SmallString<12> Storage;
+    StringRef Value;
+    if (!parseScalarString(N, Value, Storage))
+      return None;
+
+    if (Value.equals_insensitive("fallthrough")) {
+      return RedirectingFileSystem::RedirectKind::Fallthrough;
+    } else if (Value.equals_insensitive("fallback")) {
+      return RedirectingFileSystem::RedirectKind::Fallback;
+    } else if (Value.equals_insensitive("redirect-only")) {
+      return RedirectingFileSystem::RedirectKind::RedirectOnly;
+    }
+    return None;
+  }
+
   struct KeyStatus {
     bool Required;
     bool Seen = false;
@@ -1731,6 +1804,7 @@ class llvm::vfs::RedirectingFileSystemParser {
         KeyStatusPair("use-external-names", false),
         KeyStatusPair("overlay-relative", false),
         KeyStatusPair("fallthrough", false),
+        KeyStatusPair("redirecting-with", false),
         KeyStatusPair("roots", true),
     };
 
@@ -1789,8 +1863,34 @@ class llvm::vfs::RedirectingFileSystemParser {
         if (!parseScalarBool(I.getValue(), FS->UseExternalNames))
           return false;
       } else if (Key == "fallthrough") {
-        if (!parseScalarBool(I.getValue(), FS->IsFallthrough))
+        if (Keys["redirecting-with"].Seen) {
+          error(I.getValue(),
+                "'fallthrough' and 'redirecting-with' are mutually exclusive");
           return false;
+        }
+
+        bool ShouldFallthrough = false;
+        if (!parseScalarBool(I.getValue(), ShouldFallthrough))
+          return false;
+
+        if (ShouldFallthrough) {
+          FS->Redirection = RedirectingFileSystem::RedirectKind::Fallthrough;
+        } else {
+          FS->Redirection = RedirectingFileSystem::RedirectKind::RedirectOnly;
+        }
+      } else if (Key == "redirecting-with") {
+        if (Keys["fallthrough"].Seen) {
+          error(I.getValue(),
+                "'fallthrough' and 'redirecting-with' are mutually exclusive");
+          return false;
+        }
+
+        if (auto Kind = parseRedirectKind(I.getValue())) {
+          FS->Redirection = *Kind;
+        } else {
+          error(I.getValue(), "expected valid redirect kind");
+          return false;
+        }
       } else {
         llvm_unreachable("key missing from Keys");
       }
@@ -1923,13 +2023,6 @@ RedirectingFileSystem::LookupResult::LookupResult(
   }
 }
 
-bool RedirectingFileSystem::shouldFallBackToExternalFS(
-    std::error_code EC, RedirectingFileSystem::Entry *E) const {
-  if (E && !isa<RedirectingFileSystem::DirectoryRemapEntry>(E))
-    return false;
-  return shouldUseExternalFS() && EC == llvm::errc::no_such_file_or_directory;
-}
-
 std::error_code
 RedirectingFileSystem::makeCanonical(SmallVectorImpl<char> &Path) const {
   if (std::error_code EC = makeAbsolute(Path))
@@ -2046,17 +2139,31 @@ ErrorOr<Status> RedirectingFileSystem::status(const Twine &OriginalPath) {
   if (std::error_code EC = makeCanonical(CanonicalPath))
     return EC;
 
+  if (Redirection == RedirectKind::Fallback) {
+    // Attempt to find the original file first, only falling back to the
+    // mapped file if that fails.
+    ErrorOr<Status> S = getExternalStatus(CanonicalPath, OriginalPath);
+    if (S)
+      return S;
+  }
+
   ErrorOr<RedirectingFileSystem::LookupResult> Result =
       lookupPath(CanonicalPath);
   if (!Result) {
-    if (shouldFallBackToExternalFS(Result.getError())) {
+    // Was not able to map file, fallthrough to using the original path if
+    // that was the specified redirection type.
+    if (Redirection == RedirectKind::Fallthrough &&
+        isFileNotFound(Result.getError()))
       return getExternalStatus(CanonicalPath, OriginalPath);
-    }
     return Result.getError();
   }
 
   ErrorOr<Status> S = status(CanonicalPath, OriginalPath, *Result);
-  if (!S && shouldFallBackToExternalFS(S.getError(), Result->E)) {
+  if (!S && Redirection == RedirectKind::Fallthrough &&
+      isFileNotFound(S.getError(), Result->E)) {
+    // Mapped the file but it wasn't found in the underlying filesystem,
+    // fallthrough to using the original path if that was the specified
+    // redirection type.
     return getExternalStatus(CanonicalPath, OriginalPath);
   }
 
@@ -2110,13 +2217,24 @@ RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) {
   if (std::error_code EC = makeCanonical(CanonicalPath))
     return EC;
 
+  if (Redirection == RedirectKind::Fallback) {
+    // Attempt to find the original file first, only falling back to the
+    // mapped file if that fails.
+    auto F = File::getWithPath(ExternalFS->openFileForRead(CanonicalPath),
+                               OriginalPath);
+    if (F)
+      return F;
+  }
+
   ErrorOr<RedirectingFileSystem::LookupResult> Result =
       lookupPath(CanonicalPath);
   if (!Result) {
-    if (shouldFallBackToExternalFS(Result.getError()))
+    // Was not able to map file, fallthrough to using the original path if
+    // that was the specified redirection type.
+    if (Redirection == RedirectKind::Fallthrough &&
+        isFileNotFound(Result.getError()))
       return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath),
                                OriginalPath);
-
     return Result.getError();
   }
 
@@ -2133,9 +2251,14 @@ RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) {
   auto ExternalFile = File::getWithPath(
       ExternalFS->openFileForRead(CanonicalRemappedPath), ExtRedirect);
   if (!ExternalFile) {
-    if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E))
+    if (Redirection == RedirectKind::Fallthrough &&
+        isFileNotFound(ExternalFile.getError(), Result->E)) {
+      // Mapped the file but it wasn't found in the underlying filesystem,
+      // fallthrough to using the original path if that was the specified
+      // redirection type.
       return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath),
                                OriginalPath);
+    }
     return ExternalFile;
   }
 
@@ -2143,7 +2266,8 @@ RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) {
   if (!ExternalStatus)
     return ExternalStatus.getError();
 
-  // FIXME: Update the status with the name and VFSMapped.
+  // Otherwise, the file was successfully remapped. Mark it as such. Also
+  // replace the underlying path if the external name is being used.
   Status S = getRedirectedFileStatus(
       OriginalPath, RE->useExternalName(UseExternalNames), *ExternalStatus);
   return std::unique_ptr<File>(
@@ -2151,18 +2275,30 @@ RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) {
 }
 
 std::error_code
-RedirectingFileSystem::getRealPath(const Twine &Path_,
+RedirectingFileSystem::getRealPath(const Twine &OriginalPath,
                                    SmallVectorImpl<char> &Output) const {
-  SmallString<256> Path;
-  Path_.toVector(Path);
+  SmallString<256> CanonicalPath;
+  OriginalPath.toVector(CanonicalPath);
 
-  if (std::error_code EC = makeCanonical(Path))
+  if (std::error_code EC = makeCanonical(CanonicalPath))
     return EC;
 
-  ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
+  if (Redirection == RedirectKind::Fallback) {
+    // Attempt to find the original file first, only falling back to the
+    // mapped file if that fails.
+    std::error_code EC = ExternalFS->getRealPath(CanonicalPath, Output);
+    if (!EC)
+      return EC;
+  }
+
+  ErrorOr<RedirectingFileSystem::LookupResult> Result =
+      lookupPath(CanonicalPath);
   if (!Result) {
-    if (shouldFallBackToExternalFS(Result.getError()))
-      return ExternalFS->getRealPath(Path, Output);
+    // Was not able to map file, fallthrough to using the original path if
+    // that was the specified redirection type.
+    if (Redirection == RedirectKind::Fallthrough &&
+        isFileNotFound(Result.getError()))
+      return ExternalFS->getRealPath(CanonicalPath, Output);
     return Result.getError();
   }
 
@@ -2170,16 +2306,21 @@ RedirectingFileSystem::getRealPath(const Twine &Path_,
   // path in the external file system.
   if (auto ExtRedirect = Result->getExternalRedirect()) {
     auto P = ExternalFS->getRealPath(*ExtRedirect, Output);
-    if (!P && shouldFallBackToExternalFS(P, Result->E)) {
-      return ExternalFS->getRealPath(Path, Output);
+    if (P && Redirection == RedirectKind::Fallthrough &&
+        isFileNotFound(P, Result->E)) {
+      // Mapped the file but it wasn't found in the underlying filesystem,
+      // fallthrough to using the original path if that was the specified
+      // redirection type.
+      return ExternalFS->getRealPath(CanonicalPath, Output);
     }
     return P;
   }
 
-  // If we found a DirectoryEntry, still 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;
+  // If we found a DirectoryEntry, still fallthrough to the original path if
+  // allowed, because directories don't have a single external contents path.
+  if (Redirection == RedirectKind::Fallthrough)
+    return ExternalFS->getRealPath(CanonicalPath, Output);
+  return llvm::errc::invalid_argument;
 }
 
 std::unique_ptr<FileSystem>

diff  --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index 6b191c640168..657fc0afb7fc 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1910,7 +1910,25 @@ TEST_F(VFSFromYAMLTest, IllegalVFSFile) {
       Lower);
   EXPECT_EQ(nullptr, FS.get());
 
-  EXPECT_EQ(26, NumDiagnostics);
+  // invalid redirect kind
+  FS = getFromYAMLString("{ 'redirecting-with': 'none', 'roots': [{\n"
+                         "  'type': 'directory-remap',\n"
+                         "  'name': '//root/A',\n"
+                         "  'external-contents': '//root/B' }]}",
+                         Lower);
+  EXPECT_EQ(nullptr, FS.get());
+
+  // redirect and fallthrough passed
+  FS = getFromYAMLString("{ 'redirecting-with': 'fallthrough',\n"
+                         "  'fallthrough': true,\n"
+                         "  'roots': [{\n"
+                         "    'type': 'directory-remap',\n"
+                         "    'name': '//root/A',\n"
+                         "    'external-contents': '//root/B' }]}",
+                         Lower);
+  EXPECT_EQ(nullptr, FS.get());
+
+  EXPECT_EQ(28, NumDiagnostics);
 }
 
 TEST_F(VFSFromYAMLTest, UseExternalName) {
@@ -2710,6 +2728,121 @@ TEST_F(VFSFromYAMLTest, YAMLVFSWriterTestHandleDirs) {
   EXPECT_FALSE(FS->exists(_c.path("c")));
 }
 
+TEST_F(VFSFromYAMLTest, RedirectingWith) {
+  IntrusiveRefCntPtr<DummyFileSystem> Both(new DummyFileSystem());
+  Both->addDirectory("//root/a");
+  Both->addRegularFile("//root/a/f");
+  Both->addDirectory("//root/b");
+  Both->addRegularFile("//root/b/f");
+
+  IntrusiveRefCntPtr<DummyFileSystem> AOnly(new DummyFileSystem());
+  AOnly->addDirectory("//root/a");
+  AOnly->addRegularFile("//root/a/f");
+
+  IntrusiveRefCntPtr<DummyFileSystem> BOnly(new DummyFileSystem());
+  BOnly->addDirectory("//root/b");
+  BOnly->addRegularFile("//root/b/f");
+
+  auto BaseStr = std::string("  'roots': [\n"
+                             "    {\n"
+                             "      'type': 'directory-remap',\n"
+                             "      'name': '//root/a',\n"
+                             "      'external-contents': '//root/b'\n"
+                             "    }\n"
+                             "  ]\n"
+                             "}");
+  auto FallthroughStr = "{ 'redirecting-with': 'fallthrough',\n" + BaseStr;
+  auto FallbackStr = "{ 'redirecting-with': 'fallback',\n" + BaseStr;
+  auto RedirectOnlyStr = "{ 'redirecting-with': 'redirect-only',\n" + BaseStr;
+
+  auto ExpectPath = [&](vfs::FileSystem &FS, StringRef Expected,
+                        StringRef Message) {
+    auto AF = FS.openFileForRead("//root/a/f");
+    ASSERT_FALSE(AF.getError()) << Message;
+    auto AFName = (*AF)->getName();
+    ASSERT_FALSE(AFName.getError()) << Message;
+    EXPECT_EQ(Expected.str(), AFName.get()) << Message;
+
+    auto AS = FS.status("//root/a/f");
+    ASSERT_FALSE(AS.getError()) << Message;
+    EXPECT_EQ(Expected.str(), AS->getName()) << Message;
+  };
+
+  auto ExpectFailure = [&](vfs::FileSystem &FS, StringRef Message) {
+    EXPECT_TRUE(FS.openFileForRead("//root/a/f").getError()) << Message;
+    EXPECT_TRUE(FS.status("//root/a/f").getError()) << Message;
+  };
+
+  {
+    // `f` in both `a` and `b`
+
+    // `fallthrough` tries `external-name` first, so should be `b`
+    IntrusiveRefCntPtr<vfs::FileSystem> Fallthrough =
+        getFromYAMLString(FallthroughStr, Both);
+    ASSERT_TRUE(Fallthrough.get() != nullptr);
+    ExpectPath(*Fallthrough, "//root/b/f", "fallthrough, both exist");
+
+    // `fallback` tries the original name first, so should be `a`
+    IntrusiveRefCntPtr<vfs::FileSystem> Fallback =
+        getFromYAMLString(FallbackStr, Both);
+    ASSERT_TRUE(Fallback.get() != nullptr);
+    ExpectPath(*Fallback, "//root/a/f", "fallback, both exist");
+
+    // `redirect-only` is the same as `fallthrough` but doesn't try the
+    // original on failure, so no change here (ie. `b`)
+    IntrusiveRefCntPtr<vfs::FileSystem> Redirect =
+        getFromYAMLString(RedirectOnlyStr, Both);
+    ASSERT_TRUE(Redirect.get() != nullptr);
+    ExpectPath(*Redirect, "//root/b/f", "redirect-only, both exist");
+  }
+
+  {
+    // `f` in `a` only
+
+    // Fallthrough to the original path, `a`
+    IntrusiveRefCntPtr<vfs::FileSystem> Fallthrough =
+        getFromYAMLString(FallthroughStr, AOnly);
+    ASSERT_TRUE(Fallthrough.get() != nullptr);
+    ExpectPath(*Fallthrough, "//root/a/f", "fallthrough, a only");
+
+    // Original first, so still `a`
+    IntrusiveRefCntPtr<vfs::FileSystem> Fallback =
+        getFromYAMLString(FallbackStr, AOnly);
+    ASSERT_TRUE(Fallback.get() != nullptr);
+    ExpectPath(*Fallback, "//root/a/f", "fallback, a only");
+
+    // Fails since no fallthrough
+    IntrusiveRefCntPtr<vfs::FileSystem> Redirect =
+        getFromYAMLString(RedirectOnlyStr, AOnly);
+    ASSERT_TRUE(Redirect.get() != nullptr);
+    ExpectFailure(*Redirect, "redirect-only, a only");
+  }
+
+  {
+    // `f` in `b` only
+
+    // Tries `b` first (no fallthrough)
+    IntrusiveRefCntPtr<vfs::FileSystem> Fallthrough =
+        getFromYAMLString(FallthroughStr, BOnly);
+    ASSERT_TRUE(Fallthrough.get() != nullptr);
+    ExpectPath(*Fallthrough, "//root/b/f", "fallthrough, b only");
+
+    // Tries original first but then fallsback to `b`
+    IntrusiveRefCntPtr<vfs::FileSystem> Fallback =
+        getFromYAMLString(FallbackStr, BOnly);
+    ASSERT_TRUE(Fallback.get() != nullptr);
+    ExpectPath(*Fallback, "//root/b/f", "fallback, b only");
+
+    // Redirect exists, so uses it (`b`)
+    IntrusiveRefCntPtr<vfs::FileSystem> Redirect =
+        getFromYAMLString(RedirectOnlyStr, BOnly);
+    ASSERT_TRUE(Redirect.get() != nullptr);
+    ExpectPath(*Redirect, "//root/b/f", "redirect-only, b only");
+  }
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
 TEST(VFSFromRemappedFilesTest, Basic) {
   IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS =
       new vfs::InMemoryFileSystem;


        


More information about the llvm-commits mailing list