erik.pilkington created this revision.
erik.pilkington added reviewers: bruno, rsmith.
Herald added a subscriber: dexonsmith.
This patch makes clang include headers found in modulemap files in the `.d`
file. This is necessary to capture symlinked headers, which previously were
ignored, since only the canonical version of the file makes it into the pcm.
This is a corollary to the -module-dependency-dir version from r264971.
rdar://44604913
Thanks for taking a look!
Erik
Repository:
rC Clang
https://reviews.llvm.org/D53522
Files:
clang/include/clang/Lex/ModuleMap.h
clang/lib/Frontend/DependencyFile.cpp
clang/lib/Frontend/ModuleDependencyCollector.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Serialization/ASTReader.cpp
clang/test/Modules/Inputs/dfg-symlinks.modulemap
clang/test/Modules/dependency-file-symlinks.c
clang/test/Modules/dependency-gen-pch.m
clang/test/Modules/dependency-gen.m
clang/test/Modules/dependency-gen.modulemap
Index: clang/test/Modules/dependency-gen.modulemap
===================================================================
--- clang/test/Modules/dependency-gen.modulemap
+++ clang/test/Modules/dependency-gen.modulemap
@@ -27,6 +27,7 @@
// For an explicit use of a module via -fmodule-file=, the other module maps
// and included headers are not dependencies of this target (they are instead
// dependencies of the explicitly-specified .pcm input).
+// FIXME: Shouldn't we include all transitive dependencies here?
//
// EXPLICIT: {{^}}explicit.pcm:
// EXPLICIT-NOT: dependency-gen-
Index: clang/test/Modules/dependency-gen.m
===================================================================
--- clang/test/Modules/dependency-gen.m
+++ clang/test/Modules/dependency-gen.m
@@ -4,19 +4,19 @@
// RUN: %clang_cc1 -x objective-c -isystem %S/Inputs/System/usr/include -dependency-file %t.d.1 -MT %s.o -I %S/Inputs -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t-mcp %s
// RUN: FileCheck %s < %t.d.1
// CHECK: dependency-gen.m
-// CHECK: Inputs{{.}}module.map
// CHECK: Inputs{{.}}diamond_top.h
+// CHECK: Inputs{{.}}module.map
// CHECK-NOT: usr{{.}}include{{.}}module.map
// CHECK-NOT: stdint.h
// RUN: %clang_cc1 -x objective-c -isystem %S/Inputs/System/usr/include -dependency-file %t.d.2 -MT %s.o -I %S/Inputs -sys-header-deps -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t-mcp %s
// RUN: FileCheck %s -check-prefix=CHECK-SYS < %t.d.2
// CHECK-SYS: dependency-gen.m
-// CHECK-SYS: Inputs{{.}}module.map
// CHECK-SYS: Inputs{{.}}diamond_top.h
-// CHECK-SYS: usr{{.}}include{{.}}module.map
+// CHECK-SYS: Inputs{{.}}module.map
// CHECK-SYS: stdint.h
+// CHECK-SYS: usr{{.}}include{{.}}module.map
#import "diamond_top.h"
#import "stdint.h" // inside sysroot
Index: clang/test/Modules/dependency-gen-pch.m
===================================================================
--- clang/test/Modules/dependency-gen-pch.m
+++ clang/test/Modules/dependency-gen-pch.m
@@ -5,9 +5,9 @@
// RUN: %clang_cc1 -isysroot %S/Inputs/System -triple x86_64-apple-darwin10 -module-file-deps -dependency-file %t.d -MT %s.o -I %S/Inputs -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t-mcp -emit-pch -o %t.pch %s
// RUN: FileCheck %s < %t.d
// CHECK: dependency-gen-pch.m.o
-// CHECK-NEXT: dependency-gen-pch.m
-// CHECK-NEXT: Inputs{{.}}module.map
-// CHECK-NEXT: diamond_top.pcm
-// CHECK-NEXT: Inputs{{.}}diamond_top.h
+// CHECK: dependency-gen-pch.m
+// CHECK: Inputs{{.}}diamond_top.h
+// CHECK: Inputs{{.}}module.map
+// CHECK: diamond_top.pcm
#import "diamond_top.h"
Index: clang/test/Modules/dependency-file-symlinks.c
===================================================================
--- /dev/null
+++ clang/test/Modules/dependency-file-symlinks.c
@@ -0,0 +1,37 @@
+// REQUIRES: shell
+
+// RUN: rm -fr %t
+// RUN: mkdir -p %t/cache
+
+// Set up %t as follows:
+// * misc.h #includes target.h
+// * target.h is empty
+// * link.h is a symlink to target.h
+
+// RUN: cp %S/Inputs/dfg-symlinks.modulemap %t/module.modulemap
+// RUN: echo "int foo();" > %t/target.h
+// RUN: ln -s %t/target.h %t/link.h
+// RUN: echo '#include "target.h"' >> %t/misc.h
+// RUN: echo 'int foo();' >> %t/misc.h
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache %s \
+// RUN: -I %t -MT dependency-file-symlinks.o -dependency-file - | FileCheck %s
+
+// Run clang again, to make sure that we get identical output with the cached
+// modules.
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache %s \
+// RUN: -I %t -MT dependency-file-symlinks.o -dependency-file - | FileCheck %s
+
+#include "misc.h"
+
+int main() {
+ void *p = foo;
+}
+
+// CHECK: dependency-file-symlinks.o:
+// CHECK-NEXT: {{.*}}dependency-file-symlinks.c
+// CHECK-NEXT: {{.*}}link.h
+// CHECK-NEXT: {{.*}}misc.h
+// CHECK-NEXT: {{.*}}module.modulemap
+// CHECK-NEXT: {{.*}}target.h
Index: clang/test/Modules/Inputs/dfg-symlinks.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/dfg-symlinks.modulemap
@@ -0,0 +1,4 @@
+module my_module {
+ header "link.h" export *
+ header "misc.h" export *
+}
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1799,7 +1799,7 @@
// going to use this information to rebuild the module, so it doesn't make
// a lot of difference.
Module::Header H = { key.Filename, FileMgr.getFile(Filename) };
- ModMap.addHeader(Mod, H, HeaderRole, /*Imported*/true);
+ ModMap.addHeader(Mod, H, HeaderRole, /*OrigFileName=*/"", /*Imported*/true);
HFI.isModuleHeader |= !(HeaderRole & ModuleMap::TextualHeader);
}
Index: clang/lib/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -265,7 +265,7 @@
if (Header.Kind == Module::HK_Excluded)
excludeHeader(Mod, H);
else
- addHeader(Mod, H, headerKindToRole(Header.Kind));
+ addHeader(Mod, H, headerKindToRole(Header.Kind), Header.FileName);
}
} else if (Header.HasBuiltinHeader && !Header.Size && !Header.ModTime) {
// There's a builtin header but no corresponding on-disk header. Assume
@@ -1090,7 +1090,8 @@
// Notify callbacks that we just added a new header.
for (const auto &Cb : Callbacks)
- Cb->moduleMapAddUmbrellaHeader(&SourceMgr.getFileManager(), UmbrellaHeader);
+ Cb->moduleMapAddUmbrellaHeader(&SourceMgr.getFileManager(), UmbrellaHeader,
+ Mod->IsSystem);
}
void ModuleMap::setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
@@ -1162,7 +1163,8 @@
}
void ModuleMap::addHeader(Module *Mod, Module::Header Header,
- ModuleHeaderRole Role, bool Imported) {
+ ModuleHeaderRole Role, StringRef OrigHeaderName,
+ bool Imported) {
KnownHeader KH(Mod, Role);
// Only add each header to the headers list once.
@@ -1186,8 +1188,21 @@
}
// Notify callbacks that we just added a new header.
- for (const auto &Cb : Callbacks)
- Cb->moduleMapAddHeader(Header.Entry->getName());
+ for (const auto &Cb : Callbacks) {
+ Cb->moduleMapAddHeader(Header.Entry->getName(), Mod->IsSystem);
+
+ // If the header in the module map refers to a symlink, Header.Entry refers
+ // to the actual file. The callback should be notified of both.
+
+ if (OrigHeaderName.empty())
+ continue;
+
+ SmallString<128> FullPath(Mod->Directory->getName());
+ llvm::sys::path::append(FullPath, OrigHeaderName);
+
+ if (!FullPath.equals(Header.Entry->getName()))
+ Cb->moduleMapAddHeader(FullPath, Mod->IsSystem);
+ }
}
void ModuleMap::excludeHeader(Module *Mod, Module::Header Header) {
Index: clang/lib/Frontend/ModuleDependencyCollector.cpp
===================================================================
--- clang/lib/Frontend/ModuleDependencyCollector.cpp
+++ clang/lib/Frontend/ModuleDependencyCollector.cpp
@@ -63,14 +63,15 @@
ModuleDependencyMMCallbacks(ModuleDependencyCollector &Collector)
: Collector(Collector) {}
- void moduleMapAddHeader(StringRef HeaderPath) override {
+ void moduleMapAddHeader(StringRef HeaderPath, bool IsSystem) override {
if (llvm::sys::path::is_absolute(HeaderPath))
Collector.addFile(HeaderPath);
}
void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
- const FileEntry *Header) override {
+ const FileEntry *Header,
+ bool IsSystem) override {
StringRef HeaderFilename = Header->getName();
- moduleMapAddHeader(HeaderFilename);
+ moduleMapAddHeader(HeaderFilename, IsSystem);
// 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
@@ -92,7 +93,7 @@
llvm::sys::path::append(AltHeaderFilename, UmbrellaDir,
llvm::sys::path::filename(HeaderFilename));
if (FileMgr->getFile(AltHeaderFilename))
- moduleMapAddHeader(AltHeaderFilename);
+ moduleMapAddHeader(AltHeaderFilename, IsSystem);
}
}
};
Index: clang/lib/Frontend/DependencyFile.cpp
===================================================================
--- clang/lib/Frontend/DependencyFile.cpp
+++ clang/lib/Frontend/DependencyFile.cpp
@@ -89,6 +89,21 @@
/*IsModuleFile*/false,
/*IsMissing*/false);
}
+
+ void moduleMapAddHeader(StringRef HeaderPath, bool IsSystem) override {
+ if (!llvm::sys::path::is_absolute(HeaderPath))
+ return;
+
+ DepCollector.maybeAddDependency(HeaderPath, /*FromModule=*/false, IsSystem,
+ /*IsModuleFile*/ false,
+ /*IsMissing=*/false);
+ }
+
+ void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
+ const FileEntry *Header,
+ bool IsSystem) override {
+ DepCollectorMMCallbacks::moduleMapAddHeader(Header->getName(), IsSystem);
+ }
};
struct DepCollectorASTListener : public ASTReaderListener {
@@ -215,12 +230,28 @@
class DFGMMCallback : public ModuleMapCallbacks {
DFGImpl &Parent;
+
+ void addFile(StringRef Path, bool IsSystem) {
+ if (!IsSystem || Parent.includeSystemHeaders())
+ Parent.AddFilename(Path);
+ }
+
public:
DFGMMCallback(DFGImpl &Parent) : Parent(Parent) {}
+
void moduleMapFileRead(SourceLocation Loc, const FileEntry &Entry,
bool IsSystem) override {
- if (!IsSystem || Parent.includeSystemHeaders())
- Parent.AddFilename(Entry.getName());
+ addFile(Entry.getName(), IsSystem);
+ }
+
+ void moduleMapAddHeader(StringRef HeaderPath, bool IsSystem) override {
+ if (llvm::sys::path::is_absolute(HeaderPath))
+ addFile(HeaderPath, IsSystem);
+ }
+
+ void moduleMapAddUmbrellaHeader(FileManager *FileMgr, const FileEntry *Header,
+ bool IsSystem) override {
+ DFGMMCallback::moduleMapAddHeader(Header->getName(), IsSystem);
}
};
Index: clang/include/clang/Lex/ModuleMap.h
===================================================================
--- clang/include/clang/Lex/ModuleMap.h
+++ clang/include/clang/Lex/ModuleMap.h
@@ -60,14 +60,17 @@
/// Called when a header is added during module map parsing.
///
/// \param Filename The header file itself.
- virtual void moduleMapAddHeader(StringRef Filename) {}
+ /// \param IsSystem Whether this header is from a system include path.
+ virtual void moduleMapAddHeader(StringRef Filename, bool IsSystem) {}
/// Called when an umbrella header is added during module map parsing.
///
/// \param FileMgr FileManager instance
/// \param Header The umbrella header to collect.
+ /// \param IsSystem Whether this header is from a system include path.
virtual void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
- const FileEntry *Header) {}
+ const FileEntry *Header,
+ bool IsSystem) {}
};
class ModuleMap {
@@ -641,8 +644,11 @@
/// Adds this header to the given module.
/// \param Role The role of the header wrt the module.
+ /// \param OrigHeaderName The header name as spelt in the module map file.
+ /// This can differ from \c Header's name due to symlinks.
void addHeader(Module *Mod, Module::Header Header,
- ModuleHeaderRole Role, bool Imported = false);
+ ModuleHeaderRole Role, StringRef OrigHeaderName = "",
+ bool Imported = false);
/// Marks this header as being excluded from the given module.
void excludeHeader(Module *Mod, Module::Header Header);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
