Skip to content

Commit c7b6bcc

Browse files
artemcmartemcm
committed
[Dependency Scanning] Refine cross-import overlay detection algorithm
The algorithm already performs pairwise checks on module dependencies brought into compilation per-source-file. Previously, the algorithm considered the entire sub-graph of a given source file. Actual source compiles do not consider the full transitive module dependency set for cross-import-overlay lookup, but rather only directly-imported modules in a given source file, and '@_exported import' Swift transitive dependencies. This change adds tracking of whether a given import statement is 'exported' to the dependency scanner and then refines the cross-import overlay lookup logic to only consider transitive modules that are exported by directly-imported dependencies.
1 parent ce65b15 commit c7b6bcc

9 files changed

+60
-24
lines changed

‎include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ using IsFrameworkField = BCFixed<1>;
5757
using IsSystemField = BCFixed<1>;
5858
/// A bit that indicates whether or not a module is that of a static archive
5959
using IsStaticField = BCFixed<1>;
60-
/// A bit taht indicates whether or not an import statement is @_exported
60+
/// A bit that indicates whether or not an import statement is @_exported
6161
using IsExportedImport = BCFixed<1>;
6262

6363
/// Source location fields

‎lib/AST/ModuleDependencies.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ void ModuleDependencyInfo::addModuleImport(
124124
StringRef module, bool isExported, llvm::StringSet<> *alreadyAddedModules,
125125
const SourceManager *sourceManager, SourceLoc sourceLocation) {
126126
auto scannerImportLocToDiagnosticLocInfo =
127-
[&sourceManager, isExported](SourceLoc sourceLocation) {
127+
[&sourceManager](SourceLoc sourceLocation) {
128128
auto lineAndColumnNumbers =
129129
sourceManager->getLineAndColumnInBuffer(sourceLocation);
130130
return ScannerImportStatementInfo::ImportDiagnosticLocationInfo(

‎lib/ClangImporter/ClangModuleDependencyScanner.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,10 @@ ModuleDependencyVector ClangImporter::bridgeClangModuleDependencies(
302302

303303
std::vector<ModuleDependencyID> directDependencyIDs;
304304
for (const auto &moduleName : clangModuleDep.ClangModuleDeps) {
305-
dependencies.addModuleImport(moduleName.ModuleName, &alreadyAddedModules);
305+
// FIXME: This assumes, conservatively, that all Clang module imports
306+
// are exported. We need to fix this once the clang scanner gains the appropriate
307+
// API to query this.
308+
dependencies.addModuleImport(moduleName.ModuleName, /* isExported */ true, &alreadyAddedModules);
306309
// It is safe to assume that all dependencies of a Clang module are Clang modules.
307310
directDependencyIDs.push_back({moduleName.ModuleName, ModuleDependencyKind::Clang});
308311
}

‎lib/DependencyScan/ModuleDependencyScanner.cpp

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -561,18 +561,19 @@ static void discoverCrossImportOverlayFiles(
561561
mainModuleName.str(), ModuleDependencyKind::SwiftSource});
562562

563563
llvm::StringMap<ModuleDependencyIDSet> perSourceFileDependencies;
564-
const ModuleDependencyIDSet directSwiftDepsSet{
564+
const ModuleDependencyIDSet mainModuleDirectSwiftDepsSet{
565565
mainModuleInfo.getImportedSwiftDependencies().begin(),
566566
mainModuleInfo.getImportedSwiftDependencies().end()};
567-
const ModuleDependencyIDSet directClangDepsSet{
567+
const ModuleDependencyIDSet mainModuleDirectClangDepsSet{
568568
mainModuleInfo.getImportedClangDependencies().begin(),
569569
mainModuleInfo.getImportedClangDependencies().end()};
570570

571571
// A utility to map an import identifier to one of the
572572
// known resolved module dependencies
573573
auto getModuleIDForImportIdentifier =
574-
[directSwiftDepsSet, directClangDepsSet](
575-
const std::string &importIdentifierStr) -> ModuleDependencyID {
574+
[](const std::string &importIdentifierStr,
575+
const ModuleDependencyIDSet &directSwiftDepsSet,
576+
const ModuleDependencyIDSet &directClangDepsSet) -> ModuleDependencyID {
576577
if (auto textualDepIt = directSwiftDepsSet.find(
577578
{importIdentifierStr, ModuleDependencyKind::SwiftInterface});
578579
textualDepIt != directSwiftDepsSet.end())
@@ -592,8 +593,9 @@ static void discoverCrossImportOverlayFiles(
592593
// Collect the set of directly-imported module dependencies
593594
// for each source file in the source module under scan.
594595
for (const auto &import : mainModuleInfo.getModuleImports()) {
595-
auto importResolvedModuleID =
596-
getModuleIDForImportIdentifier(import.importIdentifier);
596+
auto importResolvedModuleID = getModuleIDForImportIdentifier(
597+
import.importIdentifier, mainModuleDirectSwiftDepsSet,
598+
mainModuleDirectClangDepsSet);
597599
for (const auto &importLocation : import.importLocations)
598600
perSourceFileDependencies[importLocation.bufferIdentifier].insert(
599601
importResolvedModuleID);
@@ -611,19 +613,29 @@ static void discoverCrossImportOverlayFiles(
611613
auto moduleID = worklist.pop_back_val();
612614
perSourceFileDependencies[bufferIdentifier].insert(moduleID);
613615
if (isSwiftDependencyKind(moduleID.Kind)) {
614-
for (const auto &directSwiftDepID :
615-
cache.getImportedSwiftDependencies(moduleID)) {
616-
if (perSourceFileDependencies[bufferIdentifier].count(directSwiftDepID))
617-
continue;
618-
worklist.push_back(directSwiftDepID);
616+
auto moduleInfo = cache.findKnownDependency(moduleID);
617+
if (llvm::any_of(moduleInfo.getModuleImports(),
618+
[](const ScannerImportStatementInfo &importInfo) {
619+
return importInfo.isExported;
620+
})) {
621+
const ModuleDependencyIDSet directSwiftDepsSet{
622+
moduleInfo.getImportedSwiftDependencies().begin(),
623+
moduleInfo.getImportedSwiftDependencies().end()};
624+
const ModuleDependencyIDSet directClangDepsSet{
625+
moduleInfo.getImportedClangDependencies().begin(),
626+
moduleInfo.getImportedClangDependencies().end()};
627+
for (const auto &import : moduleInfo.getModuleImports()) {
628+
if (import.isExported) {
629+
auto importResolvedDepID = getModuleIDForImportIdentifier(
630+
import.importIdentifier, directSwiftDepsSet,
631+
directClangDepsSet);
632+
if (!perSourceFileDependencies[bufferIdentifier].count(
633+
importResolvedDepID))
634+
worklist.push_back(importResolvedDepID);
635+
}
636+
}
619637
}
620638
}
621-
for (const auto &directClangDepID :
622-
cache.getImportedClangDependencies(moduleID)) {
623-
if (perSourceFileDependencies[bufferIdentifier].count(directClangDepID))
624-
continue;
625-
worklist.push_back(directClangDepID);
626-
}
627639
}
628640
}
629641

‎test/ScanDependencies/Inputs/Swift/EWrapper.swiftinterface

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// swift-interface-format-version: 1.0
22
// swift-module-flags: -module-name EWrapper
33
import Swift
4-
import E
4+
@_exported import E
55
public func funcEWrapper() { }

‎test/ScanDependencies/Inputs/Swift/EWrapperNonExported.swiftinterface

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// swift-interface-format-version: 1.0
2+
// swift-module-flags: -module-name EWrapperNonExported
3+
import Swift
4+
import E
5+
public func funcEWrapperNonExported() { }

‎test/ScanDependencies/Inputs/Swift/SubEWrapper.swiftinterface

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// swift-interface-format-version: 1.0
22
// swift-module-flags: -module-name SubEWrapper
33
import Swift
4-
import SubE
4+
@_exported import SubE
55
public func funcSubEWrapper() { }

‎test/ScanDependencies/module_deps_cross_import_overlay.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
// RUN: %empty-directory(%t)
22
// RUN: mkdir -p %t/clang-module-cache
3-
// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -emit-dependencies -emit-dependencies-path %t/deps.d -import-objc-header %S/Inputs/CHeaders/Bridging.h -swift-version 4 -module-name CrossImportTestModule -enable-cross-import-overlays
3+
// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -import-objc-header %S/Inputs/CHeaders/Bridging.h -swift-version 4 -module-name CrossImportTestModule -enable-cross-import-overlays
44
// Check the contents of the JSON output
55
// RUN: %validate-json %t/deps.json | %FileCheck %s
66

77
// Ensure that round-trip serialization does not affect result
8-
// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -test-dependency-scan-cache-serialization -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -emit-dependencies -emit-dependencies-path %t/deps.d -import-objc-header %S/Inputs/CHeaders/Bridging.h -swift-version 4 -module-name CrossImportTestModule -enable-cross-import-overlays
8+
// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -test-dependency-scan-cache-serialization -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -import-objc-header %S/Inputs/CHeaders/Bridging.h -swift-version 4 -module-name CrossImportTestModule -enable-cross-import-overlays
99
// RUN: %validate-json %t/deps.json | %FileCheck %s
1010

1111
// REQUIRES: executable_test

‎test/ScanDependencies/module_deps_no_cross_import_overlay_transitive.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// REQUIRES: objc_interop
2+
// RUN: %empty-directory(%t)
3+
// RUN: %empty-directory(%t/clang-module-cache)
4+
// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -module-name CrossImportTestModule -enable-cross-import-overlays
5+
// Check the contents of the JSON output
6+
// RUN: %validate-json %t/deps.json | %FileCheck %s
7+
8+
// Ensure that round-trip serialization does not affect result
9+
// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -test-dependency-scan-cache-serialization -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -module-name CrossImportTestModule -enable-cross-import-overlays
10+
// RUN: %validate-json %t/deps.json | %FileCheck %s
11+
12+
// Ensure that if one of the two components of a cross-import overlay is a transitive (non-exported) Swift dependency, then no overlay is required to be brought in.
13+
import EWrapperNonExported
14+
import SubEWrapper
15+
16+
// CHECK-NOT: "swift": "_cross_import_E"

0 commit comments

Comments
 (0)