[llvm] f0cf544 - Revert "[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:11:58 PST 2021
Author: Keith Smiley
Date: 2021-11-13T10:11:51-08:00
New Revision: f0cf544d6f6fe6cbca4c07772998272d6bb433d8
URL: https://github.com/llvm/llvm-project/commit/f0cf544d6f6fe6cbca4c07772998272d6bb433d8
DIFF: https://github.com/llvm/llvm-project/commit/f0cf544d6f6fe6cbca4c07772998272d6bb433d8.diff
LOG: Revert "[VFS] Use original path when falling back to external FS"
```
/work/omp-vega20-0/openmp-offload-amdgpu-runtime/llvm.src/llvm/lib/Support/VirtualFileSystem.cpp: In static member function 'static llvm::ErrorOr<std::unique_ptr<llvm::vfs::File> > llvm::vfs::File::getWithPath(llvm::ErrorOr<std::unique_ptr<llvm::vfs::File> >, const llvm::Twine&)':
/work/omp-vega20-0/openmp-offload-amdgpu-runtime/llvm.src/llvm/lib/Support/VirtualFileSystem.cpp:2084:10: error: could not convert 'F' from 'std::unique_ptr<llvm::vfs::File>' to 'llvm::ErrorOr<std::unique_ptr<llvm::vfs::File> >'
return F;
^
```
This reverts commit c972175649f4bb50d40d911659a04d5620ce6fe0.
Added:
Modified:
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp
Removed:
clang/test/VFS/relative-path-errors.c
################################################################################
diff --git a/clang/test/VFS/relative-path-errors.c b/clang/test/VFS/relative-path-errors.c
deleted file mode 100644
index 8d3773a5eb59..000000000000
--- a/clang/test/VFS/relative-path-errors.c
+++ /dev/null
@@ -1,11 +0,0 @@
-// 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 10d2389ee079..38f426ff0443 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -121,14 +121,6 @@ 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.
@@ -765,12 +757,6 @@ 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
@@ -828,8 +814,7 @@ class RedirectingFileSystem : public vfs::FileSystem {
Entry *From) const;
/// Get the status for a path with the provided \c LookupResult.
- ErrorOr<Status> status(const Twine &CanonicalPath, const Twine &OriginalPath,
- const LookupResult &Result);
+ ErrorOr<Status> status(const Twine &Path, 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 d0ba3114aa5c..a4abfe19bcbd 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -194,7 +194,6 @@ class RealFile : public File {
bool RequiresNullTerminator,
bool IsVolatile) override;
std::error_code close() override;
- void setPath(const Twine &Path) override;
};
} // namespace
@@ -230,12 +229,6 @@ 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.
@@ -646,8 +639,6 @@ class InMemoryFileAdaptor : public File {
}
std::error_code close() override { return {}; }
-
- void setPath(const Twine &Path) override { RequestedName = Path.str(); }
};
} // namespace
@@ -1253,7 +1244,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, Dir, *Result);
+ ErrorOr<Status> S = status(Path, *Result);
if (!S) {
if (shouldFallBackToExternalFS(S.getError(), Result->E))
return ExternalFS->dir_begin(Dir, EC);
@@ -1980,68 +1971,47 @@ RedirectingFileSystem::lookupPathImpl(
return make_error_code(llvm::errc::no_such_file_or_directory);
}
-static Status getRedirectedFileStatus(const Twine &OriginalPath,
- bool UseExternalNames,
+static Status getRedirectedFileStatus(const Twine &Path, bool UseExternalNames,
Status ExternalStatus) {
Status S = ExternalStatus;
if (!UseExternalNames)
- S = Status::copyWithNewName(S, OriginalPath);
+ S = Status::copyWithNewName(S, Path);
S.IsVFSMapped = true;
return S;
}
ErrorOr<Status> RedirectingFileSystem::status(
- const Twine &CanonicalPath, const Twine &OriginalPath,
- const RedirectingFileSystem::LookupResult &Result) {
+ const Twine &Path, const RedirectingFileSystem::LookupResult &Result) {
if (Optional<StringRef> ExtRedirect = Result.getExternalRedirect()) {
- SmallString<256> CanonicalRemappedPath((*ExtRedirect).str());
- if (std::error_code EC = makeCanonical(CanonicalRemappedPath))
- return EC;
-
- ErrorOr<Status> S = ExternalFS->status(CanonicalRemappedPath);
+ ErrorOr<Status> S = ExternalFS->status(*ExtRedirect);
if (!S)
return S;
- S = Status::copyWithNewName(*S, *ExtRedirect);
auto *RE = cast<RedirectingFileSystem::RemapEntry>(Result.E);
- return getRedirectedFileStatus(OriginalPath,
- RE->useExternalName(UseExternalNames), *S);
+ return getRedirectedFileStatus(Path, RE->useExternalName(UseExternalNames),
+ *S);
}
auto *DE = cast<RedirectingFileSystem::DirectoryEntry>(Result.E);
- return Status::copyWithNewName(DE->getStatus(), CanonicalPath);
-}
-
-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();
- }
+ return Status::copyWithNewName(DE->getStatus(), Path);
}
-ErrorOr<Status> RedirectingFileSystem::status(const Twine &OriginalPath) {
- SmallString<256> CanonicalPath;
- OriginalPath.toVector(CanonicalPath);
+ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path_) {
+ SmallString<256> Path;
+ Path_.toVector(Path);
- if (std::error_code EC = makeCanonical(CanonicalPath))
+ if (std::error_code EC = makeCanonical(Path))
return EC;
- ErrorOr<RedirectingFileSystem::LookupResult> Result =
- lookupPath(CanonicalPath);
+ ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
if (!Result) {
- if (shouldFallBackToExternalFS(Result.getError())) {
- return getExternalStatus(CanonicalPath, OriginalPath);
- }
+ if (shouldFallBackToExternalFS(Result.getError()))
+ return ExternalFS->status(Path);
return Result.getError();
}
- ErrorOr<Status> S = status(CanonicalPath, OriginalPath, *Result);
- if (!S && shouldFallBackToExternalFS(S.getError(), Result->E)) {
- return getExternalStatus(CanonicalPath, OriginalPath);
- }
-
+ ErrorOr<Status> S = status(Path, *Result);
+ if (!S && shouldFallBackToExternalFS(S.getError(), Result->E))
+ S = ExternalFS->status(Path);
return S;
}
@@ -2066,39 +2036,22 @@ 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>>
-File::getWithPath(ErrorOr<std::unique_ptr<File>> Result, const Twine &P) {
- if (!Result)
- return Result;
-
- 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);
+RedirectingFileSystem::openFileForRead(const Twine &Path_) {
+ SmallString<256> Path;
+ Path_.toVector(Path);
- if (std::error_code EC = makeCanonical(CanonicalPath))
+ if (std::error_code EC = makeCanonical(Path))
return EC;
- ErrorOr<RedirectingFileSystem::LookupResult> Result =
- lookupPath(CanonicalPath);
+ ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
if (!Result) {
if (shouldFallBackToExternalFS(Result.getError()))
- return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath),
- OriginalPath);
-
+ return ExternalFS->openFileForRead(Path);
return Result.getError();
}
@@ -2106,18 +2059,12 @@ RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) {
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 = File::getWithPath(
- ExternalFS->openFileForRead(CanonicalRemappedPath), ExtRedirect);
+ auto ExternalFile = ExternalFS->openFileForRead(ExtRedirect);
if (!ExternalFile) {
if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E))
- return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath),
- OriginalPath);
+ return ExternalFS->openFileForRead(Path);
return ExternalFile;
}
@@ -2127,7 +2074,7 @@ RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) {
// FIXME: Update the status with the name and VFSMapped.
Status S = getRedirectedFileStatus(
- OriginalPath, RE->useExternalName(UseExternalNames), *ExternalStatus);
+ Path, 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 be32971908ea..ca35b5e9f8ba 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1627,114 +1627,6 @@ 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