keith updated this revision to Diff 372041.
keith added a comment.
Handle all cases by passing relative path to ExternalFS first
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/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
@@ -1607,6 +1607,26 @@
EXPECT_EQ(0, NumDiagnostics);
}
+TEST_F(VFSFromYAMLTest, RelativePathFallthrough) {
+ 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());
+ auto OpenedS = (*OpenedF)->status();
+ ASSERT_FALSE(OpenedS.getError());
+ EXPECT_EQ("a", Name.get());
+ EXPECT_EQ("a", OpenedS->getName());
+ EXPECT_FALSE(OpenedS->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
@@ -1141,6 +1141,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))
@@ -1149,15 +1152,17 @@
return {};
}
-std::error_code RedirectingFileSystem::isLocal(const Twine &Path_,
+std::error_code RedirectingFileSystem::isLocal(const Twine &Path,
bool &Result) {
- SmallString<256> Path;
- Path_.toVector(Path);
+ if (!ExternalFS->isLocal(Path, Result))
+ return {};
- if (std::error_code EC = makeCanonical(Path))
+ SmallString<256> CanonicalPath;
+ Path.toVector(CanonicalPath);
+ if (std::error_code EC = makeCanonical(CanonicalPath))
return {};
- return ExternalFS->isLocal(Path, Result);
+ return ExternalFS->isLocal(CanonicalPath, Result);
}
std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl<char> &Path) const {
@@ -1191,23 +1196,28 @@
directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
std::error_code &EC) {
- SmallString<256> Path;
- Dir.toVector(Path);
+ SmallString<256> CanonicalPath;
+ Dir.toVector(CanonicalPath);
- EC = makeCanonical(Path);
+ EC = makeCanonical(CanonicalPath);
if (EC)
return {};
- ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(Path);
+ ErrorOr<RedirectingFileSystem::LookupResult> Result = lookupPath(CanonicalPath);
if (!Result) {
EC = Result.getError();
- if (shouldFallBackToExternalFS(EC))
- return ExternalFS->dir_begin(Path, EC);
+ if (shouldFallBackToExternalFS(EC)) {
+ directory_iterator Result = ExternalFS->dir_begin(Dir, EC);
+ if (!EC)
+ return Result;
+
+ return ExternalFS->dir_begin(CanonicalPath, EC);
+ }
return {};
}
// Use status to make sure the path exists and refers to a directory.
- ErrorOr<Status> S = status(Path, *Result);
+ ErrorOr<Status> S = status(CanonicalPath, *Result);
if (!S) {
if (shouldFallBackToExternalFS(S.getError(), Result->E))
return ExternalFS->dir_begin(Dir, EC);
@@ -1231,18 +1241,18 @@
// Update the paths in the results to use the virtual directory's path.
DirIter =
directory_iterator(std::make_shared<RedirectingFSDirRemapIterImpl>(
- std::string(Path), DirIter));
+ std::string(CanonicalPath), DirIter));
}
} else {
auto DE = cast<DirectoryEntry>(Result->E);
DirIter = directory_iterator(std::make_shared<RedirectingFSDirIterImpl>(
- Path, DE->contents_begin(), DE->contents_end(), EC));
+ CanonicalPath, DE->contents_begin(), DE->contents_end(), EC));
}
if (!shouldUseExternalFS())
return DirIter;
return directory_iterator(std::make_shared<CombiningDirIterImpl>(
- DirIter, ExternalFS, std::string(Path), EC));
+ DirIter, ExternalFS, std::string(CanonicalPath), EC));
}
void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) {
@@ -1957,23 +1967,33 @@
return Status::copyWithNewName(DE->getStatus(), Path);
}
-ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path_) {
- SmallString<256> Path;
- Path_.toVector(Path);
+ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path) {
+ SmallString<256> CanonicalPath;
+ Path.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())) {
+ auto Result = ExternalFS->status(Path);
+ if (!Result.getError())
+ return Result;
+
+ return ExternalFS->status(CanonicalPath);
+ }
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, *Result);
+ if (!S && shouldFallBackToExternalFS(S.getError(), Result->E)) {
+ auto Result = ExternalFS->status(Path);
+ if (!Result.getError())
+ return Result;
+
+ S = ExternalFS->status(CanonicalPath);
+ }
return S;
}
@@ -2003,17 +2023,22 @@
} // namespace
ErrorOr<std::unique_ptr<File>>
-RedirectingFileSystem::openFileForRead(const Twine &Path_) {
- SmallString<256> Path;
- Path_.toVector(Path);
+RedirectingFileSystem::openFileForRead(const Twine &Path) {
+ SmallString<256> CanonicalPath;
+ Path.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->openFileForRead(Path);
+ if (shouldFallBackToExternalFS(Result.getError())) {
+ auto Result = ExternalFS->openFileForRead(Path);
+ if (!Result.getError())
+ return Result;
+
+ return ExternalFS->openFileForRead(CanonicalPath);
+ }
return Result.getError();
}
@@ -2025,8 +2050,13 @@
auto ExternalFile = ExternalFS->openFileForRead(ExtRedirect);
if (!ExternalFile) {
- if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E))
- return ExternalFS->openFileForRead(Path);
+ if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E)) {
+ auto Result = ExternalFS->openFileForRead(Path);
+ if (!Result.getError())
+ return Result;
+
+ return ExternalFS->openFileForRead(CanonicalPath);
+ }
return ExternalFile;
}
@@ -2036,24 +2066,28 @@
// FIXME: Update the status with the name and VFSMapped.
Status S = getRedirectedFileStatus(
- Path, RE->useExternalName(UseExternalNames), *ExternalStatus);
+ CanonicalPath, RE->useExternalName(UseExternalNames), *ExternalStatus);
return std::unique_ptr<File>(
std::make_unique<FileWithFixedStatus>(std::move(*ExternalFile), S));
}
std::error_code
-RedirectingFileSystem::getRealPath(const Twine &Path_,
+RedirectingFileSystem::getRealPath(const Twine &Path,
SmallVectorImpl<char> &Output) const {
- SmallString<256> Path;
- Path_.toVector(Path);
+ SmallString<256> CanonicalPath;
+ Path.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->getRealPath(Path, Output);
+ if (shouldFallBackToExternalFS(Result.getError())) {
+ if (!ExternalFS->getRealPath(Path, Output))
+ return {};
+
+ return ExternalFS->getRealPath(CanonicalPath, Output);
+ }
return Result.getError();
}
@@ -2062,15 +2096,23 @@
if (auto ExtRedirect = Result->getExternalRedirect()) {
auto P = ExternalFS->getRealPath(*ExtRedirect, Output);
if (!P && shouldFallBackToExternalFS(P, Result->E)) {
- return ExternalFS->getRealPath(Path, Output);
+ if (!ExternalFS->getRealPath(Path, Output))
+ return {};
+
+ return ExternalFS->getRealPath(CanonicalPath, Output);
}
return P;
}
// If we found a DirectoryEntry, still fall back to ExternalFS if allowed,
// because directories don't have a single external contents path.
- return shouldUseExternalFS() ? ExternalFS->getRealPath(Path, Output)
- : llvm::errc::invalid_argument;
+ if (shouldUseExternalFS()) {
+ if (!ExternalFS->getRealPath(Path, Output))
+ return {};
+ return ExternalFS->getRealPath(CanonicalPath, Output);
+ }
+
+ return llvm::errc::invalid_argument;
}
std::unique_ptr<FileSystem>
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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
