[VFS] Update uses of getVFSFromYAML to use the new getVFSFromYAMLs API

[bnbarham]

Summary

Includes two test fixes (since chained mappings are no longer allowed)
and adds a new test for multiple overlays.

Uses other than CompilerInvocation.cpp are simple 1:1 mappings, but
without the need to read into a buffer first.

Depends on D121425

[bnbarham]

llvm/include/llvm/Support/Error.h

1284

Should this be in a change all by itself?

[bnbarham]

Handle empty overlay file in clang tidy

[bnbarham]

Update to single review dependency

[dexonsmith]

Includes two test fixes (since chained mappings are no longer allowed)
and adds a new test for multiple overlays.

Are we sure no one needs to support chained mappings? Has there been a clang-dev discourse discussion about it already? Just concerned that some vendor might rely on being able to support this.

llvm/include/llvm/Support/Error.h

1284

Yes, but I also know you already split this out so I guess you just need to rebase :).

[bnbarham]

In D121426#3376442, @dexonsmith wrote:

Includes two test fixes (since chained mappings are no longer allowed)
and adds a new test for multiple overlays.

Are we sure no one needs to support chained mappings? Has there been a clang-dev discourse discussion about it already? Just concerned that some vendor might rely on being able to support this.

I'm not *positive*, no, but I would be fairly surprised. You could just add the A -> C mapping if you really do want it. But I can start up that conversation if you think it needs having.

I actually didn't initially realise that there was a test for this case - vfsroot-with-overlay.c did test "indirection", but I completely missed it when I was looking through. I *thought* the only case was the one Nathan added in directory.c (and in that case what we really wanted was was what's now fallback).

@vsapsai do you know any clients of the chaining/nesting/indirection?

[bnbarham]

llvm/include/llvm/Support/Error.h

1284

Heh, when you suggested I pull the rename out I basically thought "welll, that answer this question"

So yep, it's out and I'll rebase this one :). Thanks!

[bnbarham]

Merged into D121425 instead.

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
   if (Config.getNumOccurrences() > 0)
     return LoadConfig(Config, "<command-line-config>");
 
   return std::make_unique<FileOptionsProvider>(
       std::move(GlobalOptions), std::move(DefaultOptions),
       std::move(OverrideOptions), std::move(FS));
 }
 
