Skip to content

Commit ed07fe7

Browse files
committed
[clang][modules][deps] Transitive module maps are not affecting
Currently, the algorithm for gathering affecting module maps includes those defining transitive dependencies. This seems like an over-approximation, since those don't change the semantics of current module build. (With this patch, `ModulesToProcess` only ever holds modules whose headers will be serialized into the current PCM.) Reviewed By: Bigcheese Differential Revision: https://reviews.llvm.org/D137197
1 parent 997d7d1 commit ed07fe7

File tree

2 files changed

+69
-21
lines changed

2 files changed

+69
-21
lines changed

clang/lib/Serialization/ASTWriter.cpp

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -190,30 +190,20 @@ std::set<const FileEntry *> GetAffectingModuleMaps(const HeaderSearch &HS,
190190
}
191191
}
192192

193-
while (!ModulesToProcess.empty()) {
194-
auto *CurrentModule = ModulesToProcess.pop_back_val();
195-
ProcessedModules.insert(CurrentModule);
193+
const ModuleMap &MM = HS.getModuleMap();
196194

197-
Optional<FileEntryRef> ModuleMapFile =
198-
HS.getModuleMap().getModuleMapFileForUniquing(CurrentModule);
199-
if (!ModuleMapFile) {
200-
continue;
201-
}
202-
203-
ModuleMaps.insert(*ModuleMapFile);
204-
205-
for (auto *ImportedModule : (CurrentModule)->Imports) {
206-
if (!ImportedModule ||
207-
ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
208-
continue;
209-
}
210-
ModulesToProcess.push_back(ImportedModule);
211-
}
195+
auto ProcessModuleOnce = [&](const Module *Mod) {
196+
if (ProcessedModules.insert(Mod).second)
197+
if (auto ModuleMapFile = MM.getModuleMapFileForUniquing(Mod))
198+
ModuleMaps.insert(*ModuleMapFile);
199+
};
212200

201+
for (const Module *CurrentModule : ModulesToProcess) {
202+
ProcessModuleOnce(CurrentModule);
203+
for (const Module *ImportedModule : CurrentModule->Imports)
204+
ProcessModuleOnce(ImportedModule);
213205
for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses)
214-
if (UndeclaredModule &&
215-
ProcessedModules.find(UndeclaredModule) == ProcessedModules.end())
216-
ModulesToProcess.push_back(UndeclaredModule);
206+
ProcessModuleOnce(UndeclaredModule);
217207
}
218208

219209
return ModuleMaps;
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
4+
//--- tu.m
5+
#include "first.h"
6+
7+
//--- first/module.modulemap
8+
module first { header "first.h" }
9+
//--- first/first.h
10+
#include "second.h"
11+
12+
//--- second/module.modulemap
13+
module second { header "second.h" }
14+
//--- second/second.h
15+
#include "third.h"
16+
17+
//--- third/module.modulemap
18+
module third { header "third.h" }
19+
//--- third/third.h
20+
// empty
21+
22+
//--- cdb.json.template
23+
[{
24+
"file": "DIR/tu.c",
25+
"directory": "DIR",
26+
"command": "clang -I DIR/first -I DIR/second -I DIR/third -fmodules -fmodules-cache-path=DIR/cache -c DIR/tu.m -o DIR/tu.o"
27+
}]
28+
29+
// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
30+
// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
31+
// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
32+
33+
// CHECK: {
34+
// CHECK-NEXT: "modules": [
35+
// CHECK-NEXT: {
36+
// CHECK-NEXT: "clang-module-deps": [
37+
// CHECK-NEXT: {
38+
// CHECK-NEXT: "context-hash": "{{.*}}",
39+
// CHECK-NEXT: "module-name": "second"
40+
// CHECK-NEXT: }
41+
// CHECK-NEXT: ],
42+
// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/first/module.modulemap",
43+
// CHECK-NEXT: "command-line": [
44+
// CHECK-NEXT: "-cc1",
45+
// CHECK-NOT: "-fmodule-map-file=[[PREFIX]]/third/module.modulemap"
46+
// CHECK: "-fmodule-map-file=[[PREFIX]]/second/module.modulemap"
47+
// CHECK-NOT: "-fmodule-map-file=[[PREFIX]]/third/module.modulemap"
48+
// CHECK: ],
49+
// CHECK-NEXT: "context-hash": "{{.*}}",
50+
// CHECK-NEXT: "file-deps": [
51+
// CHECK-NEXT: "[[PREFIX]]/first/first.h"
52+
// CHECK-NEXT: "[[PREFIX]]/first/module.modulemap"
53+
// CHECK-NEXT: "[[PREFIX]]/second/module.modulemap"
54+
// CHECK-NEXT: ],
55+
// CHECK-NEXT: "name": "first"
56+
// CHECK-NEXT: }
57+
// CHECK: ]
58+
// CHECK: }

0 commit comments

Comments
 (0)