Index: clang/lib/Frontend/CompilerInvocation.cpp =================================================================== --- clang/lib/Frontend/CompilerInvocation.cpp +++ clang/lib/Frontend/CompilerInvocation.cpp @@ -3295,25 +3295,27 @@ if (CI.getHeaderSearchOpts().VFSOverlayFiles.empty()) return BaseFS; - IntrusiveRefCntPtr Overlay( - new llvm::vfs::OverlayFileSystem(BaseFS)); + IntrusiveRefCntPtr Result = BaseFS; // earlier vfs files are on the bottom for (const auto &File : CI.getHeaderSearchOpts().VFSOverlayFiles) { llvm::ErrorOr> Buffer = - BaseFS->getBufferForFile(File); + Result->getBufferForFile(File); if (!Buffer) { Diags.Report(diag::err_missing_vfs_overlay_file) << File; continue; } IntrusiveRefCntPtr FS = llvm::vfs::getVFSFromYAML( - std::move(Buffer.get()), /*DiagHandler*/ nullptr, File); - if (FS) - Overlay->pushOverlay(FS); - else + std::move(Buffer.get()), /*DiagHandler*/ nullptr, File, + /*DiagContext*/ nullptr, Result); + if (!FS) { Diags.Report(diag::err_invalid_vfs_overlay) << File; + continue; + } + + Result = FS; } - return Overlay; + return Result; } } // namespace clang Index: clang/test/VFS/Inputs/Broken.framework/Headers/Error.h =================================================================== --- /dev/null +++ clang/test/VFS/Inputs/Broken.framework/Headers/Error.h @@ -0,0 +1,3 @@ +// Error.h + +#error Should not include this header in a module Index: clang/test/VFS/Inputs/Broken.framework/Modules/module.modulemap =================================================================== --- /dev/null +++ clang/test/VFS/Inputs/Broken.framework/Modules/module.modulemap @@ -0,0 +1,6 @@ +framework module Broken [extern_c] { + umbrella "Headers" + export * + module * { export * } +} + Index: clang/test/VFS/Inputs/Broken.framework/VFSHeaders/A.h =================================================================== --- /dev/null +++ clang/test/VFS/Inputs/Broken.framework/VFSHeaders/A.h @@ -0,0 +1 @@ +// A.h Index: clang/test/VFS/Inputs/vfsroot.yaml =================================================================== --- /dev/null +++ clang/test/VFS/Inputs/vfsroot.yaml @@ -0,0 +1,55 @@ +{ + 'version': 0, + 'use-external-names': false, + 'fallthrough': false, + 'roots': [ + { 'name': '/tests', 'type': 'directory', + 'contents': [ + { 'name': 'vfsroot-include.c', 'type': 'file', + 'external-contents': 'TEST_DIR/vfsroot-include.c' + }, + { 'name': 'vfsroot-with-overlay.c', 'type': 'file', + 'external-contents': 'TEST_DIR/vfsroot-with-overlay.c' + }, + { 'name': 'vfsroot-module.m', 'type': 'file', + 'external-contents': 'TEST_DIR/vfsroot-module.m' + } + ] + }, + { 'name': '/direct-vfs-root-files', 'type': 'directory', + 'contents': [ + { 'name': 'not_real.h', 'type': 'file', + 'external-contents': 'TEST_DIR/Inputs/actual_header.h' + }, + { 'name': 'vfsoverlay.yaml', 'type': 'file', + 'external-contents': 'OUT_DIR/vfsoverlay.yaml' + } + ] + }, + { 'name': '/indirect-vfs-root-files', 'type': 'directory', + 'contents': [ + { 'name': 'actual_header.h', 'type': 'file', + 'external-contents': 'TEST_DIR/Inputs/actual_header.h' + } + ] + }, + { 'name': 'TEST_DIR/Inputs/Broken.framework', 'type': 'directory', + 'contents': [ + { 'name': 'Headers/A.h', 'type': 'file', + 'external-contents': 'TEST_DIR/Inputs/Broken.framework/VFSHeaders/A.h' + }, + { 'name': 'Modules/module.modulemap', 'type': 'file', + 'external-contents': 'TEST_DIR/Inputs/Broken.framework/Modules/module.modulemap' + } + ] + }, + # Locations for modules. + { 'name': 'OUT_DIR/cache', 'type': 'directory', + 'contents': [ + { 'name': 'Broken.pcm', 'type': 'file', + 'external-contents': 'OUT_DIR/cache/Broken.pcm' + } + ] + } + ] +} Index: clang/test/VFS/vfsroot-include.c =================================================================== --- /dev/null +++ clang/test/VFS/vfsroot-include.c @@ -0,0 +1,17 @@ +// REQUIRES: shell +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: sed -e "s:TEST_DIR:%S:g" -e "s:OUT_DIR:%t:g" %S/Inputs/vfsroot.yaml > %t.yaml +// RUN: not %clang_cc1 -Werror -ivfsoverlay %t.yaml -I %S/Inputs -I /direct-vfs-root-files -fsyntax-only /tests/vfsroot-include.c 2>&1 | FileCheck %s +// The line above tests that the compiler input file is looked up through VFS. + +// Test successful include through the VFS. +#include "not_real.h" + +// Test that a file missing from the VFS root is not found, even if it is +// discoverable through the real file system. Fatal error should be the last +// in the file as it hides other errors. +#include "actual_header.h" +// CHECK: fatal error: 'actual_header.h' file not found +// CHECK: 1 error generated. +// CHECK-NOT: error Index: clang/test/VFS/vfsroot-module.m =================================================================== --- /dev/null +++ clang/test/VFS/vfsroot-module.m @@ -0,0 +1,10 @@ +// REQUIRES: shell +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: sed -e "s:TEST_DIR:%S:g" -e "s:OUT_DIR:%t:g" %S/Inputs/vfsroot.yaml > %t.yaml +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/cache -ivfsoverlay %t.yaml -F %S/Inputs -fsyntax-only /tests/vfsroot-module.m + +// Test that a file missing from the VFS root is not found, even if it is +// discoverable through the real file system at location that is a part of +// the framework. +@import Broken; Index: clang/test/VFS/vfsroot-with-overlay.c =================================================================== --- /dev/null +++ clang/test/VFS/vfsroot-with-overlay.c @@ -0,0 +1,12 @@ +// REQUIRES: shell +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: sed -e "s:TEST_DIR:%S:g" -e "s:OUT_DIR:%t:g" %S/Inputs/vfsroot.yaml > %t.yaml +// RUN: sed -e "s:INPUT_DIR:/indirect-vfs-root-files:g" -e "s:OUT_DIR:/overlay-dir:g" %S/Inputs/vfsoverlay.yaml > %t/vfsoverlay.yaml +// RUN: %clang_cc1 -Werror -ivfsoverlay %t.yaml -ivfsoverlay /direct-vfs-root-files/vfsoverlay.yaml -I /overlay-dir -fsyntax-only /tests/vfsroot-with-overlay.c + +#include "not_real.h" + +void foo() { + bar(); +} Index: llvm/lib/Support/VirtualFileSystem.cpp =================================================================== --- llvm/lib/Support/VirtualFileSystem.cpp +++ llvm/lib/Support/VirtualFileSystem.cpp @@ -993,16 +993,44 @@ static bool classof(const Entry *E) { return E->getKind() == EK_File; } }; +// FIXME: reuse implementation common with OverlayFSDirIterImpl as these +// iterators are conceptually similar. class VFSFromYamlDirIterImpl : public llvm::vfs::detail::DirIterImpl { std::string Dir; RedirectingDirectoryEntry::iterator Current, End; - std::error_code incrementImpl(); + // To handle 'fallthrough' mode we need to iterate at first through + // RedirectingDirectoryEntry and then through ExternalFS. These operations are + // done sequentially, we just need to keep a track of what kind of iteration + // we are currently performing. + + /// Flag telling if we should iterate through ExternalFS or stop at the last + /// RedirectingDirectoryEntry::iterator. + bool IterateExternalFS; + /// Flag telling if we have switched to iterating through ExternalFS. + bool IsExternalFSCurrent = false; + FileSystem &ExternalFS; + directory_iterator ExternalDirIter; + llvm::StringSet<> SeenNames; + + /// To combine multiple iterations, different methods are responsible for + /// different iteration steps. + /// @{ + + /// Responsible for dispatching between RedirectingDirectoryEntry iteration + /// and ExternalFS iteration. + std::error_code incrementImpl(bool IsFirstTime); + /// Responsible for RedirectingDirectoryEntry iteration. + std::error_code incrementContent(bool IsFirstTime); + /// Responsible for ExternalFS iteration. + std::error_code incrementExternal(); + /// @} public: VFSFromYamlDirIterImpl(const Twine &Path, RedirectingDirectoryEntry::iterator Begin, RedirectingDirectoryEntry::iterator End, + bool IterateExternalFS, FileSystem &ExternalFS, std::error_code &EC); std::error_code increment() override; @@ -1028,6 +1056,7 @@ /// 'case-sensitive': /// 'use-external-names': /// 'overlay-relative': +/// 'fallthrough': /// /// Virtual directories are represented as /// \verbatim @@ -1091,6 +1120,10 @@ /// Whether to use to use the value of 'external-contents' for the /// names of files. This global value is overridable on a per-file basis. bool UseExternalNames = true; + + /// Whether to attempt a file lookup in external file system after it wasn't + /// found in VFS. + bool IsFallthrough = true; /// @} /// Virtual file paths and external files could be canonicalized without "..", @@ -1141,6 +1174,8 @@ ErrorOr E = lookupPath(Dir); if (!E) { EC = E.getError(); + if (IsFallthrough && EC == errc::no_such_file_or_directory) + return ExternalFS->dir_begin(Dir, EC); return {}; } ErrorOr S = status(Dir, *E); @@ -1156,7 +1191,8 @@ auto *D = cast(*E); return directory_iterator(std::make_shared( - Dir, D->contents_begin(), D->contents_end(), EC)); + Dir, D->contents_begin(), D->contents_end(), + /*IterateExternalFS=*/IsFallthrough, *ExternalFS, EC)); } void setExternalContentsPrefixDir(StringRef PrefixDir) { @@ -1538,6 +1574,7 @@ KeyStatusPair("case-sensitive", false), KeyStatusPair("use-external-names", false), KeyStatusPair("overlay-relative", false), + KeyStatusPair("fallthrough", false), KeyStatusPair("roots", true), }; @@ -1595,6 +1632,9 @@ } else if (Key == "use-external-names") { if (!parseScalarBool(I.getValue(), FS->UseExternalNames)) return false; + } else if (Key == "fallthrough") { + if (!parseScalarBool(I.getValue(), FS->IsFallthrough)) + return false; } else { llvm_unreachable("key missing from Keys"); } @@ -1760,8 +1800,13 @@ ErrorOr RedirectingFileSystem::status(const Twine &Path) { ErrorOr Result = lookupPath(Path); - if (!Result) + if (!Result) { + if (IsFallthrough && + Result.getError() == llvm::errc::no_such_file_or_directory) { + return ExternalFS->status(Path); + } return Result.getError(); + } return status(Path, *Result); } @@ -1793,8 +1838,13 @@ ErrorOr> RedirectingFileSystem::openFileForRead(const Twine &Path) { ErrorOr E = lookupPath(Path); - if (!E) + if (!E) { + if (IsFallthrough && + E.getError() == llvm::errc::no_such_file_or_directory) { + return ExternalFS->openFileForRead(Path); + } return E.getError(); + } auto *F = dyn_cast(*E); if (!F) // FIXME: errc::not_a_file? @@ -2035,18 +2085,42 @@ VFSFromYamlDirIterImpl::VFSFromYamlDirIterImpl( const Twine &_Path, RedirectingDirectoryEntry::iterator Begin, - RedirectingDirectoryEntry::iterator End, std::error_code &EC) - : Dir(_Path.str()), Current(Begin), End(End) { - EC = incrementImpl(); + RedirectingDirectoryEntry::iterator End, bool IterateExternalFS, + FileSystem &ExternalFS, std::error_code &EC) + : Dir(_Path.str()), Current(Begin), End(End), + IterateExternalFS(IterateExternalFS), ExternalFS(ExternalFS) { + EC = incrementImpl(/*IsFirstTime=*/true); } std::error_code VFSFromYamlDirIterImpl::increment() { - assert(Current != End && "cannot iterate past end"); - ++Current; - return incrementImpl(); + return incrementImpl(/*IsFirstTime=*/false); +} + +std::error_code VFSFromYamlDirIterImpl::incrementExternal() { + assert(!(IsExternalFSCurrent && ExternalDirIter == directory_iterator()) && + "incrementing past end"); + std::error_code EC; + if (IsExternalFSCurrent) { + ExternalDirIter.increment(EC); + } else if (IterateExternalFS) { + ExternalDirIter = ExternalFS.dir_begin(Dir, EC); + IsExternalFSCurrent = true; + if (EC && EC != errc::no_such_file_or_directory) + return EC; + EC = {}; + } + if (EC || ExternalDirIter == directory_iterator()) { + CurrentEntry = directory_entry(); + } else { + CurrentEntry = *ExternalDirIter; + } + return EC; } -std::error_code VFSFromYamlDirIterImpl::incrementImpl() { +std::error_code VFSFromYamlDirIterImpl::incrementContent(bool IsFirstTime) { + assert(IsFirstTime || Current != End && "cannot iterate past end"); + if (!IsFirstTime) + ++Current; while (Current != End) { SmallString<128> PathStr(Dir); llvm::sys::path::append(PathStr, (*Current)->getName()); @@ -2060,12 +2134,22 @@ break; } CurrentEntry = directory_entry(PathStr.str(), Type); - break; + return {}; } + return incrementExternal(); +} - if (Current == End) - CurrentEntry = directory_entry(); - return {}; +std::error_code VFSFromYamlDirIterImpl::incrementImpl(bool IsFirstTime) { + while (true) { + std::error_code EC = IsExternalFSCurrent ? incrementExternal() + : incrementContent(IsFirstTime); + if (EC || CurrentEntry.path().empty()) + return EC; + StringRef Name = llvm::sys::path::filename(CurrentEntry.path()); + if (SeenNames.insert(Name).second) + return EC; // name not seen before + } + llvm_unreachable("returned above"); } vfs::recursive_directory_iterator::recursive_directory_iterator( Index: llvm/unittests/Support/VirtualFileSystemTest.cpp =================================================================== --- llvm/unittests/Support/VirtualFileSystemTest.cpp +++ llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -1599,3 +1599,89 @@ EXPECT_EQ(3, NumDiagnostics); } + +TEST_F(VFSFromYAMLTest, NonFallthroughDirectoryIteration) { + IntrusiveRefCntPtr Lower(new DummyFileSystem()); + Lower->addDirectory("//root/"); + Lower->addRegularFile("//root/a"); + Lower->addRegularFile("//root/b"); + IntrusiveRefCntPtr FS = getFromYAMLString( + "{ 'use-external-names': false,\n" + " 'fallthrough': false,\n" + " 'roots': [\n" + "{\n" + " 'type': 'directory',\n" + " 'name': '//root/',\n" + " 'contents': [ {\n" + " 'type': 'file',\n" + " 'name': 'c',\n" + " 'external-contents': '//root/a'\n" + " }\n" + " ]\n" + "}\n" + "]\n" + "}", + Lower); + ASSERT_TRUE(FS.get() != nullptr); + + std::error_code EC; + checkContents(FS->dir_begin("//root/", EC), + {"//root/c"}); +} + +TEST_F(VFSFromYAMLTest, DirectoryIterationWithDuplicates) { + IntrusiveRefCntPtr Lower(new DummyFileSystem()); + Lower->addDirectory("//root/"); + Lower->addRegularFile("//root/a"); + Lower->addRegularFile("//root/b"); + IntrusiveRefCntPtr FS = getFromYAMLString( + "{ 'use-external-names': false,\n" + " 'roots': [\n" + "{\n" + " 'type': 'directory',\n" + " 'name': '//root/',\n" + " 'contents': [ {\n" + " 'type': 'file',\n" + " 'name': 'a',\n" + " 'external-contents': '//root/a'\n" + " }\n" + " ]\n" + "}\n" + "]\n" + "}", + Lower); + ASSERT_TRUE(FS.get() != nullptr); + + std::error_code EC; + checkContents(FS->dir_begin("//root/", EC), + {"//root/a", "//root/b"}); +} + +TEST_F(VFSFromYAMLTest, DirectoryIterationErrorInVFSLayer) { + IntrusiveRefCntPtr Lower(new DummyFileSystem()); + Lower->addDirectory("//root/"); + Lower->addDirectory("//root/foo"); + Lower->addRegularFile("//root/foo/a"); + Lower->addRegularFile("//root/foo/b"); + IntrusiveRefCntPtr FS = getFromYAMLString( + "{ 'use-external-names': false,\n" + " 'roots': [\n" + "{\n" + " 'type': 'directory',\n" + " 'name': '//root/',\n" + " 'contents': [ {\n" + " 'type': 'file',\n" + " 'name': 'bar/a',\n" + " 'external-contents': '//root/foo/a'\n" + " }\n" + " ]\n" + "}\n" + "]\n" + "}", + Lower); + ASSERT_TRUE(FS.get() != nullptr); + + std::error_code EC; + checkContents(FS->dir_begin("//root/foo", EC), + {"//root/foo/a", "//root/foo/b"}); +}