bruno updated this revision to Diff 57263.
bruno added a comment.
Updated after Ben's suggestions!
http://reviews.llvm.org/D20194
Files:
include/clang/Lex/ModuleMap.h
lib/Frontend/ModuleDependencyCollector.cpp
lib/Lex/ModuleMap.cpp
test/Modules/Inputs/crash-recovery/Frameworks/A.framework/Headers/A.h
test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Headers/B.h
test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Modules/module.modulemap
test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/I.h
test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Modules/module.modulemap
test/Modules/Inputs/crash-recovery/Frameworks/module.modulemap
test/Modules/crash-vfs-umbrella-frameworks.m
Index: test/Modules/crash-vfs-umbrella-frameworks.m
===================================================================
--- /dev/null
+++ test/Modules/crash-vfs-umbrella-frameworks.m
@@ -0,0 +1,42 @@
+// REQUIRES: crash-recovery, shell
+
+// FIXME: This XFAIL is cargo-culted from crash-report.c. Do we need it?
+// XFAIL: mingw32
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/i %t/m %t
+// RUN: cp -a %S/Inputs/crash-recovery/Frameworks %t/i/
+// RUN: mkdir -p %t/i/Frameworks/A.framework/Frameworks
+// RUN: ln -s ../../B.framework %t/i/Frameworks/A.framework/Frameworks/B.framework
+
+// RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
+// RUN: %clang -nostdinc -fsyntax-only %s \
+// RUN: -F %/t/i/Frameworks -fmodules \
+// RUN: -fmodules-cache-path=%t/m/ 2>&1 | FileCheck %s
+
+// RUN: FileCheck --check-prefix=CHECKYAML %s -input-file \
+// RUN: %t/crash-vfs-*.cache/vfs/vfs.yaml
+// RUN: find %t/crash-vfs-*.cache/vfs | \
+// RUN: grep "B.framework/Headers/B.h" | count 1
+
+// CHECK: Preprocessed source(s) and associated run script(s) are located at:
+// CHECK-NEXT: note: diagnostic msg: {{.*}}.m
+// CHECK-NEXT: note: diagnostic msg: {{.*}}.cache
+
+// CHECKYAML: 'type': 'directory',
+// CHECKYAML: 'name': "/[[PATH:.*]]/i/Frameworks/A.framework/Frameworks/B.framework/Headers",
+// CHECKYAML-NEXT: 'contents': [
+// CHECKYAML-NEXT: {
+// CHECKYAML-NEXT: 'type': 'file',
+// CHECKYAML-NEXT: 'name': "B.h",
+// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/i/Frameworks/B.framework/Headers/B.h"
+
+// CHECKYAML: 'type': 'directory',
+// CHECKYAML: 'name': "/[[PATH]]/i/Frameworks/B.framework/Headers",
+// CHECKYAML-NEXT: 'contents': [
+// CHECKYAML-NEXT: {
+// CHECKYAML-NEXT: 'type': 'file',
+// CHECKYAML-NEXT: 'name': "B.h",
+// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/i/Frameworks/B.framework/Headers/B.h"
+
+@import I;
Index: test/Modules/Inputs/crash-recovery/Frameworks/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/crash-recovery/Frameworks/module.modulemap
@@ -0,0 +1,2 @@
+framework module * [extern_c] {
+}
Index: test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Modules/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Modules/module.modulemap
@@ -0,0 +1,5 @@
+framework module I [extern_c] {
+ umbrella header "I.h"
+ export *
+ module * { export * }
+}
Index: test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/I.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/I.h
@@ -0,0 +1,2 @@
+
+#import <A/A.h>
Index: test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Modules/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Modules/module.modulemap
@@ -0,0 +1,5 @@
+framework module B [extern_c] {
+ umbrella header "B.h"
+ export *
+ module * { export * }
+}
Index: test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Headers/B.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Headers/B.h
@@ -0,0 +1 @@
+// B.h
Index: test/Modules/Inputs/crash-recovery/Frameworks/A.framework/Headers/A.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/crash-recovery/Frameworks/A.framework/Headers/A.h
@@ -0,0 +1 @@
+#include <B/B.h>
Index: lib/Lex/ModuleMap.cpp
===================================================================
--- lib/Lex/ModuleMap.cpp
+++ lib/Lex/ModuleMap.cpp
@@ -760,6 +760,10 @@
Mod->Umbrella = UmbrellaHeader;
Mod->UmbrellaAsWritten = NameAsWritten.str();
UmbrellaDirs[UmbrellaHeader->getDir()] = Mod;
+
+ // Notify callbacks that we just added a new header.
+ for (const auto &Cb : Callbacks)
+ Cb->moduleMapAddUmbrellaHeader(&SourceMgr.getFileManager(), UmbrellaHeader);
}
void ModuleMap::setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
Index: lib/Frontend/ModuleDependencyCollector.cpp
===================================================================
--- lib/Frontend/ModuleDependencyCollector.cpp
+++ lib/Frontend/ModuleDependencyCollector.cpp
@@ -48,6 +48,34 @@
if (llvm::sys::path::is_absolute(HeaderPath))
Collector.addFile(HeaderPath);
}
+ void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
+ const FileEntry *Header) override {
+ StringRef HeaderFilename = Header->getName();
+ moduleMapAddHeader(HeaderFilename);
+ // The FileManager can find and cache the symbolic link for a framework
+ // header before its real path, this means a module can have some of its
+ // headers to use other paths. Although this is usually not a problem, it's
+ // inconsistent, and not collecting the original path header leads to
+ // umbrella clashes while rebuilding modules in the crash reproducer. For
+ // example:
+ // ApplicationServices.framework/Frameworks/ImageIO.framework/ImageIO.h
+ // instead of:
+ // ImageIO.framework/ImageIO.h
+ //
+ // FIXME: this shouldn't be necessary once we have FileName instances
+ // around instead of FileEntry ones. For now, make sure we collect all
+ // that we need for the reproducer to work correctly.
+ StringRef UmbreallDirFromHeader =
+ llvm::sys::path::parent_path(HeaderFilename);
+ StringRef UmbrellaDir = Header->getDir()->getName();
+ if (!UmbrellaDir.equals(UmbreallDirFromHeader)) {
+ SmallString<128> AltHeaderFilename;
+ llvm::sys::path::append(AltHeaderFilename, UmbrellaDir,
+ llvm::sys::path::filename(HeaderFilename));
+ if (FileMgr->getFile(AltHeaderFilename))
+ moduleMapAddHeader(AltHeaderFilename);
+ }
+ }
};
}
Index: include/clang/Lex/ModuleMap.h
===================================================================
--- include/clang/Lex/ModuleMap.h
+++ include/clang/Lex/ModuleMap.h
@@ -55,6 +55,13 @@
///
/// \param Filename The header file itself.
virtual void moduleMapAddHeader(StringRef Filename) {}
+
+ /// \brief Called when an umbrella header is added during module map parsing.
+ ///
+ /// \param FileMgr FileManager instance
+ /// \param Filename The umbreall header to collect.
+ virtual void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
+ const FileEntry *Header) {}
};
class ModuleMap {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
