[llvm] c972175 - [VFS] Use original path when falling back to external FS
Keith Smiley via llvm-commits
llvm-commits at lists.llvm.org
Sat Nov 13 10:06:31 PST 2021
Author: Keith Smiley
Date: 2021-11-13T09:34:44-08:00
New Revision: c972175649f4bb50d40d911659a04d5620ce6fe0
URL: https://github.com/llvm/llvm-project/commit/c972175649f4bb50d40d911659a04d5620ce6fe0
DIFF: https://github.com/llvm/llvm-project/commit/c972175649f4bb50d40d911659a04d5620ce6fe0.diff
LOG: [VFS] Use original path when falling back to external FS
This is a follow up to 0be9ca7c0f9a733f846bb6bc4e8e36d46b518162 to make
paths in the case of falling back to the external file system use the
original format, preserving relative paths, and allow the external
filesystem to canonicalize them if needed.
Reviewed By: dexonsmith
Differential Revision: https://reviews.llvm.org/D109128
Added:
clang/test/VFS/relative-path-errors.c
Modified:
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp
Removed:
################################################################################
diff --git a/clang/test/VFS/relative-path-errors.c b/clang/test/VFS/relative-path-errors.c
new file mode 100644
index 000000000000..8d3773a5eb59
--- /dev/null
+++ b/clang/test/VFS/relative-path-errors.c
@@ -0,0 +1,11 @@
+// RUN: mkdir -p %t
+// RUN: cd %t
+// RUN: cp %s main.c
+// RUN: not %clang_cc1 main.c 2>&1 | FileCheck %s
+// RUN: echo '{"roots": [],"version": 0}' > %t.yaml
+// RUN: not %clang_cc1 -ivfsoverlay %t.yaml main.c 2>&1 | FileCheck %s
+// RUN: echo '{"version": 0,"roots":[{"type":"directory","name":"%/t","contents":[{"type":"file","name":"vfsname", "external-contents":"main.c"}]}]}' > %t.yaml
+// RUN: not %clang_cc1 -ivfsoverlay %t.yaml vfsname 2>&1 | FileCheck %s
+
+// CHECK: {{^}}main.c:[[# @LINE + 1]]:1: error:
+foobarbaz
diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h
index 38f426ff0443..10d2389ee079 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -121,6 +121,14 @@ class File {
/// Closes the file.
virtual std::error_code close() = 0;
+
+ // Get the same file with a
diff erent path.
+ static ErrorOr<std::unique_ptr<File>>
+ getWithPath(ErrorOr<std::unique_ptr<File>> Result, const Twine &P);
+
+protected:
+ // Set the file's underlying path.
+ virtual void setPath(const Twine &Path) {}
};
/// A member of a directory, yielded by a directory_iterator.
@@ -757,6 +765,12 @@ class RedirectingFileSystem : public vfs::FileSystem {
/// 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.
+ ErrorOr<Status> getExternalStatus(const Twine &CanonicalPath,
+ const Twine &OriginalPath) const;
+
// In a RedirectingFileSystem, keys can be specified in Posix or Windows
// style (or even a mixture of both), so this comparison helper allows
// slashes (representing a root) to match backslashes (and vice versa). Note
@@ -814,7 +828,8 @@ class RedirectingFileSystem : public vfs::FileSystem {
Entry *From) const;
/// Get the status for a path with the provided \c LookupResult.
- ErrorOr<Status> status(const Twine &Path, const LookupResult &Result);
+ ErrorOr<Status> status(const Twine &CanonicalPath, const Twine &OriginalPath,
+ const LookupResult &Result);
public:
/// Looks up \p Path in \c Roots and returns a LookupResult giving the
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index a4abfe19bcbd..d0ba3114aa5c 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -194,6 +194,7 @@ class RealFile : public File {
bool RequiresNullTerminator,
bool IsVolatile) override;
std::error_code close() override;
+ void setPath(const Twine &Path) override;
};
} // namespace
@@ -229,6 +230,12 @@ std::error_code RealFile::close() {
return EC;
}
+void RealFile::setPath(const Twine &Path) {
+ RealName = Path.str();
+ if (auto Status = status())
+ S = Status.get().copyWithNewName(Status.get(), Path);
+}
+
namespace {
/// A file system according to your operating system.
@@ -639,6 +646,8 @@ class InMemoryFileAdaptor : public File {
}
std::error_code close() override { return {}; }
+
+ void setPath(const Twine &Path) override { RequestedName = Path.str(); }
};
} // namespace
@@ -1244,7 +1253,7 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
}
// Use status to make sure the path exists and refers to a directory.
- ErrorOr<Status> S = status(Path, *Result);
+ ErrorOr<Status> S = status(Path, Dir, *Result);
if (!S) {
if (shouldFallBackToExternalFS(S.getError(), Result->E))
return ExternalFS->dir_begin(Dir, EC);
@@ -1971,47 +1980,68 @@ RedirectingFileSystem::lookupPathImpl(
return make_error_code(llvm::errc::no_such_file_or_directory);
}
-static Status getRedirectedFileStatus(const Twine &Path, bool UseExternalNames,
+static Status getRedirectedFileStatus(const Twine &OriginalPath,
+ bool UseExternalNames,
Status ExternalStatus) {
Status S = ExternalStatus;
if (!UseExternalNames)
- S = Status::copyWithNewName(S, Path);
+ S = Status::copyWithNewName(S, OriginalPath);
S.IsVFSMapped = true;
return S;
}
ErrorOr<Status> RedirectingFileSystem::status(
- const Twine &Path, const RedirectingFileSystem::LookupResult &Result) {
+ const Twine &CanonicalPath, const Twine &OriginalPath,
+ const RedirectingFileSystem::LookupResult &Result) {
if (Optional<StringRef> ExtRedirect = Result.getExternalRedirect()) {
- ErrorOr<Status> S = ExternalFS->status(*ExtRedirect);
+ SmallString<256> CanonicalRemappedPath((*ExtRedirect).str());
+ if (std::error_code EC = makeCanonical(CanonicalRemappedPath))
+ return EC;
+
+ ErrorOr<Status> S = ExternalFS->status(CanonicalRemappedPath);
if (!S)
return S;
+ S = Status::copyWithNewName(*S, *ExtRedirect);
auto *RE = cast<RedirectingFileSystem::RemapEntry>(Result.E);
- return getRedirectedFileStatus(Path, RE->useExternalName(UseExternalNames),
- *S);
+ return getRedirectedFileStatus(OriginalPath,
+ RE->useExternalName(UseExternalNames), *S);
}
auto *DE = cast<RedirectingFileSystem::DirectoryEntry>(Result.E);
- return Status::copyWithNewName(DE->getStatus(), Path);
+ return Status::copyWithNewName(DE->getStatus(), CanonicalPath);
}
-ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path_) {
- SmallString<256> Path;
- Path_.toVector(Path);
+ErrorOr<Status>
+RedirectingFileSystem::getExternalStatus(const Twine &CanonicalPath,
+ const Twine &OriginalPath) const {
+ if (auto Result = ExternalFS->status(CanonicalPath)) {
+ return Result.get().copyWithNewName(Result.get(), OriginalPath);
+ } else {
+ return Result.getError();
+ }
+}
- if (std::error_code EC = makeCanonical(Path))
+ErrorOr<Status> RedirectingFileSystem::status(const Twine &OriginalPath) {
+ SmallString<256> CanonicalPath;
+ OriginalPath.toVector(CanonicalPath);
+
+ if (std::error_code EC = makeCanonical(CanonicalPath))
return EC;
- ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
+ ErrorOr<RedirectingFileSystem::LookupResult> Result =
+ lookupPath(CanonicalPath);
if (!Result) {
- if (shouldFallBackToExternalFS(Result.getError()))
- return ExternalFS->status(Path);
+ if (shouldFallBackToExternalFS(Result.getError())) {
+ return getExternalStatus(CanonicalPath, OriginalPath);
+ }
return Result.getError();
}
- ErrorOr<Status> S = status(Path, *Result);
- if (!S && shouldFallBackToExternalFS(S.getError(), Result->E))
- S = ExternalFS->status(Path);
+ ErrorOr<Status> S = status(CanonicalPath, OriginalPath, *Result);
+ if (!S && shouldFallBackToExternalFS(S.getError(), Result->E)) {
+ return getExternalStatus(CanonicalPath, OriginalPath);
+ }
+
return S;
}
@@ -2036,22 +2066,39 @@ class FileWithFixedStatus : public File {
}
std::error_code close() override { return InnerFile->close(); }
+
+ void setPath(const Twine &Path) override { S = S.copyWithNewName(S, Path); }
};
} // namespace
ErrorOr<std::unique_ptr<File>>
-RedirectingFileSystem::openFileForRead(const Twine &Path_) {
- SmallString<256> Path;
- Path_.toVector(Path);
+File::getWithPath(ErrorOr<std::unique_ptr<File>> Result, const Twine &P) {
+ if (!Result)
+ return Result;
- if (std::error_code EC = makeCanonical(Path))
+ auto F = std::move(*Result);
+ auto Name = F->getName();
+ if (Name && Name.get() != P.str())
+ F->setPath(P);
+ return F;
+}
+
+ErrorOr<std::unique_ptr<File>>
+RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) {
+ SmallString<256> CanonicalPath;
+ OriginalPath.toVector(CanonicalPath);
+
+ if (std::error_code EC = makeCanonical(CanonicalPath))
return EC;
- ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
+ ErrorOr<RedirectingFileSystem::LookupResult> Result =
+ lookupPath(CanonicalPath);
if (!Result) {
if (shouldFallBackToExternalFS(Result.getError()))
- return ExternalFS->openFileForRead(Path);
+ return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath),
+ OriginalPath);
+
return Result.getError();
}
@@ -2059,12 +2106,18 @@ RedirectingFileSystem::openFileForRead(const Twine &Path_) {
return make_error_code(llvm::errc::invalid_argument);
StringRef ExtRedirect = *Result->getExternalRedirect();
+ SmallString<256> CanonicalRemappedPath(ExtRedirect.str());
+ if (std::error_code EC = makeCanonical(CanonicalRemappedPath))
+ return EC;
+
auto *RE = cast<RedirectingFileSystem::RemapEntry>(Result->E);
- auto ExternalFile = ExternalFS->openFileForRead(ExtRedirect);
+ auto ExternalFile = File::getWithPath(
+ ExternalFS->openFileForRead(CanonicalRemappedPath), ExtRedirect);
if (!ExternalFile) {
if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E))
- return ExternalFS->openFileForRead(Path);
+ return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath),
+ OriginalPath);
return ExternalFile;
}
@@ -2074,7 +2127,7 @@ RedirectingFileSystem::openFileForRead(const Twine &Path_) {
// FIXME: Update the status with the name and VFSMapped.
Status S = getRedirectedFileStatus(
- Path, RE->useExternalName(UseExternalNames), *ExternalStatus);
+ OriginalPath, RE->useExternalName(UseExternalNames), *ExternalStatus);
return std::unique_ptr<File>(
std::make_unique<FileWithFixedStatus>(std::move(*ExternalFile), S));
}
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index ca35b5e9f8ba..be32971908ea 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1627,6 +1627,114 @@ TEST_F(VFSFromYAMLTest, RemappedDirectoryOverlayNoFallthrough) {
EXPECT_EQ(0, NumDiagnostics);
}
+TEST_F(VFSFromYAMLTest, ReturnsRequestedPathVFSMiss) {
+ IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS(
+ new vfs::InMemoryFileSystem);
+ BaseFS->addFile("//root/foo/a", 0,
+ MemoryBuffer::getMemBuffer("contents of a"));
+ ASSERT_FALSE(BaseFS->setCurrentWorkingDirectory("//root/foo"));
+ auto RemappedFS = vfs::RedirectingFileSystem::create(
+ {}, /*UseExternalNames=*/false, *BaseFS);
+
+ auto OpenedF = RemappedFS->openFileForRead("a");
+ ASSERT_FALSE(OpenedF.getError());
+ llvm::ErrorOr<std::string> Name = (*OpenedF)->getName();
+ ASSERT_FALSE(Name.getError());
+ EXPECT_EQ("a", Name.get());
+
+ auto OpenedS = (*OpenedF)->status();
+ ASSERT_FALSE(OpenedS.getError());
+ EXPECT_EQ("a", OpenedS->getName());
+ EXPECT_FALSE(OpenedS->IsVFSMapped);
+
+ auto DirectS = RemappedFS->status("a");
+ ASSERT_FALSE(DirectS.getError());
+ EXPECT_EQ("a", DirectS->getName());
+ EXPECT_FALSE(DirectS->IsVFSMapped);
+
+ EXPECT_EQ(0, NumDiagnostics);
+}
+
+TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) {
+ IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS(
+ new vfs::InMemoryFileSystem);
+ BaseFS->addFile("//root/foo/realname", 0,
+ MemoryBuffer::getMemBuffer("contents of a"));
+ auto FS =
+ getFromYAMLString("{ 'use-external-names': true,\n"
+ " 'roots': [\n"
+ "{\n"
+ " 'type': 'directory',\n"
+ " 'name': '//root/foo',\n"
+ " 'contents': [ {\n"
+ " 'type': 'file',\n"
+ " 'name': 'vfsname',\n"
+ " 'external-contents': 'realname'\n"
+ " }\n"
+ " ]\n"
+ "}]}",
+ BaseFS);
+ ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
+
+ auto OpenedF = FS->openFileForRead("vfsname");
+ ASSERT_FALSE(OpenedF.getError());
+ llvm::ErrorOr<std::string> Name = (*OpenedF)->getName();
+ ASSERT_FALSE(Name.getError());
+ EXPECT_EQ("realname", Name.get());
+
+ auto OpenedS = (*OpenedF)->status();
+ ASSERT_FALSE(OpenedS.getError());
+ EXPECT_EQ("realname", OpenedS->getName());
+ EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+ auto DirectS = FS->status("vfsname");
+ ASSERT_FALSE(DirectS.getError());
+ EXPECT_EQ("realname", DirectS->getName());
+ EXPECT_TRUE(DirectS->IsVFSMapped);
+
+ EXPECT_EQ(0, NumDiagnostics);
+}
+
+TEST_F(VFSFromYAMLTest, ReturnsInternalPathVFSHit) {
+ IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS(
+ new vfs::InMemoryFileSystem);
+ BaseFS->addFile("//root/foo/realname", 0,
+ MemoryBuffer::getMemBuffer("contents of a"));
+ auto FS =
+ getFromYAMLString("{ 'use-external-names': false,\n"
+ " 'roots': [\n"
+ "{\n"
+ " 'type': 'directory',\n"
+ " 'name': '//root/foo',\n"
+ " 'contents': [ {\n"
+ " 'type': 'file',\n"
+ " 'name': 'vfsname',\n"
+ " 'external-contents': 'realname'\n"
+ " }\n"
+ " ]\n"
+ "}]}",
+ BaseFS);
+ ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
+
+ auto OpenedF = FS->openFileForRead("vfsname");
+ ASSERT_FALSE(OpenedF.getError());
+ llvm::ErrorOr<std::string> Name = (*OpenedF)->getName();
+ ASSERT_FALSE(Name.getError());
+ EXPECT_EQ("vfsname", Name.get());
+
+ auto OpenedS = (*OpenedF)->status();
+ ASSERT_FALSE(OpenedS.getError());
+ EXPECT_EQ("vfsname", OpenedS->getName());
+ EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+ auto DirectS = FS->status("vfsname");
+ ASSERT_FALSE(DirectS.getError());
+ EXPECT_EQ("vfsname", DirectS->getName());
+ EXPECT_TRUE(DirectS->IsVFSMapped);
+
+ EXPECT_EQ(0, NumDiagnostics);
+}
+
TEST_F(VFSFromYAMLTest, CaseInsensitive) {
IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
Lower->addRegularFile("//root/foo/bar/a");
More information about the llvm-commits
mailing list