vsapsai updated this revision to Diff 171210.
vsapsai added a comment.
- Rebase once again.
- Use Doxygen comments in some places.
https://reviews.llvm.org/D50539
Files:
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
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp
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<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: 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': <boolean, default=true>
/// 'use-external-names': <boolean, default=true>
/// 'overlay-relative': <boolean, default=false>
+/// 'fallthrough': <boolean, default=true>
///
/// 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<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);
@@ -1156,7 +1191,8 @@
auto *D = cast<RedirectingDirectoryEntry>(*E);
return directory_iterator(std::make_shared<VFSFromYamlDirIterImpl>(
- 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<Status> RedirectingFileSystem::status(const Twine &Path) {
ErrorOr<Entry *> 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<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?
@@ -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: 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,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/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
@@ -3295,25 +3295,27 @@
if (CI.getHeaderSearchOpts().VFSOverlayFiles.empty())
return BaseFS;
- IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> Overlay(
- new llvm::vfs::OverlayFileSystem(BaseFS));
+ IntrusiveRefCntPtr<llvm::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<llvm::vfs::FileSystem> 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
