Support: Add RedirectingFileSystem::create from simple list of redirections

[dexonsmith]

Summary

Add an overload of RedirectingFileSystem::create that builds a
redirecting filesystem off of a simple vector of string pairs. This is
intended to be used to support clang::arcmt::FileRemapper and
clang::PreprocessorOptions::RemappedFiles.

[dexonsmith]

ping

[jansvoboda11]

LGTM.

[JDevlieghere]

llvm/include/llvm/Support/VirtualFileSystem.h

735

• Do you need to know that this is a RedirectingFileSystem or can it be a FileSystem?
• Is there a reason this should be a unique_ptr instead of an IntrusiveRefCntPtr (which is more prevalent across the VFS, although I'm not sure why)

[dexonsmith]

llvm/include/llvm/Support/VirtualFileSystem.h

735

For my use case it doesn't matter what kind of FileSystem is returned; I figure this could be convenient for a future client. I would tend toward keeping it this type to match the other overload of RedirectingFileSystem::create, but happy to change it if you disagree.

Regarding unique_ptr, I chose that as a sounder alternative to the raw pointer above; leaves options open to clients if they do want unique ownership, which seems nice to support since it's trivial. My use case will ultimately assign it to an IntrusiveRefCntPtr so if you disagree I can change it.

[dexonsmith]

llvm/include/llvm/Support/VirtualFileSystem.h

735

It does seem nicer to be consistent, so I just sent out https://reviews.llvm.org/D92888, which allows constructing an IntrusiveRefCntPtr from a moved unique_ptr, and https://reviews.llvm.org/D92890, which update existing APIs to use unique_ptr.

[JDevlieghere]

LGTM with the other patches. Thanks!

llvm/include/llvm/Support/VirtualFileSystem.h

public:
 
   /// Parses \p Buffer, which is expected to be in YAML format and
   /// returns a virtual file system representing its contents.
   static std::unique_ptr<RedirectingFileSystem>
   create(std::unique_ptr<MemoryBuffer> Buffer,
          SourceMgr::DiagHandlerTy DiagHandler, StringRef YAMLFilePath,
          void *DiagContext, IntrusiveRefCntPtr<FileSystem> ExternalFS);
 
+  /// Redirect each of the remapped files from first to second.
+  static std::unique_ptr<RedirectingFileSystem>
+  create(ArrayRef<std::pair<std::string, std::string>> RemappedFiles,
+         bool UseExternalNames, FileSystem &ExternalFS);
+
   ErrorOr<Status> status(const Twine &Path) override;
   ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) override;
 
   std::error_code getRealPath(const Twine &Path,
                               SmallVectorImpl<char> &Output) const override;
 
   llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override;
 

llvm/lib/Support/VirtualFileSystem.cpp

for (const auto &I : Keys) {
       if (I.second.Required && !I.second.Seen) {
         error(Obj, Twine("missing key '") + I.first + "'");
         return false;
       }
     }
     return true;
   }
 
