vsapsai created this revision.
vsapsai added reviewers: bruno, benlangmuir.
Herald added a subscriber: dexonsmith.
Default property value 'true' preserves current behavior. Value 'false' can be
used to create VFS "root", file system that gives better control over which
files compiler can use during compilation as there are no unpredictable
accesses to real file system.
Non-fallthrough use case changes how we treat multiple VFS overlay
files. Instead of all of them being at the same level just above a real
file system, now they are nested and subsequent overlays can refer to
files in previous overlays.
rdar://problem/39465552
https://reviews.llvm.org/D50539
Files:
clang/lib/Basic/VirtualFileSystem.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/VFS/Inputs/Broken.framework/Headers/Error.h
clang/test/VFS/Inputs/Broken.framework/Modules/module.modulemap
clang/test/VFS/Inputs/Broken.framework/VFSHeaders/A.h
clang/test/VFS/Inputs/vfsroot.yaml
clang/test/VFS/vfsroot-include.c
clang/test/VFS/vfsroot-module.m
clang/test/VFS/vfsroot-with-overlay.c
clang/unittests/Basic/VirtualFileSystemTest.cpp
Index: clang/unittests/Basic/VirtualFileSystemTest.cpp
===================================================================
--- clang/unittests/Basic/VirtualFileSystemTest.cpp
+++ clang/unittests/Basic/VirtualFileSystemTest.cpp
@@ -1500,3 +1500,89 @@
EXPECT_EQ(3, NumDiagnostics);
}
+
+TEST_F(VFSFromYAMLTest, NonFallthroughDirectoryIteration) {
+ IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
+ Lower->addDirectory("//root/");
+ Lower->addRegularFile("//root/a");
+ Lower->addRegularFile("//root/b");
+ IntrusiveRefCntPtr<vfs::FileSystem> 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<DummyFileSystem> Lower(new DummyFileSystem());
+ Lower->addDirectory("//root/");
+ Lower->addRegularFile("//root/a");
+ Lower->addRegularFile("//root/b");
+ IntrusiveRefCntPtr<vfs::FileSystem> 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<DummyFileSystem> Lower(new DummyFileSystem());
+ Lower->addDirectory("//root/");
+ Lower->addDirectory("//root/foo");
+ Lower->addRegularFile("//root/foo/a");
+ Lower->addRegularFile("//root/foo/b");
+ IntrusiveRefCntPtr<vfs::FileSystem> 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"});
+}
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: 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-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/Inputs/vfsroot.yaml
===================================================================
--- /dev/null
+++ clang/test/VFS/Inputs/vfsroot.yaml
@@ -0,0 +1,56 @@
+{
+ 'version': 0,
+ 'ignore-non-existent-contents': false,
+ '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/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/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/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/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3222,25 +3222,27 @@
if (CI.getHeaderSearchOpts().VFSOverlayFiles.empty())
return BaseFS;
- IntrusiveRefCntPtr<vfs::OverlayFileSystem> Overlay(
- new vfs::OverlayFileSystem(BaseFS));
+ IntrusiveRefCntPtr<vfs::FileSystem> Result = BaseFS;
// earlier vfs files are on the bottom
for (const auto &File : CI.getHeaderSearchOpts().VFSOverlayFiles) {
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
- BaseFS->getBufferForFile(File);
+ Result->getBufferForFile(File);
if (!Buffer) {
Diags.Report(diag::err_missing_vfs_overlay_file) << File;
continue;
}
- IntrusiveRefCntPtr<vfs::FileSystem> FS = vfs::getVFSFromYAML(
- std::move(Buffer.get()), /*DiagHandler*/ nullptr, File);
- if (FS)
- Overlay->pushOverlay(FS);
- else
+ IntrusiveRefCntPtr<vfs::FileSystem> FS =
+ vfs::getVFSFromYAML(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/lib/Basic/VirtualFileSystem.cpp
===================================================================
--- clang/lib/Basic/VirtualFileSystem.cpp
+++ clang/lib/Basic/VirtualFileSystem.cpp
@@ -932,14 +932,20 @@
std::string Dir;
RedirectingFileSystem &FS;
RedirectingDirectoryEntry::iterator Current, End;
+ bool IsExternalFSCurrent;
+ FileSystem &ExternalFS;
+ directory_iterator ExternalDirIter;
+ llvm::StringSet<> SeenNames;
- std::error_code incrementImpl();
+ std::error_code incrementExternal();
+ std::error_code incrementContent(bool IsFirstTime);
+ std::error_code incrementImpl(bool IsFirstTime);
public:
VFSFromYamlDirIterImpl(const Twine &Path, RedirectingFileSystem &FS,
RedirectingDirectoryEntry::iterator Begin,
RedirectingDirectoryEntry::iterator End,
- std::error_code &EC);
+ FileSystem &ExternalFS, std::error_code &EC);
std::error_code increment() override;
};
@@ -965,6 +971,7 @@
/// 'use-external-names': <boolean, default=true>
/// 'overlay-relative': <boolean, default=false>
/// 'ignore-non-existent-contents': <boolean, default=true>
+/// 'fallthrough': <boolean, default=true>
///
/// Virtual directories are represented as
/// \verbatim
@@ -1036,6 +1043,10 @@
/// paths in 'external-contents'. This global value is overridable on a
/// per-file basis.
bool IgnoreNonExistentContents = 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 "..",
@@ -1063,6 +1074,7 @@
public:
/// Looks up \p Path in \c Roots.
ErrorOr<Entry *> lookupPath(const Twine &Path);
+ ErrorOr<Status> status(const Twine &Path, bool ShouldCheckExternalFS);
/// Parses \p Buffer, which is expected to be in YAML format and
/// returns a virtual file system representing its contents.
@@ -1082,10 +1094,12 @@
return ExternalFS->setCurrentWorkingDirectory(Path);
}
- directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override{
+ directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override {
ErrorOr<Entry *> 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<Status> S = status(Dir, *E);
@@ -1100,8 +1114,8 @@
}
auto *D = cast<RedirectingDirectoryEntry>(*E);
- return directory_iterator(std::make_shared<VFSFromYamlDirIterImpl>(Dir,
- *this, D->contents_begin(), D->contents_end(), EC));
+ return directory_iterator(std::make_shared<VFSFromYamlDirIterImpl>(
+ Dir, *this, D->contents_begin(), D->contents_end(), *ExternalFS, EC));
}
void setExternalContentsPrefixDir(StringRef PrefixDir) {
@@ -1116,6 +1130,10 @@
return IgnoreNonExistentContents;
}
+ bool isFallthrough() const {
+ return IsFallthrough;
+ }
+
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
LLVM_DUMP_METHOD void dump() const {
for (const auto &Root : Roots)
@@ -1489,6 +1507,7 @@
KeyStatusPair("use-external-names", false),
KeyStatusPair("overlay-relative", false),
KeyStatusPair("ignore-non-existent-contents", false),
+ KeyStatusPair("fallthrough", false),
KeyStatusPair("roots", true),
};
@@ -1549,6 +1568,9 @@
} else if (Key == "ignore-non-existent-contents") {
if (!parseScalarBool(I.getValue(), FS->IgnoreNonExistentContents))
return false;
+ } else if (Key == "fallthrough") {
+ if (!parseScalarBool(I.getValue(), FS->IsFallthrough))
+ return false;
} else {
llvm_unreachable("key missing from Keys");
}
@@ -1712,13 +1734,23 @@
}
}
-ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path) {
+ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path,
+ bool ShouldCheckExternalFS) {
ErrorOr<Entry *> Result = lookupPath(Path);
- if (!Result)
+ if (!Result) {
+ if (ShouldCheckExternalFS &&
+ Result.getError() == llvm::errc::no_such_file_or_directory) {
+ return ExternalFS->status(Path);
+ }
return Result.getError();
+ }
return status(Path, *Result);
}
+ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path) {
+ return status(Path, IsFallthrough);
+}
+
namespace {
/// Provide a file wrapper with an overriden status.
@@ -1747,8 +1779,13 @@
ErrorOr<std::unique_ptr<File>>
RedirectingFileSystem::openFileForRead(const Twine &Path) {
ErrorOr<Entry *> 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<RedirectingFileEntry>(*E);
if (!F) // FIXME: errc::not_a_file?
@@ -1997,22 +2034,46 @@
VFSFromYamlDirIterImpl::VFSFromYamlDirIterImpl(
const Twine &_Path, RedirectingFileSystem &FS,
RedirectingDirectoryEntry::iterator Begin,
- RedirectingDirectoryEntry::iterator End, std::error_code &EC)
- : Dir(_Path.str()), FS(FS), Current(Begin), End(End) {
- EC = incrementImpl();
+ RedirectingDirectoryEntry::iterator End, FileSystem &ExternalFS,
+ std::error_code &EC)
+ : Dir(_Path.str()), FS(FS), Current(Begin), End(End),
+ IsExternalFSCurrent(false), ExternalFS(ExternalFS) {
+ EC = incrementImpl(true);
}
std::error_code VFSFromYamlDirIterImpl::increment() {
- assert(Current != End && "cannot iterate past end");
- ++Current;
- return incrementImpl();
+ return incrementImpl(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 (FS.isFallthrough()) {
+ 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 = Status();
+ } 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());
- llvm::ErrorOr<vfs::Status> S = FS.status(PathStr);
+ llvm::ErrorOr<vfs::Status> S = FS.status(PathStr, false);
if (!S) {
// Skip entries which do not map to a reliable external content.
if (FS.ignoreNonExistentContents() &&
@@ -2024,12 +2085,22 @@
}
}
CurrentEntry = *S;
- break;
+ return {};
}
+ return incrementExternal();
+}
- if (Current == End)
- CurrentEntry = Status();
- return {};
+std::error_code VFSFromYamlDirIterImpl::incrementImpl(bool IsFirstTime) {
+ while (true) {
+ std::error_code EC = IsExternalFSCurrent ? incrementExternal()
+ : incrementContent(IsFirstTime);
+ if (EC || !CurrentEntry.isStatusKnown())
+ return EC;
+ StringRef Name = llvm::sys::path::filename(CurrentEntry.getName());
+ if (SeenNames.insert(Name).second)
+ return EC; // name not seen before
+ }
+ llvm_unreachable("returned above");
}
vfs::recursive_directory_iterator::recursive_directory_iterator(FileSystem &FS_,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
