Skip to content

Commit f0cf544

Browse files
keithkeith
committed
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 c972175.
1 parent b19e823 commit f0cf544

File tree

4 files changed

+28
-215
lines changed

4 files changed

+28
-215
lines changed

‎clang/test/VFS/relative-path-errors.c

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎llvm/include/llvm/Support/VirtualFileSystem.h

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -121,14 +121,6 @@ class File {
121121

122122
/// Closes the file.
123123
virtual std::error_code close() = 0;
124-
125-
// Get the same file with a different path.
126-
static ErrorOr<std::unique_ptr<File>>
127-
getWithPath(ErrorOr<std::unique_ptr<File>> Result, const Twine &P);
128-
129-
protected:
130-
// Set the file's underlying path.
131-
virtual void setPath(const Twine &Path) {}
132124
};
133125

134126
/// A member of a directory, yielded by a directory_iterator.
@@ -765,12 +757,6 @@ class RedirectingFileSystem : public vfs::FileSystem {
765757
/// with the given error code on a path associated with the provided Entry.
766758
bool shouldFallBackToExternalFS(std::error_code EC, Entry *E = nullptr) const;
767759

768-
/// Get the File status, or error, from the underlying external file system.
769-
/// This returns the status with the originally requested name, while looking
770-
/// up the entry using the canonical path.
771-
ErrorOr<Status> getExternalStatus(const Twine &CanonicalPath,
772-
const Twine &OriginalPath) const;
773-
774760
// In a RedirectingFileSystem, keys can be specified in Posix or Windows
775761
// style (or even a mixture of both), so this comparison helper allows
776762
// slashes (representing a root) to match backslashes (and vice versa). Note
@@ -828,8 +814,7 @@ class RedirectingFileSystem : public vfs::FileSystem {
828814
Entry *From) const;
829815

830816
/// Get the status for a path with the provided \c LookupResult.
831-
ErrorOr<Status> status(const Twine &CanonicalPath, const Twine &OriginalPath,
832-
const LookupResult &Result);
817+
ErrorOr<Status> status(const Twine &Path, const LookupResult &Result);
833818

834819
public:
835820
/// Looks up \p Path in \c Roots and returns a LookupResult giving the

‎llvm/lib/Support/VirtualFileSystem.cpp

Lines changed: 27 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,6 @@ class RealFile : public File {
194194
bool RequiresNullTerminator,
195195
bool IsVolatile) override;
196196
std::error_code close() override;
197-
void setPath(const Twine &Path) override;
198197
};
199198

200199
} // namespace
@@ -230,12 +229,6 @@ std::error_code RealFile::close() {
230229
return EC;
231230
}
232231

233-
void RealFile::setPath(const Twine &Path) {
234-
RealName = Path.str();
235-
if (auto Status = status())
236-
S = Status.get().copyWithNewName(Status.get(), Path);
237-
}
238-
239232
namespace {
240233

241234
/// A file system according to your operating system.
@@ -646,8 +639,6 @@ class InMemoryFileAdaptor : public File {
646639
}
647640

648641
std::error_code close() override { return {}; }
649-
650-
void setPath(const Twine &Path) override { RequestedName = Path.str(); }
651642
};
652643
} // namespace
653644

@@ -1253,7 +1244,7 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
12531244
}
12541245