-  RedirectingFileSystem::Entry *
+public:
+  static RedirectingFileSystem::Entry *
   lookupOrCreateEntry(RedirectingFileSystem *FS, StringRef Name,
                       RedirectingFileSystem::Entry *ParentEntry = nullptr) {
     if (!ParentEntry) { // Look for a existent root
       for (const auto &Root : FS->Roots) {
         if (Name.equals(Root->getName())) {
           ParentEntry = Root.get();
           return ParentEntry;
         }
lookupOrCreateEntry(RedirectingFileSystem *FS, StringRef Name,
     }
 
     auto *DE =
         cast<RedirectingFileSystem::RedirectingDirectoryEntry>(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);
       // Empty directories could be present in the YAML as a way to
RedirectingFileSystem::create(std::unique_ptr<MemoryBuffer> Buffer,
   }
 
   if (!P.parse(Root, FS.get()))
     return nullptr;
 
   return FS;
 }
 
+std::unique_ptr<RedirectingFileSystem> RedirectingFileSystem::create(
+    ArrayRef<std::pair<std::string, std::string>> RemappedFiles,
+    bool UseExternalNames, FileSystem &ExternalFS) {
+  std::unique_ptr<RedirectingFileSystem> FS(
+      new RedirectingFileSystem(&ExternalFS));
+  FS->UseExternalNames = UseExternalNames;
+
+  StringMap<RedirectingFileSystem::Entry *> Entries;
+
+  for (auto &Mapping : llvm::reverse(RemappedFiles)) {
+    SmallString<128> From = StringRef(Mapping.first);
+    SmallString<128> To = StringRef(Mapping.second);
+    {
+      auto EC = ExternalFS.makeAbsolute(From);
+      (void)EC;
+      assert(!EC && "Could not make absolute path");
+    }
+
+    // Check if we've already mapped this file. The first one we see (in the
+    // reverse iteration) wins.
+    RedirectingFileSystem::Entry *&ToEntry = Entries[From];
+    if (ToEntry)
+      continue;
+
+    // Add parent directories.
+    RedirectingFileSystem::Entry *Parent = nullptr;
+    StringRef FromDirectory = llvm::sys::path::parent_path(From);
+    for (auto I = llvm::sys::path::begin(FromDirectory),
+              E = llvm::sys::path::end(FromDirectory);
+         I != E; ++I) {
+      Parent = RedirectingFileSystemParser::lookupOrCreateEntry(FS.get(), *I,
+                                                                Parent);
+    }
+    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);
+    ToEntry = NewFile.get();
+    cast<RedirectingFileSystem::RedirectingDirectoryEntry>(Parent)->addContent(
+        std::move(NewFile));
+  }
+
+  return FS;
+}
+
 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;

llvm/unittests/Support/VirtualFileSystemTest.cpp

TEST_F(VFSFromYAMLTest, YAMLVFSWriterTestHandleDirs) {
 
   IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLRawString(Buffer, Lower);
   ASSERT_TRUE(FS.get() != nullptr);
 
   EXPECT_FALSE(FS->exists(_a.path("a")));
   EXPECT_FALSE(FS->exists(_b.path("b")));
   EXPECT_FALSE(FS->exists(_c.path("c")));
 }
+
+TEST(VFSFromRemappedFilesTest, Basic) {
+  IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS =
+      new vfs::InMemoryFileSystem;
+  BaseFS->addFile("//root/b", 0, MemoryBuffer::getMemBuffer("contents of b"));
+  BaseFS->addFile("//root/c", 0, MemoryBuffer::getMemBuffer("contents of c"));
+
+  std::vector<std::pair<std::string, std::string>> RemappedFiles = {
+      {"//root/a/a", "//root/b"},
+      {"//root/a/b/c", "//root/c"},
+  };
+  auto RemappedFS = vfs::RedirectingFileSystem::create(
+      RemappedFiles, /*UseExternalNames=*/false, *BaseFS);
+
+  auto StatA = RemappedFS->status("//root/a/a");
+  auto StatB = RemappedFS->status("//root/a/b/c");
+  ASSERT_TRUE(StatA);
+  ASSERT_TRUE(StatB);
+  EXPECT_EQ("//root/a/a", StatA->getName());
+  EXPECT_EQ("//root/a/b/c", StatB->getName());
+
+  auto BufferA = RemappedFS->getBufferForFile("//root/a/a");
+  auto BufferB = RemappedFS->getBufferForFile("//root/a/b/c");
+  ASSERT_TRUE(BufferA);
+  ASSERT_TRUE(BufferB);
+  EXPECT_EQ("contents of b", (*BufferA)->getBuffer());
+  EXPECT_EQ("contents of c", (*BufferB)->getBuffer());
+}
+
+TEST(VFSFromRemappedFilesTest, UseExternalNames) {
+  IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS =
+      new vfs::InMemoryFileSystem;
+  BaseFS->addFile("//root/b", 0, MemoryBuffer::getMemBuffer("contents of b"));
+  BaseFS->addFile("//root/c", 0, MemoryBuffer::getMemBuffer("contents of c"));
+
+  std::vector<std::pair<std::string, std::string>> RemappedFiles = {
+      {"//root/a/a", "//root/b"},
+      {"//root/a/b/c", "//root/c"},
+  };
+  auto RemappedFS = vfs::RedirectingFileSystem::create(
+      RemappedFiles, /*UseExternalNames=*/true, *BaseFS);
+
+  auto StatA = RemappedFS->status("//root/a/a");
+  auto StatB = RemappedFS->status("//root/a/b/c");
+  ASSERT_TRUE(StatA);
+  ASSERT_TRUE(StatB);
+  EXPECT_EQ("//root/b", StatA->getName());
+  EXPECT_EQ("//root/c", StatB->getName());
+
+  auto BufferA = RemappedFS->getBufferForFile("//root/a/a");
+  auto BufferB = RemappedFS->getBufferForFile("//root/a/b/c");
+  ASSERT_TRUE(BufferA);
+  ASSERT_TRUE(BufferB);
+  EXPECT_EQ("contents of b", (*BufferA)->getBuffer());
+  EXPECT_EQ("contents of c", (*BufferB)->getBuffer());
+}
+
+TEST(VFSFromRemappedFilesTest, LastMappingWins) {
+  IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS =
+      new vfs::InMemoryFileSystem;
+  BaseFS->addFile("//root/b", 0, MemoryBuffer::getMemBuffer("contents of b"));
+  BaseFS->addFile("//root/c", 0, MemoryBuffer::getMemBuffer("contents of c"));
+
+  std::vector<std::pair<std::string, std::string>> RemappedFiles = {
+      {"//root/a", "//root/b"},
+      {"//root/a", "//root/c"},
+  };
+  auto RemappedFSKeepName = vfs::RedirectingFileSystem::create(
+      RemappedFiles, /*UseExternalNames=*/false, *BaseFS);
+  auto RemappedFSExternalName = vfs::RedirectingFileSystem::create(
+      RemappedFiles, /*UseExternalNames=*/true, *BaseFS);
+
+  auto StatKeepA = RemappedFSKeepName->status("//root/a");
+  auto StatExternalA = RemappedFSExternalName->status("//root/a");
+  ASSERT_TRUE(StatKeepA);
+  ASSERT_TRUE(StatExternalA);
+  EXPECT_EQ("//root/a", StatKeepA->getName());
+  EXPECT_EQ("//root/c", StatExternalA->getName());
+
+  auto BufferKeepA = RemappedFSKeepName->getBufferForFile("//root/a");
+  auto BufferExternalA = RemappedFSExternalName->getBufferForFile("//root/a");
+  ASSERT_TRUE(BufferKeepA);
+  ASSERT_TRUE(BufferExternalA);
+  EXPECT_EQ("contents of c", (*BufferKeepA)->getBuffer());
+  EXPECT_EQ("contents of c", (*BufferExternalA)->getBuffer());
+}