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"); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits