Skip to content

Commit d73daa9

Browse files
jansvoboda11jansvoboda11
committed
[clang][deps] Don't prune search paths used by dependencies
When pruning header search paths (to reduce the number of modules we need to build explicitly), we can't prune the search paths used in (transitive) dependencies of a module. Otherwise, we could end up with either of the following dependency graphs: ``` X:<hash1> -> Y:<hash2> X:<hash1> -> Y:<hash3> ``` depending on the search paths of the translation unit we discovered `X` and `Y` from. This patch fixes that. Depends on D121295. Reviewed By: dexonsmith Differential Revision: https://reviews.llvm.org/D121303
1 parent 6007b0b commit d73daa9

File tree

2 files changed

+184
-3
lines changed

2 files changed

+184
-3
lines changed

‎clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,21 @@ static void optimizeHeaderSearchOpts(HeaderSearchOptions &Opts,
2323
// Only preserve search paths that were used during the dependency scan.
2424
std::vector<HeaderSearchOptions::Entry> Entries = Opts.UserEntries;
2525
Opts.UserEntries.clear();
26-
for (unsigned I = 0; I < Entries.size(); ++I)
27-
if (MF.SearchPathUsage[I])
28-
Opts.UserEntries.push_back(Entries[I]);
26+
27+
llvm::BitVector SearchPathUsage(Entries.size());
28+
llvm::DenseSet<const serialization::ModuleFile *> Visited;
29+
std::function<void(const serialization::ModuleFile *)> VisitMF =
30+
[&](const serialization::ModuleFile *MF) {
31+
SearchPathUsage |= MF->SearchPathUsage;
32+
Visited.insert(MF);
33+
for (const serialization::ModuleFile *Import : MF->Imports)
34+
if (!Visited.contains(Import))
35+
VisitMF(Import);
36+
};
37+
VisitMF(&MF);
38+
39+
for (auto Idx : SearchPathUsage.set_bits())
40+
Opts.UserEntries.push_back(Entries[Idx]);
2941
}
3042

