[llvm] 5889874 - [llvm][vfs] Make vfs::FileSystem::exists() virtual NFC (#88575)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 12 13:34:51 PDT 2024
Author: Artem Chikin
Date: 2024-04-12T13:34:47-07:00
New Revision: 588987405a979f724ab0aa7ad788cc5e87af3649
URL: https://github.com/llvm/llvm-project/commit/588987405a979f724ab0aa7ad788cc5e87af3649
DIFF: https://github.com/llvm/llvm-project/commit/588987405a979f724ab0aa7ad788cc5e87af3649.diff
LOG: [llvm][vfs] Make vfs::FileSystem::exists() virtual NFC (#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.
Co-authored-by: Ben Langmuir <blangmuir at apple.com>
Added:
Modified:
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp
Removed:
################################################################################
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