keith updated this revision to Diff 379881.
keith marked an inline comment as done.
keith added a comment.
Extract fallback status logic to another function
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109128/new/
https://reviews.llvm.org/D109128
Files:
clang/test/VFS/relative-path-errors.c
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp
Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===================================================================
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1627,6 +1627,114 @@
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");
Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -194,6 +194,7 @@
bool RequiresNullTerminator,
bool IsVolatile) override;
std::error_code close() override;
+ void setPath(const Twine &Path) override;
};
} // namespace
@@ -229,6 +230,12 @@
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 @@
}
std::error_code close() override { return {}; }
+
+ void setPath(const Twine &Path) override { RequestedName = Path.str(); }
};
} // namespace
@@ -1167,6 +1176,9 @@
if (!exists(Path))
return errc::no_such_file_or_directory;
+ if (ExternalFS)
+ ExternalFS->setCurrentWorkingDirectory(Path);
+
SmallString<128> AbsolutePath;
Path.toVector(AbsolutePath);
if (std::error_code EC = makeAbsolute(AbsolutePath))
@@ -1233,7 +1245,7 @@
}
// 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);
@@ -1959,47 +1971,61 @@
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);
if (!S)
return S;
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();
+ }
+}
+
+ErrorOr<Status> RedirectingFileSystem::status(const Twine &OriginalPath) {
+ SmallString<256> CanonicalPath;
+ OriginalPath.toVector(CanonicalPath);
- if (std::error_code EC = makeCanonical(Path))
+ 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;
}
@@ -2024,22 +2050,39 @@
}
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();
}
@@ -2052,7 +2095,8 @@
auto ExternalFile = ExternalFS->openFileForRead(ExtRedirect);
if (!ExternalFile) {
if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E))
- return ExternalFS->openFileForRead(Path);
+ return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath),
+ OriginalPath);
return ExternalFile;
}
@@ -2062,7 +2106,7 @@
// 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));
}
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===================================================================
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -121,6 +121,14 @@
/// Closes the file.
virtual std::error_code close() = 0;
+
+ // Get the same file with a different 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,11 @@
/// 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
@@ -819,7 +832,8 @@
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
Index: clang/test/VFS/relative-path-errors.c
===================================================================
--- /dev/null
+++ clang/test/VFS/relative-path-errors.c
@@ -0,0 +1,8 @@
+// RUN: mkdir -p %t
+// RUN: cd %t
+// RUN: echo '{"roots": [],"version": 0}' > %t.yaml
+// RUN: cp %s main.c
+// RUN: not %clang_cc1 -ivfsoverlay %t.yaml main.c 2>&1 | FileCheck %s
+
+// CHECK: {{^}}main.c:8:1: error:
+foobarbaz
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
