diff --git a/clang/test/VFS/directory.c b/clang/test/VFS/directory.c --- a/clang/test/VFS/directory.c +++ b/clang/test/VFS/directory.c @@ -17,7 +17,9 @@ // RUN: sed -e "s@INPUT_DIR@Overlay@g" -e "s@OUT_DIR@%{/t:regex_replacement}/Underlying@g" %S/Inputs/vfsoverlay-directory-relative.yaml > %t/vfs-relative.yaml // RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs-relative.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=DIRECT %s +// DIRECT: {{^}}# 1 "{{.*(/|\\\\)Underlying(/|\\\\)}}B.h" // DIRECT: {{^}}// B.h in Underlying +// DIRECT: {{^}}# 1 "{{.*(/|\\\\)Overlay(/|\\\\)}}C.h" // DIRECT: {{^}}// C.h in Overlay // 2) Underlying -> Middle -> Overlay (C.h found, B.h falling back to Underlying) @@ -32,14 +34,18 @@ // RUN: rm -f %t/Overlay/C.h // RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=FALLBACK %s +// FALLBACK: {{^}}# 1 "{{.*(/|\\\\)Underlying(/|\\\\)}}B.h" // FALLBACK: {{^}}// B.h in Underlying +// FALLBACK: {{^}}# 1 "{{.*(/|\\\\)Middle(/|\\\\)}}C.h" // FALLBACK: {{^}}// C.h in Middle // 3) Underlying -> Middle -> Overlay (C.h falling back to Underlying, B.h falling back to Underlying) // RUN: rm -f %t/Middle/C.h // RUN: %clang_cc1 -Werror -I %t/Underlying -ivfsoverlay %t/vfs.yaml -ivfsoverlay %t/vfs2.yaml -fsyntax-only -E -C %s 2>&1 | FileCheck --check-prefix=FALLBACK2 %s +// FALLBACK2: {{^}}# 1 "{{.*(/|\\\\)Underlying(/|\\\\)}}B.h" // FALLBACK2: {{^}}// B.h in Underlying +// FALLBACK2: {{^}}# 1 "{{.*(/|\\\\)Underlying(/|\\\\)}}C.h" // FALLBACK2: {{^}}// C.h in Underlying #include "B.h" 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 @@ -260,8 +260,17 @@ virtual llvm::ErrorOr status(const Twine &Path) = 0; /// Get a \p File object for the file at \p Path, if one exists. + llvm::ErrorOr> openFileForRead(const Twine &Path) { + return openFileForReadImpl(Path, Path); + } + + /// Get a \p File object for the file at \p LookupPath. In most cases the + /// filesystem should ensure that the returned file has a path of + /// \p OriginalPath, but this is not a guarantee. In some cases the path will + /// have to differ, eg. if it has been remapped and is intentionally being + /// leaked. virtual llvm::ErrorOr> - openFileForRead(const Twine &Path) = 0; + openFileForReadImpl(const Twine &LookupPath, const Twine &OriginalPath) = 0; /// This is a convenience method that opens a file, gets its content and then /// closes the file. @@ -343,8 +352,9 @@ void pushOverlay(IntrusiveRefCntPtr FS); llvm::ErrorOr status(const Twine &Path) override; - llvm::ErrorOr> - openFileForRead(const Twine &Path) override; + llvm::ErrorOr> ErrorOr> + openFileForReadImpl(const Twine &LookupPath, + const Twine &OriginalPath) override; directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override; llvm::ErrorOr getCurrentWorkingDirectory() const override; std::error_code setCurrentWorkingDirectory(const Twine &Path) override; @@ -386,8 +396,9 @@ return FS->status(Path); } llvm::ErrorOr> - openFileForRead(const Twine &Path) override { - return FS->openFileForRead(Path); + openFileForReadImpl(const Twine &LookupPath, + const Twine &OriginalPath) override { + return FS->openFileForReadImpl(LookupPath, OriginalPath); } directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override { return FS->dir_begin(Dir, EC); @@ -487,7 +498,8 @@ llvm::ErrorOr status(const Twine &Path) override; llvm::ErrorOr> - openFileForRead(const Twine &Path) override; + openFileForReadImpl(const Twine &LookupPath, + const Twine &OriginalPath) override; directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override; llvm::ErrorOr getCurrentWorkingDirectory() const override { @@ -855,7 +867,9 @@ bool UseExternalNames, FileSystem &ExternalFS); ErrorOr status(const Twine &Path) override; - ErrorOr> openFileForRead(const Twine &Path) override; + llvm::ErrorOr> + openFileForReadImpl(const Twine &LookupPath, + const Twine &OriginalPath) override; std::error_code getRealPath(const Twine &Path, SmallVectorImpl &Output) const override; diff --git a/llvm/lib/Support/FileCollector.cpp b/llvm/lib/Support/FileCollector.cpp --- a/llvm/lib/Support/FileCollector.cpp +++ b/llvm/lib/Support/FileCollector.cpp @@ -266,10 +266,11 @@ } llvm::ErrorOr> - openFileForRead(const Twine &Path) override { - auto Result = FS->openFileForRead(Path); + openFileForReadImpl(const Twine &LookupPath, + const Twine &OriginalPath) override { + auto Result = FS->openFileForReadImpl(LookupPath, OriginalPath); if (Result && *Result) - Collector->addFile(Path); + Collector->addFile(LookupPath); return Result; } 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 @@ -267,7 +267,9 @@ } ErrorOr status(const Twine &Path) override; - ErrorOr> openFileForRead(const Twine &Path) override; + ErrorOr> + openFileForReadImpl(const Twine &LookupPath, + const Twine &OriginalPath) override; directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override; llvm::ErrorOr getCurrentWorkingDirectory() const override; @@ -308,14 +310,15 @@ } ErrorOr> -RealFileSystem::openFileForRead(const Twine &Name) { +RealFileSystem::openFileForReadImpl(const Twine &LookupPath, + const Twine &OriginalPath) { SmallString<256> RealName, Storage; Expected FDOrErr = sys::fs::openNativeFileForRead( - adjustPath(Name, Storage), sys::fs::OF_None, &RealName); + adjustPath(LookupPath, Storage), sys::fs::OF_None, &RealName); if (!FDOrErr) return errorToErrorCode(FDOrErr.takeError()); return std::unique_ptr( - new RealFile(*FDOrErr, Name.str(), RealName.str())); + new RealFile(*FDOrErr, OriginalPath.str(), RealName.str())); } llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const { @@ -422,10 +425,11 @@ } ErrorOr> -OverlayFileSystem::openFileForRead(const llvm::Twine &Path) { +OverlayFileSystem::openFileForReadImpl(const Twine &LookupPath, + const Twine &OriginalPath) { // FIXME: handle symlinks that cross file systems for (iterator I = overlays_begin(), E = overlays_end(); I != E; ++I) { - auto Result = (*I)->openFileForRead(Path); + auto Result = (*I)->openFileForReadImpl(LookupPath, OriginalPath); if (Result || Result.getError() != llvm::errc::no_such_file_or_directory) return Result; } @@ -928,8 +932,9 @@ } llvm::ErrorOr> -InMemoryFileSystem::openFileForRead(const Twine &Path) { - auto Node = lookupInMemoryNode(*this, Root.get(), Path); +InMemoryFileSystem::openFileForReadImpl(const Twine &LookupPath, + const Twine &OriginalPath) { + auto Node = lookupInMemoryNode(*this, Root.get(), LookupPath); if (!Node) return Node.getError(); @@ -937,7 +942,7 @@ // to match the ownership semantics for File. if (auto *F = dyn_cast(*Node)) return std::unique_ptr( - new detail::InMemoryFileAdaptor(*F, Path.str())); + new detail::InMemoryFileAdaptor(*F, OriginalPath.str())); // FIXME: errc::not_a_file? return make_error_code(llvm::errc::invalid_argument); @@ -2100,9 +2105,10 @@ } ErrorOr> -RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) { +RedirectingFileSystem::openFileForReadImpl(const Twine &LookupPath, + const Twine &OriginalPath) { SmallString<256> CanonicalPath; - OriginalPath.toVector(CanonicalPath); + LookupPath.toVector(CanonicalPath); if (std::error_code EC = makeCanonical(CanonicalPath)) return EC; @@ -2111,8 +2117,7 @@ lookupPath(CanonicalPath); if (!Result) { if (shouldFallBackToExternalFS(Result.getError())) - return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath), - OriginalPath); + return ExternalFS->openFileForReadImpl(CanonicalPath, OriginalPath); return Result.getError(); } @@ -2127,12 +2132,12 @@ auto *RE = cast(Result->E); - auto ExternalFile = File::getWithPath( - ExternalFS->openFileForRead(CanonicalRemappedPath), ExtRedirect); + auto ExternalFile = ExternalFS->openFileForReadImpl( + CanonicalRemappedPath, + RE->useExternalName(UseExternalNames) ? ExtRedirect : OriginalPath); if (!ExternalFile) { if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E)) - return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath), - OriginalPath); + return ExternalFS->openFileForReadImpl(CanonicalPath, OriginalPath); return ExternalFile; } @@ -2140,9 +2145,8 @@ if (!ExternalStatus) return ExternalStatus.getError(); - // FIXME: Update the status with the name and VFSMapped. - Status S = getRedirectedFileStatus( - OriginalPath, RE->useExternalName(UseExternalNames), *ExternalStatus); + Status S = *ExternalStatus; + S.IsVFSMapped = true; 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 @@ -64,10 +64,13 @@ return I->second; } ErrorOr> - openFileForRead(const Twine &Path) override { - auto S = status(Path); - if (S) - return std::unique_ptr(new DummyFile{*S}); + openFileForReadImpl(const Twine &LookupPath, + const Twine &OriginalPath) override { + auto S = status(LookupPath); + if (S) { + auto FixedS = vfs::Status::copyWithNewName(*S, OriginalPath); + return std::unique_ptr(new DummyFile{FixedS}); + } return S.getError(); } llvm::ErrorOr getCurrentWorkingDirectory() const override { @@ -2708,6 +2711,80 @@ EXPECT_FALSE(FS->exists(_c.path("c"))); } +TEST_F(VFSFromYAMLTest, MultipleRemappedDirectories) { + // Test the interaction of two overlays where one maps back to the other, + // ie. `a` -> `b` and then `b` -> `a`. This should always use `a` if it + // exists and fallback to `b` otherwise. + + IntrusiveRefCntPtr Both(new DummyFileSystem()); + Both->addDirectory("//root/a"); + Both->addRegularFile("//root/a/f"); + Both->addDirectory("//root/b"); + Both->addRegularFile("//root/b/f"); + + IntrusiveRefCntPtr AOnly(new DummyFileSystem()); + AOnly->addDirectory("//root/a"); + AOnly->addRegularFile("//root/a/f"); + + IntrusiveRefCntPtr BOnly(new DummyFileSystem()); + BOnly->addDirectory("//root/b"); + BOnly->addRegularFile("//root/b/f"); + + auto AToBStr = "{ 'roots': [\n" + " {\n" + " 'type': 'directory-remap',\n" + " 'name': '//root/a',\n" + " 'external-contents': '//root/b'\n" + " }" + "]}"; + auto BToAStr = "{ 'roots': [\n" + " {\n" + " 'type': 'directory-remap',\n" + " 'name': '//root/b',\n" + " 'external-contents': '//root/a'\n" + " }" + "]}"; + + auto ExpectPath = [&](vfs::FileSystem &FS, StringRef Expected) { + auto AF = FS.openFileForRead("//root/a/f"); + ASSERT_FALSE(AF.getError()); + auto AFName = (*AF)->getName(); + ASSERT_FALSE(AFName.getError()); + EXPECT_EQ(Expected.str(), AFName.get()); + + auto AS = FS.status("//root/a/f"); + ASSERT_FALSE(AS.getError()); + EXPECT_EQ(Expected.str(), AS->getName()); + + auto BF = FS.openFileForRead("//root/b/f"); + ASSERT_FALSE(BF.getError()); + auto BName = (*BF)->getName(); + ASSERT_FALSE(BName.getError()); + EXPECT_EQ(Expected.str(), BName.get()); + + auto BS = FS.status("//root/b/f"); + ASSERT_FALSE(BS.getError()); + EXPECT_EQ(Expected.str(), BS->getName()); + }; + + IntrusiveRefCntPtr BothFS = + getFromYAMLString(AToBStr, getFromYAMLString(BToAStr, Both)); + ASSERT_TRUE(BothFS.get() != nullptr); + ExpectPath(*BothFS, "//root/a/f"); + + IntrusiveRefCntPtr OnlyAFS = + getFromYAMLString(AToBStr, getFromYAMLString(BToAStr, AOnly)); + ASSERT_TRUE(OnlyAFS.get() != nullptr); + ExpectPath(*OnlyAFS, "//root/a/f"); + + IntrusiveRefCntPtr OnlyBFS = + getFromYAMLString(AToBStr, getFromYAMLString(BToAStr, BOnly)); + ASSERT_TRUE(OnlyBFS.get() != nullptr); + ExpectPath(*OnlyBFS, "//root/b/f"); + + EXPECT_EQ(0, NumDiagnostics); +} + TEST(VFSFromRemappedFilesTest, Basic) { IntrusiveRefCntPtr BaseFS = new vfs::InMemoryFileSystem;