3143
CompilerInvocation ModuleDepCollector::makeInvocationForModuleBuildWithoutPaths(

‎clang/test/ClangScanDeps/header-search-pruning-transitive.c

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
// This test checks that pruning of header search paths produces consistent dependency graphs.
2+
//
3+
// When pruning header search paths for a module, we can't remove any paths its dependencies use.
4+
// Otherwise, we could get either of the following dependency graphs depending on the search path
5+
// configuration of the particular TU that first discovered the module:
6+
// X:<hash1> -> Y:<hash2>
7+
// X:<hash1> -> Y:<hash3>
8+
// We can't have the same version of module X depend on multiple different versions of Y based on
9+
// the TU configuration.
10+
//
11+
// Keeping all header search paths (transitive) dependencies use will ensure we get consistent
12+
// dependency graphs:
13+
// X:<hash1> -> Y:<hash2>
14+
// X:<hash4> -> Y:<hash3>
15+
16+
// RUN: rm -rf %t && mkdir %t
17+
// RUN: split-file %s %t
18+
19+
//--- a/a.h
20+
//--- b/b.h
21+
//--- begin/begin.h
22+
//--- end/end.h
23+
//--- Y.h
24+
#include "begin.h"
25+
#if __has_include("a.h")
26+
#include "a.h"
27+
#endif
28+
#include "end.h"
29+
30+
//--- X.h
31+
#include "Y.h"
32+
33+
//--- module.modulemap
34+
module Y { header "Y.h" }
35+
module X { header "X.h" }
36+
37+
//--- test.c
38+
#include "X.h"
39+
40+
//--- cdb_with_a.json.template
41+
[{
42+
"file": "DIR/test.c",
43+
"directory": "DIR",
44+
"command": "clang -c test.c -o DIR/test.o -fmodules -fimplicit-modules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps -Ibegin -Ia -Ib -Iend"
45+
}]
46+
47+
//--- cdb_without_a.json.template
48+
[{
49+
"file": "DIR/test.c",
50+
"directory": "DIR",
51+
"command": "clang -c test.c -o DIR/test.o -fmodules -fimplicit-modules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps -Ibegin -Ib -Iend"
52+
}]
53+
54+
// RUN: sed -e "s|DIR|%/t|g" %t/cdb_with_a.json.template > %t/cdb_with_a.json
55+
// RUN: sed -e "s|DIR|%/t|g" %t/cdb_without_a.json.template > %t/cdb_without_a.json
56+
57+
// RUN: echo -%t > %t/results.json
58+
// RUN: clang-scan-deps -compilation-database %t/cdb_with_a.json -format experimental-full -optimize-args >> %t/results.json
59+
// RUN: clang-scan-deps -compilation-database %t/cdb_without_a.json -format experimental-full -optimize-args >> %t/results.json
60+
// RUN: cat %t/results.json | sed 's/\\/\//g' | FileCheck %s
61+
62+
// CHECK: -[[PREFIX:.*]]
63+
// CHECK-NEXT: {
64+
// CHECK-NEXT: "modules": [
65+
// CHECK-NEXT: {
66+
// CHECK-NEXT: "clang-module-deps": [
67+
// CHECK-NEXT: {
68+
// CHECK-NEXT: "context-hash": "[[HASH_Y_WITH_A:.*]]",
69+
// CHECK-NEXT: "module-name": "Y"
70+
// CHECK-NEXT: }
71+
// CHECK-NEXT: ],
72+
// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
73+
// CHECK-NEXT: "command-line": [
74+
// CHECK: ],
75+
// CHECK-NEXT: "context-hash": "[[HASH_X:.*]]",
76+
// CHECK-NEXT: "file-deps": [
77+
// CHECK-NEXT: "[[PREFIX]]/./X.h",
78+
// CHECK-NEXT: "[[PREFIX]]/./module.modulemap"
79+
// CHECK-NEXT: ],
80+
// CHECK-NEXT: "name": "X"
81+
// CHECK-NEXT: },
82+
// CHECK-NEXT: {
83+
// CHECK-NEXT: "clang-module-deps": [],
84+
// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
85+
// CHECK-NEXT: "command-line": [
86+
// CHECK: ],
87+
// CHECK-NEXT: "context-hash": "[[HASH_Y_WITH_A]]",
88+
// CHECK-NEXT: "file-deps": [
89+
// CHECK-NEXT: "[[PREFIX]]/./Y.h",
90+
// CHECK-NEXT: "[[PREFIX]]/./a/a.h",
91+
// CHECK-NEXT: "[[PREFIX]]/./begin/begin.h",
92+
// CHECK-NEXT: "[[PREFIX]]/./end/end.h",
93+
// CHECK-NEXT: "[[PREFIX]]/./module.modulemap"
94+
// CHECK-NEXT: ],
95+
// CHECK-NEXT: "name": "Y"
96+
// CHECK-NEXT: }
97+
// CHECK-NEXT: ],
98+
// CHECK-NEXT: "translation-units": [
99+
// CHECK-NEXT: {
100+
// CHECK-NEXT: "clang-context-hash": "{{.*}}",
101+
// CHECK-NEXT: "clang-module-deps": [
102+
// CHECK-NEXT: {
103+
// CHECK-NEXT: "context-hash": "[[HASH_X]]",
104+
// CHECK-NEXT: "module-name": "X"
105+
// CHECK-NEXT: }
106+
// CHECK-NEXT: ],
107+
// CHECK-NEXT: "command-line": [
108+
// CHECK: ],
109+
// CHECK-NEXT: "file-deps": [
110+
// CHECK-NEXT: "[[PREFIX]]/test.c"
111+
// CHECK-NEXT: ],
112+
// CHECK-NEXT: "input-file": "[[PREFIX]]/test.c"
113+
// CHECK-NEXT: }
114+
// CHECK-NEXT: ]
115+
// CHECK-NEXT: }
116+
// CHECK-NEXT: {
117+
// CHECK-NEXT: "modules": [
118+
// CHECK-NEXT: {
119+
// CHECK-NEXT: "clang-module-deps": [
120+
// CHECK-NEXT: {
121+
// CHECK-NEXT: "context-hash": "[[HASH_Y_WITHOUT_A:.*]]",
122+
// CHECK-NEXT: "module-name": "Y"
123+
// CHECK-NEXT: }
124+
// CHECK-NEXT: ],
125+
// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
126+
// CHECK-NEXT: "command-line": [
127+
// CHECK: ],
128+
// Here is the actual check that this module X (which imports different version of Y)
129+
// also has a different context hash from the first version of module X.
130+
// CHECK-NOT: "context-hash": "[[HASH_X]]",
131+
// CHECK: "file-deps": [
132+
// CHECK-NEXT: "[[PREFIX]]/./X.h",
133+
// CHECK-NEXT: "[[PREFIX]]/./module.modulemap"
134+
// CHECK-NEXT: ],
135+
// CHECK-NEXT: "name": "X"
136+
// CHECK-NEXT: },
137+
// CHECK-NEXT: {
138+
// CHECK-NEXT: "clang-module-deps": [],
139+
// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
140+
// CHECK-NEXT: "command-line": [
141+
// CHECK: ],
142+
// CHECK-NEXT: "context-hash": "[[HASH_Y_WITHOUT_A]]",
143+
// CHECK-NEXT: "file-deps": [
144+
// CHECK-NEXT: "[[PREFIX]]/./Y.h",
145+
// CHECK-NEXT: "[[PREFIX]]/./begin/begin.h",
146+
// CHECK-NEXT: "[[PREFIX]]/./end/end.h",
147+
// CHECK-NEXT: "[[PREFIX]]/./module.modulemap"
148+
// CHECK-NEXT: ],
149+
// CHECK-NEXT: "name": "Y"
150+
// CHECK-NEXT: }
151+
// CHECK-NEXT: ],
152+
// CHECK-NEXT: "translation-units": [
153+
// CHECK-NEXT: {
154+
// CHECK-NEXT: "clang-context-hash": "{{.*}}",
155+
// CHECK-NEXT: "clang-module-deps": [
156+
// CHECK-NEXT: {
157+
// CHECK-NEXT: "context-hash": "{{.*}}",
158+
// CHECK-NEXT: "module-name": "X"
159+
// CHECK-NEXT: }
160+
// CHECK-NEXT: ],
161+
// CHECK-NEXT: "command-line": [
162+
// CHECK: ],
163+
// CHECK-NEXT: "file-deps": [
164+
// CHECK-NEXT: "[[PREFIX]]/test.c"
165+
// CHECK-NEXT: ],
166+
// CHECK-NEXT: "input-file": "[[PREFIX]]/test.c"
167+
// CHECK-NEXT: }
168+
// CHECK-NEXT: ]
169+
// CHECK-NEXT: }

0 commit comments

Comments
 (0)