12551246
// Use status to make sure the path exists and refers to a directory.
1256-
ErrorOr<Status> S = status(Path, Dir, *Result);
1247+
ErrorOr<Status> S = status(Path, *Result);
12571248
if (!S) {
12581249
if (shouldFallBackToExternalFS(S.getError(), Result->E))
12591250
return ExternalFS->dir_begin(Dir, EC);
@@ -1980,68 +1971,47 @@ RedirectingFileSystem::lookupPathImpl(
19801971
return make_error_code(llvm::errc::no_such_file_or_directory);
19811972
}
19821973

1983-
static Status getRedirectedFileStatus(const Twine &OriginalPath,
1984-
bool UseExternalNames,
1974+
static Status getRedirectedFileStatus(const Twine &Path, bool UseExternalNames,
19851975
Status ExternalStatus) {
19861976
Status S = ExternalStatus;
19871977
if (!UseExternalNames)
1988-
S = Status::copyWithNewName(S, OriginalPath);
1978+
S = Status::copyWithNewName(S, Path);
19891979
S.IsVFSMapped = true;
19901980
return S;
19911981
}
19921982

19931983
ErrorOr<Status> RedirectingFileSystem::status(
1994-
const Twine &CanonicalPath, const Twine &OriginalPath,
1995-
const RedirectingFileSystem::LookupResult &Result) {
1984+
const Twine &Path, const RedirectingFileSystem::LookupResult &Result) {
19961985
if (Optional<StringRef> ExtRedirect = Result.getExternalRedirect()) {
1997-
SmallString<256> CanonicalRemappedPath((*ExtRedirect).str());
1998-
if (std::error_code EC = makeCanonical(CanonicalRemappedPath))
1999-
return EC;
2000-
2001-
ErrorOr<Status> S = ExternalFS->status(CanonicalRemappedPath);
1986+
ErrorOr<Status> S = ExternalFS->status(*ExtRedirect);
20021987
if (!S)
20031988
return S;
2004-
S = Status::copyWithNewName(*S, *ExtRedirect);
20051989
auto *RE = cast<RedirectingFileSystem::RemapEntry>(Result.E);
2006-
return getRedirectedFileStatus(OriginalPath,
2007-
RE->useExternalName(UseExternalNames), *S);
1990+
return getRedirectedFileStatus(Path, RE->useExternalName(UseExternalNames),
1991+
*S);
20081992
}
20091993

20101994
auto *DE = cast<RedirectingFileSystem::DirectoryEntry>(Result.E);
2011-
return Status::copyWithNewName(DE->getStatus(), CanonicalPath);
2012-
}
2013-
2014-
ErrorOr<Status>
2015-
RedirectingFileSystem::getExternalStatus(const Twine &CanonicalPath,
2016-
const Twine &OriginalPath) const {
2017-
if (auto Result = ExternalFS->status(CanonicalPath)) {
2018-
return Result.get().copyWithNewName(Result.get(), OriginalPath);
2019-
} else {
2020-
return Result.getError();
2021-
}
1995+
return Status::copyWithNewName(DE->getStatus(), Path);
20221996
}
20231997

2024-
ErrorOr<Status> RedirectingFileSystem::status(const Twine &OriginalPath) {
2025-
SmallString<256> CanonicalPath;
2026-
OriginalPath.toVector(CanonicalPath);
1998+
ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path_) {
1999+
SmallString<256> Path;
2000+
Path_.toVector(Path);
20272001

2028-
if (std::error_code EC = makeCanonical(CanonicalPath))
2002+
if (std::error_code EC = makeCanonical(Path))
20292003
return EC;
20302004

2031-
ErrorOr<RedirectingFileSystem::LookupResult> Result =
2032-
lookupPath(CanonicalPath);
2005+
ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
20332006
if (!Result) {
2034-
if (shouldFallBackToExternalFS(Result.getError())) {
2035-
return getExternalStatus(CanonicalPath, OriginalPath);
2036-
}
2007+
if (shouldFallBackToExternalFS(Result.getError()))
2008+
return ExternalFS->status(Path);
20372009
return Result.getError();
20382010
}
20392011

2040-
ErrorOr<Status> S = status(CanonicalPath, OriginalPath, *Result);
2041-
if (!S && shouldFallBackToExternalFS(S.getError(), Result->E)) {
2042-
return getExternalStatus(CanonicalPath, OriginalPath);
2043-
}
2044-
2012+
ErrorOr<Status> S = status(Path, *Result);
2013+
if (!S && shouldFallBackToExternalFS(S.getError(), Result->E))
2014+
S = ExternalFS->status(Path);
20452015
return S;
20462016
}
20472017

@@ -2066,58 +2036,35 @@ class FileWithFixedStatus : public File {
20662036
}
20672037

20682038
std::error_code close() override { return InnerFile->close(); }
2069-
2070-
void setPath(const Twine &Path) override { S = S.copyWithNewName(S, Path); }
20712039
};
20722040

20732041
} // namespace
20742042

