[llvm] [llvm][vfs] Make vfs::FileSystem::exists() virtual NFC (PR #88575)
Artem Chikin via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 12 13:31:33 PDT 2024
https://github.com/artemcm created https://github.com/llvm/llvm-project/pull/88575
Allow a `vfs::FileSystem` to provide a more efficient implementation of `exists()` if they are able to. The existing `FileSystem` implementations continue to default to using `status()` except that overlay, proxy, and redirecting filesystems are taught to forward calls to `exists()` correctly to their wrapped/external filesystem.
>From 2f7186e30077b59c80d0a497e6e778addaeba453 Mon Sep 17 00:00:00 2001
From: Ben Langmuir <blangmuir at apple.com>
Date: Tue, 9 Apr 2024 11:22:44 -0700
Subject: [PATCH] [llvm][vfs] Make vfs::FileSystem::exists() virtual NFC
Allow a vfs::FileSystem to provide a more efficient implementation of
exists() if they are able to. The existing FileSystem implementations
continue to default to using status() except that overlay, proxy, and
redirecting filesystems are taught to forward calls to exists()
correctly to their wrapped/external filesystem.
---
llvm/include/llvm/Support/VirtualFileSystem.h | 8 +-
llvm/lib/Support/VirtualFileSystem.cpp | 56 ++++++
.../Support/VirtualFileSystemTest.cpp | 165 ++++++++++++++++++
3 files changed, 227 insertions(+), 2 deletions(-)
diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h
index 4b1ca0c3d262b6..0113d6b7da25d3 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -300,8 +300,9 @@ class FileSystem : public llvm::ThreadSafeRefCountedBase<FileSystem>,
virtual std::error_code getRealPath(const Twine &Path,
SmallVectorImpl<char> &Output);
- /// Check whether a file exists. Provided for convenience.
- bool exists(const Twine &Path);
+ /// Check whether \p Path exists. By default this uses \c status(), but
+ /// filesystems may provide a more efficient implementation if available.
+ virtual bool exists(const Twine &Path);
/// Is the file mounted on a local filesystem?
virtual std::error_code isLocal(const Twine &Path, bool &Result);
@@ -386,6 +387,7 @@ class OverlayFileSystem : public RTTIExtends<OverlayFileSystem, FileSystem> {
void pushOverlay(IntrusiveRefCntPtr<FileSystem> FS);
llvm::ErrorOr<Status> status(const Twine &Path) override;
+ bool exists(const Twine &Path) override;
llvm::ErrorOr<std::unique_ptr<File>>
openFileForRead(const Twine &Path) override;
directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;
@@ -439,6 +441,7 @@ class ProxyFileSystem : public RTTIExtends<ProxyFileSystem, FileSystem> {
llvm::ErrorOr<Status> status(const Twine &Path) override {
return FS->status(Path);
}
+ bool exists(const Twine &Path) override { return FS->exists(Path); }
llvm::ErrorOr<std::unique_ptr<File>>
openFileForRead(const Twine &Path) override {
return FS->openFileForRead(Path);
@@ -1044,6 +1047,7 @@ class RedirectingFileSystem
bool UseExternalNames, FileSystem &ExternalFS);
ErrorOr<Status> status(const Twine &Path) override;
+ bool exists(const Twine &Path) override;
ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) override;
std::error_code getRealPath(const Twine &Path,
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index 32b480028e71b4..921af30bfcde9f 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -438,6 +438,15 @@ ErrorOr<Status> OverlayFileSystem::status(const Twine &Path) {
return make_error_code(llvm::errc::no_such_file_or_directory);
}
+bool OverlayFileSystem::exists(const Twine &Path) {
+ // FIXME: handle symlinks that cross file systems
+ for (iterator I = overlays_begin(), E = overlays_end(); I != E; ++I) {
+ if ((*I)->exists(Path))
+ return true;
+ }
+ return false;
+}
+
ErrorOr<std::unique_ptr<File>>
OverlayFileSystem::openFileForRead(const llvm::Twine &Path) {
// FIXME: handle symlinks that cross file systems
@@ -2428,6 +2437,53 @@ ErrorOr<Status> RedirectingFileSystem::status(const Twine &OriginalPath) {
return S;
}
+bool RedirectingFileSystem::exists(const Twine &OriginalPath) {
+ SmallString<256> Path;
+ OriginalPath.toVector(Path);
+
+ if (makeAbsolute(Path))
+ return false;
+
+ if (Redirection == RedirectKind::Fallback) {
+ // Attempt to find the original file first, only falling back to the
+ // mapped file if that fails.
+ if (ExternalFS->exists(Path))
+ return true;
+ }
+
+ ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
+ if (!Result) {
+ // 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->exists(Path);
+ return false;
+ }
+
+ std::optional<StringRef> ExtRedirect = Result->getExternalRedirect();
+ if (!ExtRedirect) {
+ assert(isa<RedirectingFileSystem::DirectoryEntry>(Result->E));
+ return true;
+ }
+
+ SmallString<256> RemappedPath((*ExtRedirect).str());
+ if (makeAbsolute(RemappedPath))
+ return false;
+
+ if (ExternalFS->exists(RemappedPath))
+ return true;
+
+ if (Redirection == RedirectKind::Fallthrough) {
+ // 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->exists(Path);
+ }
+
+ return false;
+}
+
namespace {
/// Provide a file wrapper with an overriden status.
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index 49a2e19e4f74cd..e9b4ac3d92e1dd 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -201,6 +201,21 @@ class ErrorDummyFileSystem : public DummyFileSystem {
}
};
+/// A version of \c DummyFileSystem that aborts on \c status() to test that
+/// \c exists() is being used.
+class NoStatusDummyFileSystem : public DummyFileSystem {
+public:
+ ErrorOr<vfs::Status> status(const Twine &Path) override {
+ llvm::report_fatal_error(
+ "unexpected call to NoStatusDummyFileSystem::status");
+ }
+
+ bool exists(const Twine &Path) override {
+ auto Status = DummyFileSystem::status(Path);
+ return Status && Status->exists();
+ }
+};
+
/// Replace back-slashes by front-slashes.
std::string getPosixPath(const Twine &S) {
SmallString<128> Result;
@@ -968,6 +983,30 @@ TEST(OverlayFileSystemTest, PrintOutput) {
Output);
}
+TEST(OverlayFileSystemTest, Exists) {
+ IntrusiveRefCntPtr<DummyFileSystem> Lower(new NoStatusDummyFileSystem());
+ IntrusiveRefCntPtr<DummyFileSystem> Upper(new NoStatusDummyFileSystem());
+ IntrusiveRefCntPtr<vfs::OverlayFileSystem> O(
+ new vfs::OverlayFileSystem(Lower));
+ O->pushOverlay(Upper);
+
+ Lower->addDirectory("/both");
+ Upper->addDirectory("/both");
+ Lower->addRegularFile("/both/lower_file");
+ Upper->addRegularFile("/both/upper_file");
+ Lower->addDirectory("/lower");
+ Upper->addDirectory("/upper");
+
+ EXPECT_TRUE(O->exists("/both"));
+ EXPECT_TRUE(O->exists("/both"));
+ EXPECT_TRUE(O->exists("/both/lower_file"));
+ EXPECT_TRUE(O->exists("/both/upper_file"));
+ EXPECT_TRUE(O->exists("/lower"));
+ EXPECT_TRUE(O->exists("/upper"));
+ EXPECT_FALSE(O->exists("/both/nope"));
+ EXPECT_FALSE(O->exists("/nope"));
+}
+
TEST(ProxyFileSystemTest, Basic) {
IntrusiveRefCntPtr<vfs::InMemoryFileSystem> Base(
new vfs::InMemoryFileSystem());
@@ -3364,6 +3403,11 @@ TEST(RedirectingFileSystemTest, ExternalPaths) {
SeenPaths.push_back(Dir.str());
return ProxyFileSystem::dir_begin(Dir, EC);
}
+
+ bool exists(const Twine &Path) override {
+ SeenPaths.push_back(Path.str());
+ return ProxyFileSystem::exists(Path);
+ }
};
std::error_code EC;
@@ -3395,3 +3439,124 @@ TEST(RedirectingFileSystemTest, ExternalPaths) {
EXPECT_EQ(CheckFS->SeenPaths, Expected);
}
+
+TEST(RedirectingFileSystemTest, Exists) {
+ IntrusiveRefCntPtr<DummyFileSystem> Dummy(new NoStatusDummyFileSystem());
+ auto YAML =
+ MemoryBuffer::getMemBuffer("{\n"
+ " 'version': 0,\n"
+ " 'roots': [\n"
+ " {\n"
+ " 'type': 'directory-remap',\n"
+ " 'name': '/dremap',\n"
+ " 'external-contents': '/a',\n"
+ " },"
+ " {\n"
+ " 'type': 'directory-remap',\n"
+ " 'name': '/dmissing',\n"
+ " 'external-contents': '/dmissing',\n"
+ " },"
+ " {\n"
+ " 'type': 'directory',\n"
+ " 'name': '/both',\n"
+ " 'contents': [\n"
+ " {\n"
+ " 'type': 'file',\n"
+ " 'name': 'vfile',\n"
+ " 'external-contents': '/c'\n"
+ " }\n"
+ " ]\n"
+ " },\n"
+ " {\n"
+ " 'type': 'directory',\n"
+ " 'name': '/vdir',\n"
+ " 'contents': ["
+ " {\n"
+ " 'type': 'directory-remap',\n"
+ " 'name': 'dremap',\n"
+ " 'external-contents': '/b'\n"
+ " },\n"
+ " {\n"
+ " 'type': 'file',\n"
+ " 'name': 'missing',\n"
+ " 'external-contents': '/missing'\n"
+ " },\n"
+ " {\n"
+ " 'type': 'file',\n"
+ " 'name': 'vfile',\n"
+ " 'external-contents': '/c'\n"
+ " }]\n"
+ " }]\n"
+ "}");
+
+ Dummy->addDirectory("/a");
+ Dummy->addRegularFile("/a/foo");
+ Dummy->addDirectory("/b");
+ Dummy->addRegularFile("/c");
+ Dummy->addRegularFile("/both/foo");
+
+ auto Redirecting = vfs::RedirectingFileSystem::create(
+ std::move(YAML), nullptr, "", nullptr, Dummy);
+
+ EXPECT_TRUE(Redirecting->exists("/dremap"));
+ EXPECT_FALSE(Redirecting->exists("/dmissing"));
+ EXPECT_FALSE(Redirecting->exists("/unknown"));
+ EXPECT_TRUE(Redirecting->exists("/both"));
+ EXPECT_TRUE(Redirecting->exists("/both/foo"));
+ EXPECT_TRUE(Redirecting->exists("/both/vfile"));
+ EXPECT_TRUE(Redirecting->exists("/vdir"));
+ EXPECT_TRUE(Redirecting->exists("/vdir/dremap"));
+ EXPECT_FALSE(Redirecting->exists("/vdir/missing"));
+ EXPECT_TRUE(Redirecting->exists("/vdir/vfile"));
+ EXPECT_FALSE(Redirecting->exists("/vdir/unknown"));
+}
+
+TEST(RedirectingFileSystemTest, ExistsFallback) {
+ IntrusiveRefCntPtr<DummyFileSystem> Dummy(new NoStatusDummyFileSystem());
+ auto YAML =
+ MemoryBuffer::getMemBuffer("{\n"
+ " 'version': 0,\n"
+ " 'redirecting-with': 'fallback',\n"
+ " 'roots': [\n"
+ " {\n"
+ " 'type': 'file',\n"
+ " 'name': '/fallback',\n"
+ " 'external-contents': '/missing',\n"
+ " },"
+ " ]\n"
+ "}");
+
+ Dummy->addRegularFile("/fallback");
+
+ auto Redirecting = vfs::RedirectingFileSystem::create(
+ std::move(YAML), nullptr, "", nullptr, Dummy);
+
+ EXPECT_TRUE(Redirecting->exists("/fallback"));
+ EXPECT_FALSE(Redirecting->exists("/missing"));
+}
+
+TEST(RedirectingFileSystemTest, ExistsRedirectOnly) {
+ IntrusiveRefCntPtr<DummyFileSystem> Dummy(new NoStatusDummyFileSystem());
+ auto YAML =
+ MemoryBuffer::getMemBuffer("{\n"
+ " 'version': 0,\n"
+ " 'redirecting-with': 'redirect-only',\n"
+ " 'roots': [\n"
+ " {\n"
+ " 'type': 'file',\n"
+ " 'name': '/vfile',\n"
+ " 'external-contents': '/a',\n"
+ " },"
+ " ]\n"
+ "}");
+
+ Dummy->addRegularFile("/a");
+ Dummy->addRegularFile("/b");
+
+ auto Redirecting = vfs::RedirectingFileSystem::create(
+ std::move(YAML), nullptr, "", nullptr, Dummy);
+
+ EXPECT_FALSE(Redirecting->exists("/a"));
+ EXPECT_FALSE(Redirecting->exists("/b"));
+ EXPECT_TRUE(Redirecting->exists("/vfile"));
+}
More information about the llvm-commits
mailing list