Skip to content

Commit de80717

Browse files
artemcmartemcm
committed
[Dependency Scanner] Add missing clang overlay dependencies for placeholder modules
We were previously forgetting about these. On top of that, make sure we treat accumulated dependencies as a set, because adding clang overlay dependencies may duplicate existing already-added dependencies for some module.
1 parent 7f2185d commit de80717

File tree

6 files changed

+133
-73
lines changed

6 files changed

+133
-73
lines changed

‎lib/FrontendTool/ScanDependencies.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -161,15 +161,15 @@ static std::vector<ModuleDependencyID> resolveDirectDependencies(
161161
auto isSwift = knownDependencies.isSwiftTextualModule();
162162

163163
// Find the dependencies of every module this module directly depends on.
164-
std::vector<ModuleDependencyID> result;
164+
std::set<ModuleDependencyID> result;
165165
for (auto dependsOn : knownDependencies.getModuleDependencies()) {
166166
// Figure out what kind of module we need.
167167
bool onlyClangModule = !isSwift || module.first == dependsOn;
168168

169169
// Retrieve the dependencies for this module.
170170
if (auto found = ctx.getModuleDependencies(
171171
dependsOn, onlyClangModule, cache, ASTDelegate)) {
172-
result.push_back({dependsOn, found->getKind()});
172+
result.insert({dependsOn, found->getKind()});
173173
}
174174
}
175175

@@ -215,13 +215,15 @@ static std::vector<ModuleDependencyID> resolveDirectDependencies(
215215
// This Clang module may have the same name as the Swift module we are resolving, so we
216216
// need to make sure we don't add a dependency from a Swift module to itself.
217217
if ((found->getKind() == ModuleDependenciesKind::SwiftTextual ||
218-
found->getKind() == ModuleDependenciesKind::SwiftBinary) &&
219-
clangDep != module.first)
220-
result.push_back({clangDep, found->getKind()});
218+
found->getKind() == ModuleDependenciesKind::SwiftBinary ||
219+
found->getKind() == ModuleDependenciesKind::SwiftPlaceholder) &&
220+
clangDep != module.first) {
221+
result.insert({clangDep, found->getKind()});
222+
}
221223
}
222224
}
223225
}
224-
return result;
226+
return std::vector<ModuleDependencyID>(result.begin(), result.end());
225227
}
226228