20752043
ErrorOr<std::unique_ptr<File>>
2076-
File::getWithPath(ErrorOr<std::unique_ptr<File>> Result, const Twine &P) {
2077-
if (!Result)
2078-
return Result;
2079-
2080-
auto F = std::move(*Result);
2081-
auto Name = F->getName();
2082-
if (Name && Name.get() != P.str())
2083-
F->setPath(P);
2084-
return F;
2085-
}
2086-
2087-
ErrorOr<std::unique_ptr<File>>
2088-
RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) {
2089-
SmallString<256> CanonicalPath;
2090-
OriginalPath.toVector(CanonicalPath);
2044+
RedirectingFileSystem::openFileForRead(const Twine &Path_) {
2045+
SmallString<256> Path;
2046+
Path_.toVector(Path);
20912047

2092-
if (std::error_code EC = makeCanonical(CanonicalPath))
2048+
if (std::error_code EC = makeCanonical(Path))
20932049
return EC;
20942050

2095-
ErrorOr<RedirectingFileSystem::LookupResult> Result =
2096-
lookupPath(CanonicalPath);
2051+
ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
20972052
if (!Result) {
20982053
if (shouldFallBackToExternalFS(Result.getError()))
2099-
return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath),
2100-
OriginalPath);
2101-
2054+
return ExternalFS->openFileForRead(Path);
21022055
return Result.getError();
21032056
}
21042057

21052058
if (!Result->getExternalRedirect()) // FIXME: errc::not_a_file?
21062059
return make_error_code(llvm::errc::invalid_argument);
21072060

21082061
StringRef ExtRedirect = *Result->getExternalRedirect();
2109-
SmallString<256> CanonicalRemappedPath(ExtRedirect.str());
2110-
if (std::error_code EC = makeCanonical(CanonicalRemappedPath))
2111-
return EC;
2112-
21132062
auto *RE = cast<RedirectingFileSystem::RemapEntry>(Result->E);
21142063

2115-
auto ExternalFile = File::getWithPath(
2116-
ExternalFS->openFileForRead(CanonicalRemappedPath), ExtRedirect);
2064+
auto ExternalFile = ExternalFS->openFileForRead(ExtRedirect);
21172065
if (!ExternalFile) {
21182066
if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E))
2119-
return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath),
2120-
OriginalPath);
2067+
return ExternalFS->openFileForRead(Path);
21212068
return ExternalFile;
21222069
}
21232070

@@ -2127,7 +2074,7 @@ RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) {
21272074

21282075
// FIXME: Update the status with the name and VFSMapped.
21292076
Status S = getRedirectedFileStatus(
2130-
OriginalPath, RE->useExternalName(UseExternalNames), *ExternalStatus);
2077+
Path, RE->useExternalName(UseExternalNames), *ExternalStatus);
21312078
return std::unique_ptr<File>(
21322079
std::make_unique<FileWithFixedStatus>(std::move(*ExternalFile), S));
21332080
}

‎llvm/unittests/Support/VirtualFileSystemTest.cpp

Lines changed: 0 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -1627,114 +1627,6 @@ TEST_F(VFSFromYAMLTest, RemappedDirectoryOverlayNoFallthrough) {
16271627
EXPECT_EQ(0, NumDiagnostics);
16281628
}
16291629

