Skip to content

Commit 871118e

Browse files
artemcmartemcm
authored
Merge pull request #67928 from artemcm/DoNotCountOverlayDepsAsDirect
[Dependency Scanning] Remove Swift Overlay dependencies from the set of direct dependencies
2 parents 535a5c6 + 4610a83 commit 871118e

11 files changed

+94
-51
lines changed

‎include/swift/AST/ModuleDependencies.h‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ class ModuleDependencyInfoStorageBase {
134134

135135
/// The set of modules on which this module depends, resolved
136136
/// to Module IDs, qualified by module kind: Swift, Clang, etc.
137-
std::vector<ModuleDependencyID> resolvedModuleDependencies;
137+
std::vector<ModuleDependencyID> resolvedDirectModuleDependencies;
138138

139139
/// The cache key for the produced module.
140140
std::string moduleCacheKey;
@@ -531,9 +531,9 @@ class ModuleDependencyInfo {
531531
}
532532

533533
/// Retreive the module-level dependencies.
534-
const ArrayRef<ModuleDependencyID> getModuleDependencies() const {
534+
const ArrayRef<ModuleDependencyID> getDirectModuleDependencies() const {
535535
assert(storage->resolved);
536-
return storage->resolvedModuleDependencies;
536+
return storage->resolvedDirectModuleDependencies;
537537
}
538538

539539
std::string getModuleCacheKey() const {
@@ -546,10 +546,10 @@ class ModuleDependencyInfo {
546546
}
547547

548548
/// Resolve a dependency's set of `imports` with qualified Module IDs
549-
void resolveDependencies(const ArrayRef<ModuleDependencyID> dependencyIDs) {
549+
void resolveDirectDependencies(const ArrayRef<ModuleDependencyID> dependencyIDs) {
550550
assert(!storage->resolved && "Resolving an already-resolved dependency");
551551
storage->resolved = true;
552-
storage->resolvedModuleDependencies.assign(dependencyIDs.begin(), dependencyIDs.end());
552+
storage->resolvedDirectModuleDependencies.assign(dependencyIDs.begin(), dependencyIDs.end());
553553
}
554554

555555
/// Set this module's set of Swift Overlay dependencies
@@ -566,6 +566,19 @@ class ModuleDependencyInfo {
566566
textualModuleDetails->swiftOverlayDependencies.assign(dependencyIDs.begin(), dependencyIDs.end());
567567
}
568568

569+
const ArrayRef<ModuleDependencyID> getSwiftOverlayDependencies() const {
570+
CommonSwiftTextualModuleDependencyDetails *textualModuleDetails = nullptr;
571+
if (auto sourceDetailsStorage = dyn_cast<SwiftSourceModuleDependenciesStorage>(storage.get()))
572+
textualModuleDetails = &sourceDetailsStorage->textualModuleDetails;
573+
else if (auto interfaceDetailsStorage = dyn_cast<SwiftInterfaceModuleDependenciesStorage>(storage.get()))
574+
textualModuleDetails = &interfaceDetailsStorage->textualModuleDetails;
575+
576+
if (textualModuleDetails)
577+
return textualModuleDetails->swiftOverlayDependencies;
578+
else
579+
return {};
580+
}
581+
569582
std::vector<std::string> getCommandline() const {
570583
if (auto *detail = getAsClangModule())
571584
return detail->buildCommandLine;

‎include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ using ModuleInfoLayout =
124124
IdentifierIDField, // moduleName
125125
ContextHashIDField, // contextHash
126126
ImportArrayIDField, // moduleImports
127-
DependencyIDArrayIDField // resolvedModuleDependencies
127+
DependencyIDArrayIDField // resolvedDirectModuleDependencies
128128
>;
129129

130130
using SwiftInterfaceModuleDetailsLayout =

‎lib/AST/ModuleDependencies.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ bool ModuleDependencyInfo::isTestableImport(StringRef moduleName) const {
104104
}
105105

106106
void ModuleDependencyInfo::addModuleDependency(ModuleDependencyID dependencyID) {
107-
storage->resolvedModuleDependencies.push_back(dependencyID);
107+
storage->resolvedDirectModuleDependencies.push_back(dependencyID);
108108
}
109109

110110
void ModuleDependencyInfo::addOptionalModuleImport(
@@ -687,7 +687,7 @@ void ModuleDependenciesCache::resolveDependencyImports(ModuleDependencyID module
687687
assert(optionalDependencyInfo.has_value() && "Resolving unknown dependency");
688688
// Copy the existing info to a mutable one we can then replace it with, after resolving its dependencies.
689689
auto dependencyInfo = *(optionalDependencyInfo.value());
690-
dependencyInfo.resolveDependencies(dependencyIDs);
690+
dependencyInfo.resolveDirectDependencies(dependencyIDs);
691691
updateDependency(moduleID, dependencyInfo);
692692
}
693693

‎lib/DependencyScan/ModuleDependencyCacheSerialization.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
298298
moduleDep.addModuleImport(moduleName);
299299

300300
// Add qualified dependencies of this module
301-
moduleDep.resolveDependencies(*currentModuleDependencyIDs);
301+
moduleDep.resolveDirectDependencies(*currentModuleDependencyIDs);
302302

303303
// Add bridging header file path
304304
if (bridgingHeaderFileID != 0) {
@@ -1114,7 +1114,7 @@ void ModuleDependenciesCacheSerializer::collectStringsAndArrays(
11141114
dependencyInfo->getModuleImports());
11151115
addDependencyIDArray(
11161116
moduleID, ModuleIdentifierArrayKind::QualifiedModuleDependencyIDs,
1117-
dependencyInfo->getModuleDependencies());
1117+
dependencyInfo->getDirectModuleDependencies());
11181118

11191119
// Add the dependency-kind-specific data
11201120
switch (dependencyInfo->getKind()) {

‎lib/DependencyScan/ScanDependencies.cpp‎

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -165,20 +165,26 @@ static void findAllImportedClangModules(ASTContext &ctx, StringRef moduleName,
165165
return;
166166

167167
auto dependencies = optionalDependencies.value();
168-
for (const auto &dep : dependencies->getModuleDependencies()) {
168+
for (const auto &dep : dependencies->getDirectModuleDependencies())
169+
findAllImportedClangModules(ctx, dep.first, cache, allModules, knownModules);
170+
for (const auto &dep : dependencies->getSwiftOverlayDependencies())
169171
findAllImportedClangModules(ctx, dep.first, cache, allModules, knownModules);
170-
}
171172
}
172173

173174
// Get all dependencies's IDs of this module from its cached
174175
// ModuleDependencyInfo
175-
static ArrayRef<ModuleDependencyID>
176-
getDependencies(const ModuleDependencyID &moduleID,
177-
const ModuleDependenciesCache &cache) {
176+
static std::vector<ModuleDependencyID>
177+
getAllDependencies(const ModuleDependencyID &moduleID,
178+
const ModuleDependenciesCache &cache) {
178179
const auto &optionalModuleInfo =
179180
cache.findDependency(moduleID.first, moduleID.second);
180181
assert(optionalModuleInfo.has_value());
181-
return optionalModuleInfo.value()->getModuleDependencies();
182+
auto directDependenciesRef = optionalModuleInfo.value()->getDirectModuleDependencies();
183+
auto overlayDependenciesRef = optionalModuleInfo.value()->getSwiftOverlayDependencies();
184+
std::vector<ModuleDependencyID> result;
185+
result.insert(std::end(result), directDependenciesRef.begin(), directDependenciesRef.end());
186+
result.insert(std::end(result), overlayDependenciesRef.begin(), overlayDependenciesRef.end());
187+
return result;
182188
}
183189

184190
/// Implements a topological sort via recursion and reverse postorder DFS.
@@ -200,7 +206,7 @@ computeTopologicalSortOfExplicitDependencies(
200206
return;
201207

202208
// Otherwise, visit each adjacent node.
203-
for (const auto &succID : getDependencies(moduleID, cache)) {
209+
for (const auto &succID : getAllDependencies(moduleID, cache)) {
204210
// We don't worry if successor is already in this current stack,
205211
// since that would mean we have found a cycle, which should not
206212
// be possible because we checked for cycles earlier.
@@ -249,7 +255,7 @@ computeTransitiveClosureOfExplicitDependencies(
249255
it != end; ++it) {
250256
const auto &modID = *it;
251257
auto &modReachableSet = result[modID];
252-
for (const auto &succID : getDependencies(modID, cache)) {
258+
for (const auto &succID : getAllDependencies(modID, cache)) {
253259
const auto &succReachableSet = result[succID];
254260
llvm::set_union(modReachableSet, succReachableSet);
255261
}
@@ -517,10 +523,10 @@ static llvm::Error resolveExplicitModuleInputs(
517523

518524
/// Resolve the direct dependencies of the given module.
519525
static std::vector<ModuleDependencyID>
520-
resolveDirectDependencies(CompilerInstance &instance, ModuleDependencyID moduleID,
521-
ModuleDependenciesCache &cache,
522-
InterfaceSubContextDelegate &ASTDelegate) {
523-
PrettyStackTraceStringAction trace("Resolving direct dependencies of: ", moduleID.first);
526+
resolveDependencies(CompilerInstance &instance, ModuleDependencyID moduleID,
527+
ModuleDependenciesCache &cache,
528+
InterfaceSubContextDelegate &ASTDelegate) {
529+
PrettyStackTraceStringAction trace("Resolving dependencies of: ", moduleID.first);
524530
auto &ctx = instance.getASTContext();
525531
auto optionalKnownDependencies = cache.findDependency(moduleID.first, moduleID.second);
526532
assert(optionalKnownDependencies.has_value());
@@ -529,7 +535,7 @@ resolveDirectDependencies(CompilerInstance &instance, ModuleDependencyID moduleI
529535
// If this dependency has already been resolved, return the result.
530536
if (knownDependencies->isResolved() &&
531537
knownDependencies->getKind() != ModuleDependencyKind::SwiftSource)
532-
return knownDependencies->getModuleDependencies();
538+
return getAllDependencies(moduleID, cache);
533539

534540
auto isSwiftInterfaceOrSource = knownDependencies->isSwiftInterfaceModule() ||
535541
knownDependencies->isSwiftSourceModule();
@@ -621,10 +627,6 @@ resolveDirectDependencies(CompilerInstance &instance, ModuleDependencyID moduleI
621627
ctx.getSwiftModuleDependencies(clangDep, cache, ASTDelegate)) {
622628
if (clangDep != moduleID.first) {
623629
swiftOverlayDependencies.insert({clangDep, found.value()->getKind()});
624-
// FIXME: Once all clients know to fetch these dependencies from
625-
// `swiftOverlayDependencies`, the goal is to no longer have them in
626-
// `directDependencies` so the following will need to go away.
627-
directDependencies.insert({clangDep, found.value()->getKind()});
628630
}
629631
}
630632
}
@@ -716,7 +718,7 @@ static void discoverCrossImportOverlayDependencies(
716718
++currentModuleIdx) {
717719
auto module = allModules[currentModuleIdx];
718720
auto moduleDependnencyIDs =
719-
resolveDirectDependencies(instance, module, cache, ASTDelegate);
721+
resolveDependencies(instance, module, cache, ASTDelegate);
720722
allModules.insert(moduleDependnencyIDs.begin(), moduleDependnencyIDs.end());
721723
}
722724

@@ -1350,7 +1352,7 @@ generateFullDependencyGraph(CompilerInstance &instance,
13501352
auto optionalDepInfo = cache.findDependency(module.first, module.second);
13511353
assert(optionalDepInfo.has_value() && "Missing dependency info during graph generation diagnosis.");
13521354
auto depInfo = optionalDepInfo.value();
1353-
auto directDependencies = depInfo->getModuleDependencies();
1355+
auto directDependencies = depInfo->getDirectModuleDependencies();
13541356

13551357
// Generate a swiftscan_clang_details_t object based on the dependency kind
13561358
auto getModuleDetails = [&]() -> swiftscan_module_details_t {
@@ -1490,7 +1492,7 @@ static bool diagnoseCycle(CompilerInstance &instance,
14901492
assert(optionalDepInfo.has_value() && "Missing dependency info during cycle diagnosis.");
14911493
auto depInfo = optionalDepInfo.value();
14921494

1493-
for (const auto &dep : depInfo->getModuleDependencies()) {
1495+
for (const auto &dep : getAllDependencies(lastOpen, cache)) {
14941496
if (closeSet.count(dep))
14951497
continue;
14961498
if (openSet.insert(dep)) {
@@ -2009,7 +2011,14 @@ swift::dependencies::performModuleScan(CompilerInstance &instance,
20092011
++currentModuleIdx) {
20102012
auto module = allModules[currentModuleIdx];
20112013
auto discoveredModules =
2012-
resolveDirectDependencies(instance, module, cache, ASTDelegate);
2014+
resolveDependencies(instance, module, cache, ASTDelegate);
2015+
2016+
// Do not need to resolve clang modules, those come pre-resolved
2017+
// from the Clang dependency scanner.
2018+
for (const auto &moduleID : discoveredModules)
2019+
if (moduleID.second != ModuleDependencyKind::Clang)
2020+
allModules.insert(moduleID);
2021+
20132022
allModules.insert(discoveredModules.begin(), discoveredModules.end());
20142023
}
20152024

@@ -2161,7 +2170,7 @@ swift::dependencies::performBatchModuleScan(
21612170
currentModuleIdx < allModules.size(); ++currentModuleIdx) {
21622171
auto module = allModules[currentModuleIdx];
21632172
auto discoveredModules =
2164-
resolveDirectDependencies(instance, module, cache, ASTDelegate);
2173+
resolveDependencies(instance, module, cache, ASTDelegate);
21652174
allModules.insert(discoveredModules.begin(), discoveredModules.end());
21662175
}
21672176

‎test/CAS/module_deps.swift‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,8 @@ import SubE
6262
// CHECK-NEXT: module_deps.swift
6363
// CHECK-NEXT: ],
6464
// CHECK-NEXT: "directDependencies": [
65-
// CHECK-DAG: "swift": "A"
6665
// CHECK-DAG: "clang": "C"
6766
// CHECK-DAG: "swift": "E"
68-
// CHECK-DAG: "swift": "F"
6967
// CHECK-DAG: "swift": "G"
7068
// CHECK-DAG: "swift": "SubE"
7169
// CHECK-DAG: "swift": "Swift"
@@ -93,6 +91,10 @@ import SubE
9391
// CHECK-NEXT: "F"
9492
// CHECK-NEXT: ]
9593

94+
// CHECK: "swiftOverlayDependencies": [
95+
// CHECK-DAG: "swift": "A"
96+
// CHECK-DAG: "swift": "F"
97+
9698
/// --------Swift module A
9799
// CHECK-LABEL: "modulePath": "{{.*}}{{/|\\}}A-{{.*}}.swiftmodule",
98100

‎test/CAS/module_deps_include_tree.swift‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,8 @@ import SubE
5757
// CHECK-NEXT: module_deps_include_tree.swift
5858
// CHECK-NEXT: ],
5959
// CHECK-NEXT: "directDependencies": [
60-
// CHECK-DAG: "swift": "A"
6160
// CHECK-DAG: "clang": "C"
6261
// CHECK-DAG: "swift": "E"
63-
// CHECK-DAG: "swift": "F"
6462
// CHECK-DAG: "swift": "G"
6563
// CHECK-DAG: "swift": "SubE"
6664
// CHECK-DAG: "swift": "Swift"
@@ -87,6 +85,10 @@ import SubE
8785
// CHECK-NEXT: "F"
8886
// CHECK-NEXT: ]
8987

88+
// CHECK: "swiftOverlayDependencies": [
89+
// CHECK-DAG: "swift": "A"
90+
// CHECK-DAG: "swift": "F"
91+
9092
/// --------Swift module A
9193
// CHECK-LABEL: "modulePath": "{{.*}}{{/|\\}}A-{{.*}}.swiftmodule",
9294

‎test/CAS/plugin_cas.swift‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,8 @@ import SubE
4848
// CHECK-NEXT: plugin_cas.swift
4949
// CHECK-NEXT: ],
5050
// CHECK-NEXT: "directDependencies": [
51-
// CHECK-DAG: "swift": "A"
5251
// CHECK-DAG: "clang": "C"
5352
// CHECK-DAG: "swift": "E"
54-
// CHECK-DAG: "swift": "F"
5553
// CHECK-DAG: "swift": "G"
5654
// CHECK-DAG: "swift": "SubE"
5755
// CHECK-DAG: "swift": "Swift"
@@ -78,6 +76,10 @@ import SubE
7876
// CHECK-NEXT: "F"
7977
// CHECK-NEXT: ]
8078

79+
// CHECK: "swiftOverlayDependencies": [
80+
// CHECK-DAG: "swift": "A"
81+
// CHECK-DAG: "swift": "F"
82+
8183
/// --------Swift module A
8284
// CHECK-LABEL: "modulePath": "{{.*}}{{/|\\}}A-{{.*}}.swiftmodule",
8385

‎test/ScanDependencies/module_deps_cache_reuse.swift‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,9 @@ import SubE
3131
// CHECK-NEXT: ],
3232
// CHECK-NEXT: "directDependencies": [
3333
// CHECK-NEXT: {
34-
// CHECK-DAG: "swift": "A"
3534
// CHECK-DAG: "clang": "C"
3635
// CHECK-DAG: "swift": "E"
37-
// CHECK-DAG: "swift": "F"
36+
// CHECK-DAG: "clang": "F"
3837
// CHECK-DAG: "swift": "G"
3938
// CHECK-DAG: "swift": "SubE"
4039
// CHECK-DAG: "swift": "Swift"
@@ -63,6 +62,10 @@ import SubE
6362
// CHECK-NEXT: "F"
6463
// CHECK-NEXT: ]
6564

65+
// CHECK: "swiftOverlayDependencies": [
66+
// CHECK-DAG: "swift": "F"
67+
// CHECK-DAG: "swift": "A"
68+
6669
/// --------Swift module A
6770
// CHECK-LABEL: "modulePath": "{{.*}}{{/|\\}}A-{{.*}}.swiftmodule",
6871

‎test/ScanDependencies/module_deps_external.swift‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ import SomeExternalModule
4444

4545
// CHECK: directDependencies
4646
// CHECK-NEXT: {
47-
// CHECK-DAG: "swift": "F"
47+
// CHECK-DAG: "clang": "F"
4848
// CHECK-DAG: "swiftPlaceholder": "SomeExternalModule"
4949
// CHECK-DAG: "swift": "Swift"
5050
// CHECK-DAG: "swift": "SwiftOnoneSupport"
@@ -53,12 +53,6 @@ import SomeExternalModule
5353
// CHECK-DAG: "clang": "_SwiftConcurrencyShims"
5454
// CHECK: ],
5555

56-
// CHECK: "extraPcmArgs": [
57-
// CHECK-NEXT: "-Xcc",
58-
// CHECK-NEXT: "-target",
59-
// CHECK-NEXT: "-Xcc",
60-
// CHECK: "-fapinotes-swift-version=4"
61-
6256
// CHECK: "bridgingHeader":
6357
// CHECK-NEXT: "path":
6458
// CHECK-SAME: Bridging.h
@@ -67,9 +61,21 @@ import SomeExternalModule
6761
// CHECK-NEXT: Bridging.h
6862
// CHECK-NEXT: BridgingOther.h
6963

70-
// CHECK: "moduleDependencies": [
71-
// CHECK-NEXT: "F"
72-
// CHECK-NEXT: ]
64+
// CHECK: "moduleDependencies": [
65+
// CHECK-NEXT: "F"
66+
// CHECK-NEXT: ],
67+
68+
// CHECK: "swiftOverlayDependencies": [
69+
// CHECK-NEXT: {
70+
// CHECK-NEXT: "swift": "F"
71+
// CHECK-NEXT: }
72+
// CHECK-NEXT: ]
73+
74+
// CHECK: "extraPcmArgs": [
75+
// CHECK-NEXT: "-Xcc",
76+
// CHECK-NEXT: "-target",
77+
// CHECK-NEXT: "-Xcc",
78+
// CHECK: "-fapinotes-swift-version=4"
7379

7480
/// --------Swift module Swift
7581
// CHECK-LABEL: "modulePath": "{{.*}}{{/|\\}}Swift-{{.*}}.swiftmodule",

0 commit comments

Comments
 (0)