227229
static void discoverCrosssImportOverlayDependencies(

‎test/ScanDependencies/can_import_placeholder.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ import SomeExternalModule
3030

3131
// CHECK: directDependencies
3232
// CHECK-NEXT: {
33+
// CHECK-NEXT: "swift": "F"
34+
// CHECK-NEXT: }
35+
// CHECK-NEXT: {
3336
// CHECK-NEXT: "swiftPlaceholder": "SomeExternalModule"
3437
// CHECK-NEXT: }
3538
// CHECK-NEXT: {
@@ -38,8 +41,5 @@ import SomeExternalModule
3841
// CHECK-NEXT: {
3942
// CHECK-NEXT: "swift": "SwiftOnoneSupport"
4043
// CHECK-NEXT: }
41-
// CHECK-NEXT: {
42-
// CHECK-NEXT: "swift": "F"
43-
// CHECK-NEXT: }
4444
// CHECK-NEXT: ],
4545

‎test/ScanDependencies/module_deps.swift

Lines changed: 58 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -34,35 +34,35 @@ import SubE
3434
// CHECK-LABEL: "modulePath": "deps.swiftmodule",
3535
// CHECK-NEXT: sourceFiles
3636
// CHECK-NEXT: module_deps.swift
37-
38-
// CHECK: directDependencies
39-
// CHECK-NEXT: {
40-
// CHECK-NEXT: "clang": "C"
41-
// CHECK-NEXT: }
42-
// CHECK-NEXT: {
43-
// CHECK-NEXT: "swift": "E"
44-
// CHECK-NEXT: }
45-
// CHECK-NEXT: {
46-
// CHECK-NEXT: "swift": "G"
47-
// CHECK-NEXT: }
48-
// CHECK-NEXT: {
49-
// CHECK-NEXT: "swift": "SubE"
50-
// CHECK-NEXT: }
51-
// CHECK-NEXT: {
52-
// CHECK-NEXT: "swift": "Swift"
53-
// CHECK-NEXT: }
54-
// CHECK-NEXT: {
55-
// CHECK-NEXT: "swift": "SwiftOnoneSupport"
56-
// CHECK-NEXT: }
57-
// CHECK-NEXT: {
58-
// CHECK-NEXT: "swift": "_cross_import_E"
59-
// CHECK-NEXT: }
60-
// CHECK-NEXT: {
61-
// CHECK-NEXT: "swift": "F"
62-
// CHECK-NEXT: }
63-
// CHECK-NEXT: {
64-
// CHECK-NEXT: "swift": "A"
65-
// CHECK-NEXT: }
37+
// CHECK-NEXT: ],
38+
// CHECK-NEXT: "directDependencies": [
39+
// CHECK-NEXT: {
40+
// CHECK-NEXT: "swift": "A"
41+
// CHECK-NEXT: },
42+
// CHECK-NEXT: {
43+
// CHECK-NEXT: "clang": "C"
44+
// CHECK-NEXT: },
45+
// CHECK-NEXT: {
46+
// CHECK-NEXT: "swift": "E"
47+
// CHECK-NEXT: },
48+
// CHECK-NEXT: {
49+
// CHECK-NEXT: "swift": "F"
50+
// CHECK-NEXT: },
51+
// CHECK-NEXT: {
52+
// CHECK-NEXT: "swift": "G"
53+
// CHECK-NEXT: },
54+
// CHECK-NEXT: {
55+
// CHECK-NEXT: "swift": "SubE"
56+
// CHECK-NEXT: },
57+
// CHECK-NEXT: {
58+
// CHECK-NEXT: "swift": "Swift"
59+
// CHECK-NEXT: },
60+
// CHECK-NEXT: {
61+
// CHECK-NEXT: "swift": "SwiftOnoneSupport"
62+
// CHECK-NEXT: },
63+
// CHECK-NEXT: {
64+
// CHECK-NEXT: "swift": "_cross_import_E"
65+
// CHECK-NEXT: }
6666
// CHECK-NEXT: ],
6767

6868
// CHECK: "extraPcmArgs": [
@@ -83,6 +83,17 @@ import SubE
8383
// CHECK-NEXT: "F"
8484
// CHECK-NEXT: ]
8585

86+
/// --------Swift module A
87+
// CHECK-LABEL: "modulePath": "A.swiftmodule",
88+
89+
// CHECK: directDependencies
90+
// CHECK-NEXT: {
91+
// CHECK-NEXT: "clang": "A"
92+
// CHECK-NEXT: }
93+
// CHECK-NEXT: {
94+
// CHECK-NEXT: "swift": "Swift"
95+
// CHECK-NEXT: },
96+
8697
/// --------Clang module C
8798
// CHECK-LABEL: "modulePath": "C.pcm",
8899

@@ -120,14 +131,30 @@ import SubE
120131
// CHECK: "moduleInterfacePath"
121132
// CHECK-SAME: E.swiftinterface
122133

134+
/// --------Swift module F
135+
// CHECK: "modulePath": "F.swiftmodule",
136+
// CHECK-NEXT: "sourceFiles": [
137+
// CHECK-NEXT: ],
138+
// CHECK-NEXT: "directDependencies": [
139+
// CHECK-NEXT: {
140+
// CHECK-NEXT: "clang": "F"
141+
// CHECK-NEXT: },
142+
// CHECK-NEXT: {
143+
// CHECK-NEXT: "swift": "Swift"
144+
// CHECK-NEXT: },
145+
// CHECK-NEXT: {
146+
// CHECK-NEXT: "swift": "SwiftOnoneSupport"
147+
// CHECK-NEXT: }
148+
// CHECK-NEXT: ],
149+
123150
/// --------Swift module G
124151
// CHECK-LABEL: "modulePath": "G.swiftmodule"
125152
// CHECK: "directDependencies"
126153
// CHECK-NEXT: {
127-
// CHECK-NEXT: "swift": "Swift"
154+
// CHECK-NEXT: "clang": "G"
128155
// CHECK-NEXT: },
129156
// CHECK-NEXT: {
130-
// CHECK-NEXT: "clang": "G"
157+
// CHECK-NEXT: "swift": "Swift"
131158
// CHECK-NEXT: },
132159
// CHECK-NEXT: {
133160
// CHECK-NEXT: "swift": "SwiftOnoneSupport"
@@ -156,33 +183,6 @@ import SubE
156183
// CHECK-NEXT: {
157184
// CHECK-NEXT: "clang": "SwiftShims"
158185

159-
/// --------Swift module F
160-
// CHECK: "modulePath": "F.swiftmodule",
161-
// CHECK-NEXT: "sourceFiles": [
162-
// CHECK-NEXT: ],
163-
// CHECK-NEXT: "directDependencies": [
164-
// CHECK-NEXT: {
165-
// CHECK-NEXT: "swift": "Swift"
166-
// CHECK-NEXT: },
167-
// CHECK-NEXT: {
168-
// CHECK-NEXT: "clang": "F"
169-
// CHECK-NEXT: },
170-
// CHECK-NEXT: {
171-
// CHECK-NEXT: "swift": "SwiftOnoneSupport"
172-
// CHECK-NEXT: }
173-
// CHECK-NEXT: ],
174-
175-
/// --------Swift module A
176-
// CHECK-LABEL: "modulePath": "A.swiftmodule",
177-
178-
// CHECK: directDependencies
179-
// CHECK-NEXT: {
180-
// CHECK-NEXT: "swift": "Swift"
181-
// CHECK-NEXT: },
182-
// CHECK-NEXT: {
183-
// CHECK-NEXT: "clang": "A"
184-
// CHECK-NEXT: }
185-
186186
/// --------Clang module B
187187
// CHECK-LABEL: "modulePath": "B.pcm"
188188

‎test/ScanDependencies/module_deps_cross_import_overlay.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ import SubEWrapper
1616
// CHECK-NEXT: "swift": "EWrapper"
1717
// CHECK-NEXT: },
1818
// CHECK-NEXT: {
19+
// CHECK-NEXT: "swift": "F"
20+
// CHECK-NEXT: },
21+
// CHECK-NEXT: {
1922
// CHECK-NEXT: "swift": "SubEWrapper"
2023
// CHECK-NEXT: },
2124
// CHECK-NEXT: {
@@ -26,8 +29,5 @@ import SubEWrapper
2629
// CHECK-NEXT: },
2730
// CHECK-NEXT: {
2831
// CHECK-NEXT: "swift": "_cross_import_E"
29-
// CHECK-NEXT: },
30-
// CHECK-NEXT: {
31-
// CHECK-NEXT: "swift": "F"
3232
// CHECK-NEXT: }
3333
// CHECK-NEXT: ],

‎test/ScanDependencies/module_deps_external.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ import SomeExternalModule
4040

4141
// CHECK: directDependencies
4242
// CHECK-NEXT: {
43+
// CHECK-NEXT: "swift": "F"
44+
// CHECK-NEXT: }
45+
// CHECK-NEXT: {
4346
// CHECK-NEXT: "swiftPlaceholder": "SomeExternalModule"
4447
// CHECK-NEXT: }
4548
// CHECK-NEXT: {
@@ -48,9 +51,6 @@ import SomeExternalModule
4851
// CHECK-NEXT: {
4952
// CHECK-NEXT: "swift": "SwiftOnoneSupport"
5053
// CHECK-NEXT: }
51-
// CHECK-NEXT: {
52-
// CHECK-NEXT: "swift": "F"
53-
// CHECK-NEXT: }
5454
// CHECK-NEXT: ],
5555

5656
// CHECK: "extraPcmArgs": [

‎test/ScanDependencies/placholder_overlay_deps.swift

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: mkdir -p %t/clang-module-cache
3+
// RUN: mkdir -p %t/inputs
4+
5+
// RUN: echo "[{" > %/t/inputs/map.json
6+
// RUN: echo "\"moduleName\": \"Darwin\"," >> %/t/inputs/map.json
7+
// RUN: echo "\"modulePath\": \"%/t/inputs/Darwin.swiftmodule\"," >> %/t/inputs/map.json
8+
// RUN: echo "\"docPath\": \"%/t/inputs/Darwin.swiftdoc\"," >> %/t/inputs/map.json
9+
// RUN: echo "\"sourceInfoPath\": \"%/t/inputs/Darwin.swiftsourceinfo\"," >> %/t/inputs/map.json
10+
// RUN: echo "\"isFramework\": false" >> %/t/inputs/map.json
11+
// RUN: echo "}]" >> %/t/inputs/map.json
12+
13+
// RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/clang-module-cache %s -placeholder-dependency-module-map-file %t/inputs/map.json -o %t/deps.json
14+
15+
// Check the contents of the JSON output
16+
// RUN: %FileCheck %s < %t/deps.json
17+
18+
// REQUIRES: executable_test
19+
// REQUIRES: objc_interop
20+
import Metal
21+
22+
// Ensure the dependency on Darwin is captured even though it is a placeholder
23+
24+
// CHECK: "modulePath": "Metal.swiftmodule",
25+
// CHECK-NEXT: "sourceFiles": [
26+
// CHECK-NEXT: ],
27+
// CHECK-NEXT: "directDependencies": [
28+
// CHECK-NEXT: {
29+
// CHECK-NEXT: "swift": "CoreFoundation"
30+
// CHECK-NEXT: },
31+
// CHECK-NEXT: {
32+
// CHECK-NEXT: "swift": "CoreGraphics"
33+
// CHECK-NEXT: },
34+
// CHECK-NEXT: {
35+
// CHECK-NEXT: "swiftPlaceholder": "Darwin"
36+
// CHECK-NEXT: },
37+
// CHECK-NEXT: {
38+
// CHECK-NEXT: "swift": "Dispatch"
39+
// CHECK-NEXT: },
40+
// CHECK-NEXT: {
41+
// CHECK-NEXT: "swift": "Foundation"
42+
// CHECK-NEXT: },
43+
// CHECK-NEXT: {
44+
// CHECK-NEXT: "swift": "IOKit"
45+
// CHECK-NEXT: },
46+
// CHECK-NEXT: {
47+
// CHECK-NEXT: "clang": "Metal"
48+
// CHECK-NEXT: },
49+
// CHECK-NEXT: {
50+
// CHECK-NEXT: "swift": "ObjectiveC"
51+
// CHECK-NEXT: },
52+
// CHECK-NEXT: {
53+
// CHECK-NEXT: "swift": "Swift"
54+
// CHECK-NEXT: },
55+
// CHECK-NEXT: {
56+
// CHECK-NEXT: "swift": "XPC"
57+
// CHECK-NEXT: }
58+
// CHECK-NEXT: ],

0 commit comments

Comments
 (0)