diff --git a/clang/test/VFS/relative-path-errors.c b/clang/test/VFS/relative-path-errors.c new file mode 100644 --- /dev/null +++ b/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 diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/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> + getWithPath(ErrorOr> 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. @@ -819,7 +827,8 @@ Entry *From) const; /// Get the status for a path with the provided \c LookupResult. - ErrorOr status(const Twine &Path, const LookupResult &Result); + ErrorOr 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 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/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 S = status(Path, *Result); + ErrorOr S = status(Path, Dir, *Result); if (!S) { if (shouldFallBackToExternalFS(S.getError(), Result->E)) return ExternalFS->dir_begin(Dir, EC); @@ -1959,47 +1971,59 @@ 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 RedirectingFileSystem::status( - const Twine &Path, const RedirectingFileSystem::LookupResult &Result) { + const Twine &CanonicalPath, const Twine &OriginalPath, + const RedirectingFileSystem::LookupResult &Result) { if (Optional ExtRedirect = Result.getExternalRedirect()) { ErrorOr S = ExternalFS->status(*ExtRedirect); if (!S) return S; auto *RE = cast(Result.E); - return getRedirectedFileStatus(Path, RE->useExternalName(UseExternalNames), - *S); + return getRedirectedFileStatus(OriginalPath, + RE->useExternalName(UseExternalNames), *S); } auto *DE = cast(Result.E); - return Status::copyWithNewName(DE->getStatus(), Path); + return Status::copyWithNewName(DE->getStatus(), CanonicalPath); } -ErrorOr RedirectingFileSystem::status(const Twine &Path_) { - SmallString<256> Path; - Path_.toVector(Path); +ErrorOr 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 Result = lookupPath(Path); + ErrorOr Result = + lookupPath(CanonicalPath); if (!Result) { - if (shouldFallBackToExternalFS(Result.getError())) - return ExternalFS->status(Path); + if (shouldFallBackToExternalFS(Result.getError())) { + if (auto Result = ExternalFS->status(CanonicalPath)) { + return Result.get().copyWithNewName(Result.get(), OriginalPath); + } else { + return Result.getError(); + } + } return Result.getError(); } - ErrorOr S = status(Path, *Result); - if (!S && shouldFallBackToExternalFS(S.getError(), Result->E)) - S = ExternalFS->status(Path); + ErrorOr S = status(CanonicalPath, OriginalPath, *Result); + if (!S && shouldFallBackToExternalFS(S.getError(), Result->E)) { + S = ExternalFS->status(CanonicalPath); + if (S) + S = S.get().copyWithNewName(S.get(), OriginalPath); + } + return S; } @@ -2024,22 +2048,39 @@ } std::error_code close() override { return InnerFile->close(); } + + void setPath(const Twine &Path) override { S = S.copyWithNewName(S, Path); } }; } // namespace ErrorOr> -RedirectingFileSystem::openFileForRead(const Twine &Path_) { - SmallString<256> Path; - Path_.toVector(Path); +File::getWithPath(ErrorOr> 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> +RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) { + SmallString<256> CanonicalPath; + OriginalPath.toVector(CanonicalPath); + + if (std::error_code EC = makeCanonical(CanonicalPath)) return EC; - ErrorOr Result = lookupPath(Path); + ErrorOr 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 +2093,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 +2104,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( std::make_unique(std::move(*ExternalFile), S)); } diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -1627,6 +1627,114 @@ EXPECT_EQ(0, NumDiagnostics); } +TEST_F(VFSFromYAMLTest, ReturnsRequestedPathVFSMiss) { + IntrusiveRefCntPtr 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 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 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 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 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 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 Lower(new DummyFileSystem()); Lower->addRegularFile("//root/foo/bar/a");