-llvm::IntrusiveRefCntPtr<vfs::FileSystem>
-getVfsFromFile(const std::string &OverlayFile,
-               llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS) {
-  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
-      BaseFS->getBufferForFile(OverlayFile);
-  if (!Buffer) {
-    llvm::errs() << "Can't load virtual filesystem overlay file '"
-                 << OverlayFile << "': " << Buffer.getError().message()
-                 << ".\n";
-    return nullptr;
+static IntrusiveRefCntPtr<vfs::OverlayFileSystem>
+getVFSFromFile(StringRef OverlayFile) {
+  if (OverlayFile.empty())
+    return makeIntrusiveRefCnt<vfs::OverlayFileSystem>(
+        vfs::getRealFileSystem());
+
+  auto OverlayFS = vfs::getVFSFromYAMLs(OverlayFile, vfs::getRealFileSystem());
+  if (auto Err = OverlayFS.takeError()) {
+    handleAllErrors(std::move(Err), [](const FileError &FE) {
+      if (FE.convertToErrorCode() == std::errc::no_such_file_or_directory) {
+        errs() << "Can't load virtual filesystem overlay file '"
+               << FE.getFileName() << "': " << FE.message() << ".\n";
+      } else {
+        errs() << "Error: invalid virtual filesystem overlay file '"
+               << FE.getFileName() << "'.\n";
       }
-
-  IntrusiveRefCntPtr<vfs::FileSystem> FS = vfs::getVFSFromYAML(
-      std::move(Buffer.get()), /*DiagHandler*/ nullptr, OverlayFile);
-  if (!FS) {
-    llvm::errs() << "Error: invalid virtual filesystem overlay file '"
-                 << OverlayFile << "'.\n";
+    });
     return nullptr;
   }
-  return FS;
+  return std::move(*OverlayFS);
 }
 
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
 
   // Enable help for -load option, if plugins are enabled.
   if (cl::Option *LoadOpt = cl::getRegisteredOptions().lookup("load"))
     LoadOpt->addCategory(ClangTidyCategory);
 
   llvm::Expected<CommonOptionsParser> OptionsParser =
       CommonOptionsParser::create(argc, argv, ClangTidyCategory,
                                   cl::ZeroOrMore);
   if (!OptionsParser) {
     llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
     return 1;
   }
 
-  llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> BaseFS(
-      new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
-
-  if (!VfsOverlay.empty()) {
-    IntrusiveRefCntPtr<vfs::FileSystem> VfsFromFile =
-        getVfsFromFile(VfsOverlay, BaseFS);
-    if (!VfsFromFile)
-      return 1;
-    BaseFS->pushOverlay(std::move(VfsFromFile));
-  }
+  llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> BaseFS =
+      getVFSFromFile(VfsOverlay);
+  if (!BaseFS)
 
   auto OwningOptionsProvider = createOptionsProvider(BaseFS);
   auto *OptionsProvider = OwningOptionsProvider.get();
   if (!OptionsProvider)
     return 1;
 
   auto MakeAbsolute = [](const std::string &Input) -> SmallString<256> {
     if (Input.empty())

clang/lib/Frontend/CompilerInvocation.cpp

clang::createVFSFromCompilerInvocation(const CompilerInvocation &CI,
   return createVFSFromCompilerInvocation(CI, Diags,
                                          llvm::vfs::getRealFileSystem());
 }
 
 IntrusiveRefCntPtr<llvm::vfs::FileSystem>
 clang::createVFSFromCompilerInvocation(
     const CompilerInvocation &CI, DiagnosticsEngine &Diags,
     IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) {
-  if (CI.getHeaderSearchOpts().VFSOverlayFiles.empty())
+  ArrayRef<std::string> OptFiles = CI.getHeaderSearchOpts().VFSOverlayFiles;
+  if (OptFiles.empty())
     return BaseFS;
 
-  IntrusiveRefCntPtr<llvm::vfs::FileSystem> Result = BaseFS;
-  // earlier vfs files are on the bottom
-  for (const auto &File : CI.getHeaderSearchOpts().VFSOverlayFiles) {
-    llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
-        Result->getBufferForFile(File);
-    if (!Buffer) {
-      Diags.Report(diag::err_missing_vfs_overlay_file) << File;
-      continue;
-    }
-
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = llvm::vfs::getVFSFromYAML(
-        std::move(Buffer.get()), /*DiagHandler*/ nullptr, File,
-        /*DiagContext*/ nullptr, Result);
-    if (!FS) {
-      Diags.Report(diag::err_invalid_vfs_overlay) << File;
-      continue;
+  SmallVector<StringRef> Files(OptFiles.begin(), OptFiles.end());
+  Expected<IntrusiveRefCntPtr<llvm::vfs::FileSystem>> OverlayFS =
+      llvm::vfs::getVFSFromYAMLs(Files, BaseFS);
+  if (auto Err = OverlayFS.takeError()) {
+    llvm::handleAllErrors(std::move(Err), [&](const llvm::FileError &FE) {
+      if (FE.convertToErrorCode() == std::errc::no_such_file_or_directory) {
+        Diags.Report(diag::err_missing_vfs_overlay_file) << FE.getFileName();
+      } else {
+        Diags.Report(diag::err_invalid_vfs_overlay) << FE.getFileName();
       }
-
-    Result = FS;
+    });
+    return BaseFS;
   }
-  return Result;
+  return *OverlayFS;
 }

clang/test/VFS/Inputs/vfsroot.yaml

'roots': [
         { 'name': 'not_real.h', 'type': 'file',
           'external-contents': 'TEST_DIR/Inputs/actual_header.h'
         },
         { 'name': 'vfsoverlay.yaml', 'type': 'file',
           'external-contents': 'OUT_DIR/vfsoverlay.yaml'
         }
       ]
     },
-    { 'name': '/indirect-vfs-root-files', 'type': 'directory',
-      'contents': [
-        { 'name': 'actual_header.h', 'type': 'file',
-          'external-contents': 'TEST_DIR/Inputs/actual_header.h'
-        }
-      ]
-    },
     { 'name': 'TEST_DIR/Inputs/Broken.framework', 'type': 'directory',
       'contents': [
         { 'name': 'Headers/A.h', 'type': 'file',
           'external-contents': 'TEST_DIR/Inputs/Broken.framework/VFSHeaders/A.h'
         },
         { 'name': 'Modules/module.modulemap', 'type': 'file',
           'external-contents': 'TEST_DIR/Inputs/Broken.framework/Modules/module.modulemap'
         }

clang/test/VFS/directory.c

 // RUN: rm -rf %t
 // RUN: mkdir -p %t/Underlying
 // RUN: mkdir -p %t/Overlay
-// RUN: mkdir -p %t/Middle
 // RUN: echo '// B.h in Underlying' > %t/Underlying/B.h
 // RUN: echo '#ifdef NESTED' >> %t/Underlying/B.h
 // RUN: echo '#include "C.h"' >> %t/Underlying/B.h
 // RUN: echo '#endif' >> %t/Underlying/B.h
 // RUN: echo '// C.h in Underlying' > %t/Underlying/C.h
-// RUN: echo '// C.h in Middle' > %t/Middle/C.h
 // RUN: echo '// C.h in Overlay' > %t/Overlay/C.h
 
 // 1) Underlying -> Overlay (C.h found, B.h falling back to Underlying)
 // RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}/Overlay@g" -e "s@OUT_DIR@%{/t:regex_replacement}/Underlying@g" %S/Inputs/vfsoverlay-directory.yaml > %t/vfs.yaml
 // RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
 // RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -fsyntax-only -DNESTED -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
 // RUN: sed -e "s@INPUT_DIR@Overlay@g" -e "s@OUT_DIR@%{/t:regex_replacement}/Underlying@g" %S/Inputs/vfsoverlay-directory-relative.yaml > %t/vfs-relative.yaml
 // RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs-relative.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
 
+// DIRECT: # 1 "{{.*(/|\\\\)Underlying(/|\\\\)}}B.h"
 // DIRECT: {{^}}// B.h in Underlying
+// DIRECT: # 1 "{{.*(/|\\\\)Overlay(/|\\\\)}}C.h"
 // DIRECT: {{^}}// C.h in Overlay
 
-// 2) Underlying -> Middle -> Overlay (C.h found, B.h falling back to Underlying)
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}/Overlay@g" -e "s@OUT_DIR@%{/t:regex_replacement}/Middle@g" %S/Inputs/vfsoverlay-directory.yaml > %t/vfs.yaml
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}/Middle@g" -e "s@OUT_DIR@%{/t:regex_replacement}/Underlying@g" %S/Inputs/vfsoverlay-directory.yaml > %t/vfs2.yaml
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -DNESTED -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s
-
-// Same as direct above
-
-// 3) Underlying -> Middle -> Overlay (C.h falling back to Middle, B.h falling back to Underlying)
-// RUN: rm -f %t/Overlay/C.h
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=FALLBACK %s
-
-// FALLBACK: {{^}}// B.h in Underlying
-// FALLBACK: {{^}}// C.h in Middle
-
-// 3) Underlying -> Middle -> Overlay (C.h falling back to Underlying, B.h falling back to Underlying)
-// RUN: rm -f %t/Middle/C.h
-// RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=FALLBACK2 %s
-
-// FALLBACK2: {{^}}// B.h in Underlying
-// FALLBACK2: {{^}}// C.h in Underlying
-
 #include "B.h"
 #ifndef NESTED
 #include "C.h"
 #endif

clang/test/VFS/multiple-overlays.c

+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@EXTERNAL_DIR@%{/t:regex_replacement}/B@g" -e "s@NAME_DIR@%{/t:regex_replacement}/A@g" %t/vfs/base.yaml > %t/vfs/a-b.yaml
+// RUN: sed -e "s@EXTERNAL_DIR@%{/t:regex_replacement}/C@g" -e "s@NAME_DIR@%{/t:regex_replacement}/B@g" %t/vfs/base.yaml > %t/vfs/b-c.yaml
+
+// Overlays should not be transitive, ie. given overlays of A -> B and B -> C, A should not remap to
+// C.
+
+// RUN: %clang_cc1 -Werror -I %t/A -ivfsoverlay %t/vfs/b-c.yaml -ivfsoverlay %t/vfs/a-b.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=FROM_B %s
+// FROM_B: # 1 "{{.*(/|\\\\)B(/|\\\\)}}Header.h"
+// FROM_B: // Header.h in B
+
+// RUN: %clang_cc1 -Werror -I %t/B -ivfsoverlay %t/vfs/b-c.yaml -ivfsoverlay %t/vfs/a-b.yaml -fsyntax-only -E -C %t/main.c 2>&1 | FileCheck --check-prefix=FROM_C %s
+// FROM_C: # 1 "{{.*(/|\\\\)C(/|\\\\)}}Header.h"
+// FROM_C: // Header.h in C
+
+//--- main.c
+#include "Header.h"
+
+//--- A/Header.h
+// Header.h in A
+
+//--- B/Header.h
+// Header.h in B
+
+//--- C/Header.h
+// Header.h in C
+
+//--- vfs/base.yaml
+{
+  'version': 0,
+  'redirecting-with': 'fallthrough',
+  'roots': [
+    { 'name': 'NAME_DIR',
+      'type': 'directory-remap',
+      'external-contents': 'EXTERNAL_DIR'
+    }
+  ]
+}

clang/test/VFS/vfsroot-with-overlay.c

 // RUN: rm -rf %t
 // RUN: mkdir -p %t
 // RUN: sed -e "s@TEST_DIR@%{/S:regex_replacement}@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsroot.yaml > %t.yaml
-// RUN: sed -e "s@INPUT_DIR@/indirect-vfs-root-files@g" -e "s@OUT_DIR@/overlay-dir@g" %S/Inputs/vfsoverlay.yaml > %t/vfsoverlay.yaml
+// RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@/overlay-dir@g" %S/Inputs/vfsoverlay.yaml > %t/vfsoverlay.yaml
 // RUN: %clang_cc1 -Werror -ivfsoverlay %t.yaml -ivfsoverlay /direct-vfs-root-files/vfsoverlay.yaml -I /overlay-dir -fsyntax-only /tests/vfsroot-with-overlay.c
 
 #include "not_real.h"
 
 void foo(void) {
   bar();
 }

lldb/source/Commands/CommandObjectReproducer.cpp

bool DoExecute(Args &command, CommandReturnObject &result) override {
     Loader *loader =
         GetLoaderFromPathOrCurrent(loader_storage, result, m_options.file);
     if (!loader)
       return false;
 
     switch (m_options.provider) {
     case eReproducerProviderFiles: {
       FileSpec vfs_mapping = loader->GetFile<FileProvider::Info>();
+      std::string overlay_path = vfs_mapping.GetPath();
 
-      // Read the VFS mapping.
-      ErrorOr<std::unique_ptr<MemoryBuffer>> buffer =
-          vfs::getRealFileSystem()->getBufferForFile(vfs_mapping.GetPath());
-      if (!buffer) {
-        SetError(result, errorCodeToError(buffer.getError()));
+      Expected<IntrusiveRefCntPtr<vfs::FileSystem>> vfs =
+          vfs::getVFSFromYAMLs(StringRef(overlay_path));
+      if (auto err = vfs.takeError()) {
+        SetError(result, std::move(err));
         return false;
       }
 
-      // Initialize a VFS from the given mapping.
-      IntrusiveRefCntPtr<vfs::FileSystem> vfs = vfs::getVFSFromYAML(
-          std::move(buffer.get()), nullptr, vfs_mapping.GetPath());
-
       // Dump the VFS to a buffer.
       std::string str;
       raw_string_ostream os(str);
-      static_cast<vfs::RedirectingFileSystem &>(*vfs).dump(os);
+      (*vfs)->dump(os);
       os.flush();
 
       // Return the string.
       result.AppendMessage(str);
       result.SetStatus(eReturnStatusSuccessFinishResult);
       return true;
     }
     case eReproducerProviderSymbolFiles: {

llvm/include/llvm/Support/Error.h

public:
 
   std::string messageWithoutFileInfo() const {
     std::string Msg;
     raw_string_ostream OS(Msg);
     Err->log(OS);
     return OS.str();
   }
 
-  StringRef getFileName() { return FileName; }
+  StringRef getFileName() const { return FileName; }
 
   Error takeError() { return Error(std::move(Err)); }
 
   std::error_code convertToErrorCode() const override;
 
   // Used by ErrorInfo::classID.
   static char ID;
 

llvm/tools/dsymutil/Reproducer.h

private:
   /// The path to the reproducer.
   std::string Root;
 
   /// The FileCollector used by the FileCollectorFileSystem.
   std::shared_ptr<FileCollector> FC;
 };
 
 /// Reproducer instance used to use an existing reproducer. The VFS returned by
-/// this instance is a RedirectingFileSystem that remaps paths to their
-/// counterpart in the reproducer.
+/// this instance remaps paths to their counterpart in the reproducer.
 class ReproducerUse : public Reproducer {
 public:
   ReproducerUse(StringRef Root, std::error_code &EC);
   ~ReproducerUse() override;
 
 private:
   /// The path to the reproducer.
   std::string Root;
 };
 
 } // end namespace dsymutil
 } // end namespace llvm
 
 #endif // LLVM_TOOLS_DSYMUTIL_REPRODUCER_H

llvm/tools/dsymutil/Reproducer.cpp

ReproducerGenerate::~ReproducerGenerate() {
   outs() << "reproducer written to " << Root << '\n';
 }
 
 ReproducerUse::~ReproducerUse() = default;
 
 ReproducerUse::ReproducerUse(StringRef Root, std::error_code &EC) {
   SmallString<128> Mapping(Root);
   sys::path::append(Mapping, "mapping.yaml");
-  ErrorOr<std::unique_ptr<MemoryBuffer>> Buffer =
-      vfs::getRealFileSystem()->getBufferForFile(Mapping.str());
 
-  if (!Buffer) {
-    EC = Buffer.getError();
+  auto OverlayFS = llvm::vfs::getVFSFromYAMLs(Mapping.str());
+  if (auto Err = OverlayFS.takeError()) {
+    llvm::handleAllErrors(std::move(Err), [&](const llvm::ErrorInfoBase &E) {
+      EC = E.convertToErrorCode();
+    });
     return;
   }
-
-  VFS = llvm::vfs::getVFSFromYAML(std::move(Buffer.get()), nullptr, Mapping);
+  VFS = std::move(*OverlayFS);
 }
 
 llvm::Expected<std::unique_ptr<Reproducer>>
 Reproducer::createReproducer(ReproducerMode Mode, StringRef Root) {
   switch (Mode) {
   case ReproducerMode::Generate: {
     std::error_code EC;
     std::unique_ptr<Reproducer> Repro =