1630-
TEST_F(VFSFromYAMLTest, ReturnsRequestedPathVFSMiss) {
1631-
IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS(
1632-
new vfs::InMemoryFileSystem);
1633-
BaseFS->addFile("//root/foo/a", 0,
1634-
MemoryBuffer::getMemBuffer("contents of a"));
1635-
ASSERT_FALSE(BaseFS->setCurrentWorkingDirectory("//root/foo"));
1636-
auto RemappedFS = vfs::RedirectingFileSystem::create(
1637-
{}, /*UseExternalNames=*/false, *BaseFS);
1638-
1639-
auto OpenedF = RemappedFS->openFileForRead("a");
1640-
ASSERT_FALSE(OpenedF.getError());
1641-
llvm::ErrorOr<std::string> Name = (*OpenedF)->getName();
1642-
ASSERT_FALSE(Name.getError());
1643-
EXPECT_EQ("a", Name.get());
1644-
1645-
auto OpenedS = (*OpenedF)->status();
1646-
ASSERT_FALSE(OpenedS.getError());
1647-
EXPECT_EQ("a", OpenedS->getName());
1648-
EXPECT_FALSE(OpenedS->IsVFSMapped);
1649-
1650-
auto DirectS = RemappedFS->status("a");
1651-
ASSERT_FALSE(DirectS.getError());
1652-
EXPECT_EQ("a", DirectS->getName());
1653-
EXPECT_FALSE(DirectS->IsVFSMapped);
1654-
1655-
EXPECT_EQ(0, NumDiagnostics);
1656-
}
1657-
1658-
TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) {
1659-
IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS(
1660-
new vfs::InMemoryFileSystem);
1661-
BaseFS->addFile("//root/foo/realname", 0,
1662-
MemoryBuffer::getMemBuffer("contents of a"));
1663-
auto FS =
1664-
getFromYAMLString("{ 'use-external-names': true,\n"
1665-
" 'roots': [\n"
1666-
"{\n"
1667-
" 'type': 'directory',\n"
1668-
" 'name': '//root/foo',\n"
1669-
" 'contents': [ {\n"
1670-
" 'type': 'file',\n"
1671-
" 'name': 'vfsname',\n"
1672-
" 'external-contents': 'realname'\n"
1673-
" }\n"
1674-
" ]\n"
1675-
"}]}",
1676-
BaseFS);
1677-
ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
1678-
1679-
auto OpenedF = FS->openFileForRead("vfsname");
1680-
ASSERT_FALSE(OpenedF.getError());
1681-
llvm::ErrorOr<std::string> Name = (*OpenedF)->getName();
1682-
ASSERT_FALSE(Name.getError());
1683-
EXPECT_EQ("realname", Name.get());
1684-
1685-
auto OpenedS = (*OpenedF)->status();
1686-
ASSERT_FALSE(OpenedS.getError());
1687-
EXPECT_EQ("realname", OpenedS->getName());
1688-
EXPECT_TRUE(OpenedS->IsVFSMapped);
1689-
1690-
auto DirectS = FS->status("vfsname");
1691-
ASSERT_FALSE(DirectS.getError());
1692-
EXPECT_EQ("realname", DirectS->getName());
1693-
EXPECT_TRUE(DirectS->IsVFSMapped);
1694-
1695-
EXPECT_EQ(0, NumDiagnostics);
1696-
}
1697-
1698-
TEST_F(VFSFromYAMLTest, ReturnsInternalPathVFSHit) {
1699-
IntrusiveRefCntPtr<vfs::InMemoryFileSystem> BaseFS(
1700-
new vfs::InMemoryFileSystem);
1701-
BaseFS->addFile("//root/foo/realname", 0,
1702-
MemoryBuffer::getMemBuffer("contents of a"));
1703-
auto FS =
1704-
getFromYAMLString("{ 'use-external-names': false,\n"
1705-
" 'roots': [\n"
1706-
"{\n"
1707-
" 'type': 'directory',\n"
1708-
" 'name': '//root/foo',\n"
1709-
" 'contents': [ {\n"
1710-
" 'type': 'file',\n"
1711-
" 'name': 'vfsname',\n"
1712-
" 'external-contents': 'realname'\n"
1713-
" }\n"
1714-
" ]\n"
1715-
"}]}",
1716-
BaseFS);
1717-
ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
1718-
1719-
auto OpenedF = FS->openFileForRead("vfsname");
1720-
ASSERT_FALSE(OpenedF.getError());
1721-
llvm::ErrorOr<std::string> Name = (*OpenedF)->getName();
1722-
ASSERT_FALSE(Name.getError());
1723-
EXPECT_EQ("vfsname", Name.get());
1724-
1725-
auto OpenedS = (*OpenedF)->status();
1726-
ASSERT_FALSE(OpenedS.getError());
1727-
EXPECT_EQ("vfsname", OpenedS->getName());
1728-
EXPECT_TRUE(OpenedS->IsVFSMapped);
1729-
1730-
auto DirectS = FS->status("vfsname");
1731-
ASSERT_FALSE(DirectS.getError());
1732-
EXPECT_EQ("vfsname", DirectS->getName());
1733-
EXPECT_TRUE(DirectS->IsVFSMapped);
1734-
1735-
EXPECT_EQ(0, NumDiagnostics);
1736-
}
1737-
17381630
TEST_F(VFSFromYAMLTest, CaseInsensitive) {
17391631
IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
17401632
Lower->addRegularFile("//root/foo/bar/a");

0 commit comments

Comments
 (0)