erik.pilkington updated this revision to Diff 171719.
erik.pilkington marked 7 inline comments as done.
erik.pilkington added a comment.
Address review comments. Thanks!
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/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/relative-dep-gen.cpp
Index: clang/test/Modules/relative-dep-gen.cpp
===================================================================
--- clang/test/Modules/relative-dep-gen.cpp
+++ clang/test/Modules/relative-dep-gen.cpp
@@ -29,10 +29,8 @@
#include "Inputs/relative-dep-gen-1.h"
-// CHECK-BUILD: mod.pcm:
+// CHECK-BUILD: mod.pcm: {{.*}}Inputs{{[/\\]}}relative-dep-gen-1.h {{.*}}Inputs{{[/\\]}}relative-dep-gen-2.h
// CHECK-BUILD: {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen{{(-cwd)?}}.modulemap
-// CHECK-BUILD: {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen-1.h
-// CHECK-BUILD: {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen-2.h
// CHECK-USE: use.o:
// CHECK-USE-DAG: {{[ \t]}}relative-dep-gen.cpp
// CHECK-EXPLICIT-DAG: mod.pcm
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,55 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %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 --check-prefix=CHECK_IMP %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 --check-prefix=CHECK_IMP %s
+
+// Verify that the an explicit use of a module via -fmodule-file doesn't include
+// any headers from that files's modulemap.
+
+// RUN: %clang_cc1 -fmodules -fmodule-name="my_module" -emit-module \
+// RUN: -o %t/my_module.pcm %t/module.modulemap
+
+// RUN: %clang_cc1 -fmodules -fmodule-file=%t/my_module.pcm %s -MT \
+// RUN: dependency-file-symlinks.o -I %t -dependency-file - | \
+// RUN: FileCheck --check-prefix CHECK_EXP %s
+
+#include "misc.h"
+
+int main() {
+ void *p = foo;
+}
+
+// CHECK_IMP: dependency-file-symlinks.o:
+// CHECK_IMP-NEXT: {{.*}}dependency-file-symlinks.c
+// CHECK_IMP-NEXT: {{.*}}link.h
+// CHECK_IMP-NEXT: {{.*}}misc.h
+// CHECK_IMP-NEXT: {{.*}}module.modulemap
+// CHECK_IMP-NEXT: {{.*}}target.h
+
+// CHECK_EXP: dependency-file-symlinks.o:
+// CHECK_EXP-NEXT: {{.*}}my_module.pcm
+// CHECK_EXP-NEXT: {{.*}}dependency-file-symlinks.c
+
+// CHECK_EXP-NOT: misc.h
+// CHECK_EXP-NOT: link.h
+// CHECK_EXP-NOT: 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/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -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,
@@ -1185,9 +1186,22 @@
isCompilingModuleHeader);
}
- // Notify callbacks that we just added a new header.
- for (const auto &Cb : Callbacks)
- Cb->moduleMapAddHeader(Header.Entry->getName());
+ SmallString<128> FullPathAsWritten;
+ if (Mod->Directory) {
+ FullPathAsWritten = Mod->Directory->getName();
+ llvm::sys::path::append(FullPathAsWritten, Header.NameAsWritten);
+ }
+
+ for (const auto &Cb : Callbacks) {
+ Cb->moduleMapAddHeader(Header.Entry->getName(), Mod->IsSystem, Imported);
+
+ // 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 (!FullPathAsWritten.empty() &&
+ !FullPathAsWritten.equals(Header.Entry->getName())) {
+ Cb->moduleMapAddHeader(FullPathAsWritten, Mod->IsSystem, Imported);
+ }
+ }
}
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,17 @@
ModuleDependencyMMCallbacks(ModuleDependencyCollector &Collector)
: Collector(Collector) {}
- void moduleMapAddHeader(StringRef HeaderPath) override {
+ void moduleMapAddHeader(StringRef HeaderPath, bool IsSystem,
+ bool Imported) override {
if (llvm::sys::path::is_absolute(HeaderPath))
Collector.addFile(HeaderPath);
}
void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
- const FileEntry *Header) override {
+ const FileEntry *Header,
+ bool IsSystem,
+ bool Imported) override {
StringRef HeaderFilename = Header->getName();
- moduleMapAddHeader(HeaderFilename);
+ moduleMapAddHeader(HeaderFilename, IsSystem, Imported);
// 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 +95,7 @@
llvm::sys::path::append(AltHeaderFilename, UmbrellaDir,
llvm::sys::path::filename(HeaderFilename));
if (FileMgr->getFile(AltHeaderFilename))
- moduleMapAddHeader(AltHeaderFilename);
+ moduleMapAddHeader(AltHeaderFilename, IsSystem, Imported);
}
}
};
Index: clang/lib/Frontend/DependencyFile.cpp
===================================================================
--- clang/lib/Frontend/DependencyFile.cpp
+++ clang/lib/Frontend/DependencyFile.cpp
@@ -89,6 +89,24 @@
/*IsModuleFile*/false,
/*IsMissing*/false);
}
+
+ void moduleMapAddHeader(StringRef HeaderPath, bool IsSystem,
+ bool Imported) override {
+ // If this file name was found embedded in an AST file, then don't track it
+ // here. This is to avoid including -fmodule-file= dependencies.
+ if (Imported)
+ return;
+
+ DepCollector.maybeAddDependency(HeaderPath, /*FromModule=*/false, IsSystem,
+ /*IsModuleFile*/ false,
+ /*IsMissing=*/false);
+ }
+
+ void moduleMapAddUmbrellaHeader(FileManager *FileMgr, const FileEntry *Header,
+ bool IsSystem, bool Imported) override {
+ DepCollectorMMCallbacks::moduleMapAddHeader(Header->getName(), IsSystem,
+ Imported);
+ }
};
struct DepCollectorASTListener : public ASTReaderListener {
@@ -215,12 +233,32 @@
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,
+ bool Imported) override {
+ // If this file name was found embedded in an AST file, then don't track it
+ // here. This is to avoid including -fmodule-file= dependencies.
+ if (Imported)
+ return;
+ addFile(HeaderPath, IsSystem);
+ }
+
+ void moduleMapAddUmbrellaHeader(FileManager *FileMgr, const FileEntry *Header,
+ bool IsSystem, bool Imported) override {
+ DFGMMCallback::moduleMapAddHeader(Header->getName(), IsSystem, Imported);
}
};
Index: clang/include/clang/Lex/ModuleMap.h
===================================================================
--- clang/include/clang/Lex/ModuleMap.h
+++ clang/include/clang/Lex/ModuleMap.h
@@ -60,14 +60,21 @@
/// 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.
+ /// \param Imported if this header was imported from an AST file.
+ virtual void moduleMapAddHeader(StringRef Filename, bool IsSystem,
+ bool Imported = false) {}
/// 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.
+ /// \param Imported if this header was imported from an AST file.
virtual void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
- const FileEntry *Header) {}
+ const FileEntry *Header,
+ bool IsSystem,
+ bool Imported = false) {}
};
class ModuleMap {
@@ -641,6 +648,7 @@
/// Adds this header to the given module.
/// \param Role The role of the header wrt the module.
+ /// \param Imported If this header was found in an AST file.
void addHeader(Module *Mod, Module::Header Header,
ModuleHeaderRole Role, bool Imported = false);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
