Author: Keith Smiley Date: 2021-11-13T09:34:44-08:00 New Revision: c972175649f4bb50d40d911659a04d5620ce6fe0
URL: https://github.com/llvm/llvm-project/commit/c972175649f4bb50d40d911659a04d5620ce6fe0 DIFF: https://github.com/llvm/llvm-project/commit/c972175649f4bb50d40d911659a04d5620ce6fe0.diff LOG: [VFS] Use original path when falling back to external FS This is a follow up to 0be9ca7c0f9a733f846bb6bc4e8e36d46b518162 to make paths in the case of falling back to the external file system use the original format, preserving relative paths, and allow the external filesystem to canonicalize them if needed. Reviewed By: dexonsmith Differential Revision: https://reviews.llvm.org/D109128 Added: clang/test/VFS/relative-path-errors.c Modified: llvm/include/llvm/Support/VirtualFileSystem.h llvm/lib/Support/VirtualFileSystem.cpp llvm/unittests/Support/VirtualFileSystemTest.cpp Removed: ################################################################################ diff --git a/clang/test/VFS/relative-path-errors.c b/clang/test/VFS/relative-path-errors.c new file mode 100644 index 000000000000..8d3773a5eb59 --- /dev/null +++ b/clang/test/VFS/relative-path-errors.c @@ -0,0 +1,11 @@ +// 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 38f426ff0443..10d2389ee079 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -121,6 +121,14 @@ 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. @@ -757,6 +765,12 @@ 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 @@ -814,7 +828,8 @@ class RedirectingFileSystem : public vfs::FileSystem { 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 diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp index a4abfe19bcbd..d0ba3114aa5c 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -194,6 +194,7 @@ class RealFile : public File { bool RequiresNullTerminator, bool IsVolatile) override; std::error_code close() override; + void setPath(const Twine &Path) override; }; } // namespace @@ -229,6 +230,12 @@ 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. @@ -639,6 +646,8 @@ class InMemoryFileAdaptor : public File { } std::error_code close() override { return {}; } + + void setPath(const Twine &Path) override { RequestedName = Path.str(); } }; } // namespace @@ -1244,7 +1253,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, *Result); + ErrorOr<Status> S = status(Path, Dir, *Result); if (!S) { if (shouldFallBackToExternalFS(S.getError(), Result->E)) return ExternalFS->dir_begin(Dir, EC); @@ -1971,47 +1980,68 @@ RedirectingFileSystem::lookupPathImpl( 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); + SmallString<256> CanonicalRemappedPath((*ExtRedirect).str()); + if (std::error_code EC = makeCanonical(CanonicalRemappedPath)) + return EC; + + ErrorOr<Status> S = ExternalFS->status(CanonicalRemappedPath); if (!S) return S; + S = Status::copyWithNewName(*S, *ExtRedirect); 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(); + } +} - if (std::error_code EC = makeCanonical(Path)) +ErrorOr<Status> RedirectingFileSystem::status(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->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; } @@ -2036,22 +2066,39 @@ 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>> -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(); } @@ -2059,12 +2106,18 @@ RedirectingFileSystem::openFileForRead(const Twine &Path_) { 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 = ExternalFS->openFileForRead(ExtRedirect); + auto ExternalFile = File::getWithPath( + ExternalFS->openFileForRead(CanonicalRemappedPath), ExtRedirect); if (!ExternalFile) { if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E)) - return ExternalFS->openFileForRead(Path); + return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath), + OriginalPath); return ExternalFile; } @@ -2074,7 +2127,7 @@ RedirectingFileSystem::openFileForRead(const Twine &Path_) { // 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)); } diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp index ca35b5e9f8ba..be32971908ea 100644 --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -1627,6 +1627,114 @@ 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"); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits