[Dependency Scanning] Perform cross-import overlay resolution per source-file

artemcm · artemcm · commit a756e2110f73 · 2025-04-08T12:57:27.000-07:00
Previous implementation took the entire transitive dependency set and cross-referenced all of its members to determine which ones introduce requried cross-import overlays. That implementation differed from the cross-import overlay loading logic during source compilation, where a corrsponding cross-import overlay module is only requested if the two constituent modules are reachable via direct 'import's from *the same source file*. Meaning the dependency scanner before this change would report cross-import overlay dependencies which never got loaded by the corresponding client source compile.

This change implements a new implementation of cross-import overlay discovery which first computes sub-graphs of module dependencies directly reachable by 'import's for each source file of the module under scan and then performs pairwise cross-import overlay query per each such sub-graph.

Resolves rdar://145157171
diff --git a/include/swift/DependencyScan/ModuleDependencyScanner.h b/include/swift/DependencyScan/ModuleDependencyScanner.h
@@ -100,52 +100,47 @@ class ModuleDependencyScanner {
   /// closure for the given module.
   /// 1. Swift modules imported directly or via another Swift dependency
   /// 2. Clang modules imported directly or via a Swift dependency
-  /// 3. Clang modules imported via textual header inputs to Swift modules (bridging headers)
-  /// 4. Swift overlay modules of all of the transitively imported Clang modules that have one
+  /// 3. Clang modules imported via textual header inputs to Swift modules
+  /// (bridging headers)
+  /// 4. Swift overlay modules of all of the transitively imported Clang modules
+  /// that have one
   ModuleDependencyIDSetVector
   resolveImportedModuleDependencies(const ModuleDependencyID &rootModuleID,
                                     ModuleDependenciesCache &cache);
-  void
-  resolveSwiftModuleDependencies(const ModuleDependencyID &rootModuleID,
-                                 ModuleDependenciesCache &cache,
-                                 ModuleDependencyIDSetVector &discoveredSwiftModules);
-  void
-  resolveAllClangModuleDependencies(ArrayRef<ModuleDependencyID> swiftModules,
-                                    ModuleDependenciesCache &cache,
-                                    ModuleDependencyIDSetVector &discoveredClangModules);
-  void
-  resolveHeaderDependencies(ArrayRef<ModuleDependencyID> swiftModules,
-                            ModuleDependenciesCache &cache,
-                            ModuleDependencyIDSetVector &discoveredHeaderDependencyClangModules);
-  void
-  resolveSwiftOverlayDependencies(ArrayRef<ModuleDependencyID> swiftModules,
-                                  ModuleDependenciesCache &cache,
-                                  ModuleDependencyIDSetVector &discoveredDependencies);
+  void resolveSwiftModuleDependencies(
+      const ModuleDependencyID &rootModuleID, ModuleDependenciesCache &cache,
+      ModuleDependencyIDSetVector &discoveredSwiftModules);
+  void resolveAllClangModuleDependencies(
+      ArrayRef<ModuleDependencyID> swiftModules, ModuleDependenciesCache &cache,
+      ModuleDependencyIDSetVector &discoveredClangModules);
+  void resolveHeaderDependencies(
+      ArrayRef<ModuleDependencyID> swiftModules, ModuleDependenciesCache &cache,
+      ModuleDependencyIDSetVector &discoveredHeaderDependencyClangModules);
+  void resolveSwiftOverlayDependencies(
+      ArrayRef<ModuleDependencyID> swiftModules, ModuleDependenciesCache &cache,
+      ModuleDependencyIDSetVector &discoveredDependencies);
 
   /// Resolve all of a given module's imports to a Swift module, if one exists.
-  void
-  resolveSwiftImportsForModule(const ModuleDependencyID &moduleID,
-                               ModuleDependenciesCache &cache,
-                               ModuleDependencyIDSetVector &importedSwiftDependencies);
+  void resolveSwiftImportsForModule(
+      const ModuleDependencyID &moduleID, ModuleDependenciesCache &cache,
+      ModuleDependencyIDSetVector &importedSwiftDependencies);
 
-  /// If a module has a bridging header or other header inputs, execute a dependency scan
-  /// on it and record the dependencies.
+  /// If a module has a bridging header or other header inputs, execute a
+  /// dependency scan on it and record the dependencies.
   void resolveHeaderDependenciesForModule(
       const ModuleDependencyID &moduleID, ModuleDependenciesCache &cache,
       ModuleDependencyIDSetVector &headerClangModuleDependencies);
 
   /// Resolve all module dependencies comprised of Swift overlays
   /// of this module's Clang module dependencies.
   void resolveSwiftOverlayDependenciesForModule(
-      const ModuleDependencyID &moduleID,
-      ModuleDependenciesCache &cache,
+      const ModuleDependencyID &moduleID, ModuleDependenciesCache &cache,
       ModuleDependencyIDSetVector &swiftOverlayDependencies);
 
-  /// Identify all cross-import overlay modules of the specified
-  /// dependency set and apply an action for each.
-  void discoverCrossImportOverlayDependencies(
-      StringRef mainModuleName, ArrayRef<ModuleDependencyID> allDependencies,
-      ModuleDependenciesCache &cache,
+  /// Identify all cross-import overlay module dependencies of the
+  /// source module under scan and apply an action for each.
+  void resolveCrossImportOverlayDependencies(
+      StringRef mainModuleName, ModuleDependenciesCache &cache,
       llvm::function_ref<void(ModuleDependencyID)> action);
 
   /// Perform an operation utilizing one of the Scanning workers
diff --git a/lib/DependencyScan/ModuleDependencyScanner.cpp b/lib/DependencyScan/ModuleDependencyScanner.cpp
@@ -532,42 +532,141 @@ ModuleDependencyScanner::getNamedSwiftModuleDependencyInfo(
   return cache.findDependency(moduleName);
 }
 
+/// For the dependency set of the main module, discover all
+/// cross-import overlays and their corresponding '.swiftcrossimport'
+/// files. Cross-import overlay dependencies are required when
+/// the two constituent modules are imported *from the same source file*,
+/// directly or indirectly.
+///
+/// Given a complete module dependency graph in this stage of the scan,
+/// the algorithm for discovering cross-import overlays is:
+/// 1. For each source file of the module under scan construct a
+///    set of module dependnecies only reachable from this source file.
+/// 2. For each module set constructed in (1), perform pair-wise lookup
+///    of cross import files for each pair of modules in the set.
+///
+/// Notably, if for some pair of modules 'A' and 'B' there exists
+/// a cross-import overlay '_A_B', and these two modules are not reachable
+/// from any single source file via direct or indirect imports, then
+/// the cross-import overlay module is not required for compilation.
 static void discoverCrossImportOverlayFiles(
-    StringRef mainModuleName, ArrayRef<ModuleDependencyID> allDependencies,
-    ModuleDependenciesCache &cache, ASTContext &scanASTContext,
-    llvm::SetVector<Identifier> &newOverlays,
+    StringRef mainModuleName, ModuleDependenciesCache &cache,
+    ASTContext &scanASTContext, llvm::SetVector<Identifier> &newOverlays,
     std::vector<std::pair<std::string, std::string>> &overlayFiles) {
-  for (auto moduleID : allDependencies) {
-    auto moduleName = moduleID.ModuleName;
-    // Do not look for overlays of main module under scan
-    if (moduleName == mainModuleName)
-      continue;
+  auto mainModuleInfo = cache.findKnownDependency(ModuleDependencyID{
+      mainModuleName.str(), ModuleDependencyKind::SwiftSource});
+
+  llvm::StringMap<ModuleDependencyIDSet> perSourceFileDependencies;
+  const ModuleDependencyIDSet directSwiftDepsSet{
+      mainModuleInfo.getImportedSwiftDependencies().begin(),
+      mainModuleInfo.getImportedSwiftDependencies().end()};
+  const ModuleDependencyIDSet directClangDepsSet{
+      mainModuleInfo.getImportedClangDependencies().begin(),
+      mainModuleInfo.getImportedClangDependencies().end()};
+
+  // A utility to map an import identifier to one of the
+  // known resolved module dependencies
+  auto getModuleIDForImportIdentifier =
+      [directSwiftDepsSet, directClangDepsSet](
+          const std::string &importIdentifierStr) -> ModuleDependencyID {
+    if (auto textualDepIt = directSwiftDepsSet.find(
+            {importIdentifierStr, ModuleDependencyKind::SwiftInterface});
+        textualDepIt != directSwiftDepsSet.end())
+      return *textualDepIt;
+    else if (auto binaryDepIt = directSwiftDepsSet.find(
+                 {importIdentifierStr, ModuleDependencyKind::SwiftBinary});
+             binaryDepIt != directSwiftDepsSet.end())
+      return *binaryDepIt;
+    else if (auto clangDepIt = directClangDepsSet.find(
+                 {importIdentifierStr, ModuleDependencyKind::Clang});
+             clangDepIt != directClangDepsSet.end())
+      return *clangDepIt;
+    llvm_unreachable(
+        "Unresolved import during cross-import overlay resolution");
+  };
 
-    auto dependencies = cache.findDependency(moduleName, moduleID.Kind).value();
-    // Collect a map from secondary module name to cross-import overlay names.
-    auto overlayMap = dependencies->collectCrossImportOverlayNames(
-        scanASTContext, moduleName, overlayFiles);
-    if (overlayMap.empty())
-      continue;
-    for (const auto &dependencyId : allDependencies) {
-      auto moduleName = dependencyId.ModuleName;
-      // Do not look for overlays of main module under scan
-      if (moduleName == mainModuleName)
-        continue;
-      // check if any explicitly imported modules can serve as a
-      // secondary module, and add the overlay names to the
-      // dependencies list.
-      for (auto overlayName : overlayMap[moduleName]) {
-        if (overlayName.str() != mainModuleName &&
-            std::find_if(allDependencies.begin(), allDependencies.end(),
-                         [&](ModuleDependencyID Id) {
-                           return moduleName == overlayName.str();
-                         }) == allDependencies.end()) {
-          newOverlays.insert(overlayName);
+  // Collect the set of directly-imported module dependencies
+  // for each source file in the source module under scan.
+  for (const auto &import : mainModuleInfo.getModuleImports()) {
+    auto importResolvedModuleID =
+        getModuleIDForImportIdentifier(import.importIdentifier);
+    for (const auto &importLocation : import.importLocations)
+      perSourceFileDependencies[importLocation.bufferIdentifier].insert(
+          importResolvedModuleID);
+  }
+
+  // For each source-file, build a set of module dependencies of the
+  // module under scan corresponding to a sub-graph of modules only reachable
+  // from this source file's direct imports.
+  for (auto &keyValPair : perSourceFileDependencies) {
+    const auto &bufferIdentifier = keyValPair.getKey();
+    auto &directDependencyIDs = keyValPair.second;
+    SmallVector<ModuleDependencyID, 8> worklist{directDependencyIDs.begin(),
+                                                directDependencyIDs.end()};
+    while (!worklist.empty()) {
+      auto moduleID = worklist.pop_back_val();
+      perSourceFileDependencies[bufferIdentifier].insert(moduleID);
+      if (isSwiftDependencyKind(moduleID.Kind)) {
+        for (const auto &directSwiftDepID :
+             cache.getImportedSwiftDependencies(moduleID)) {
+          if (perSourceFileDependencies[bufferIdentifier].count(directSwiftDepID))
+            continue;
+          worklist.push_back(directSwiftDepID);
         }
       }
+      for (const auto &directClangDepID :
+           cache.getImportedClangDependencies(moduleID)) {
+        if (perSourceFileDependencies[bufferIdentifier].count(directClangDepID))
+          continue;
+        worklist.push_back(directClangDepID);
+      }
     }
   }
+
+  // Within a provided set of module dependencies reachable via
+  // direct imports from a given file, determine the available and required
+  // cross-import overlays.
+  auto discoverCrossImportOverlayFilesForModuleSet =
+      [&mainModuleName, &cache, &scanASTContext, &newOverlays,
+       &overlayFiles](const ModuleDependencyIDSet &inputDependencies) {
+        for (auto moduleID : inputDependencies) {
+          auto moduleName = moduleID.ModuleName;
+          // Do not look for overlays of main module under scan
+          if (moduleName == mainModuleName)
+            continue;
+
+          auto dependencies =
+              cache.findDependency(moduleName, moduleID.Kind).value();
+          // Collect a map from secondary module name to cross-import overlay
+          // names.
+          auto overlayMap = dependencies->collectCrossImportOverlayNames(
+              scanASTContext, moduleName, overlayFiles);
+          if (overlayMap.empty())
+            continue;
+          for (const auto &dependencyId : inputDependencies) {
+            auto moduleName = dependencyId.ModuleName;
+            // Do not look for overlays of main module under scan
+            if (moduleName == mainModuleName)
+              continue;
+            // check if any explicitly imported modules can serve as a
+            // secondary module, and add the overlay names to the
+            // dependencies list.
+            for (auto overlayName : overlayMap[moduleName]) {
+              if (overlayName.str() != mainModuleName &&
+                  std::find_if(inputDependencies.begin(),
+                               inputDependencies.end(),
+                               [&](ModuleDependencyID Id) {
+                                 return moduleName == overlayName.str();
+                               }) == inputDependencies.end()) {
+                newOverlays.insert(overlayName);
+              }
+            }
+          }
+        }
+      };
+
+  for (const auto &keyValPair : perSourceFileDependencies)
+    discoverCrossImportOverlayFilesForModuleSet(keyValPair.second);
 }
 
 std::vector<ModuleDependencyID>
@@ -598,8 +697,8 @@ ModuleDependencyScanner::performDependencyScan(
   // binary Swift modules already encode their dependencies on cross-import overlays
   // with explicit imports.
   if (ScanCompilerInvocation.getLangOptions().EnableCrossImportOverlays)
-    discoverCrossImportOverlayDependencies(
-        rootModuleID.ModuleName, allModules.getArrayRef().slice(1), cache,
+    resolveCrossImportOverlayDependencies(
+        rootModuleID.ModuleName, cache,
         [&](ModuleDependencyID id) { allModules.insert(id); });
 
   return allModules.takeVector();
@@ -1154,15 +1253,15 @@ void ModuleDependencyScanner::resolveSwiftOverlayDependenciesForModule(
   cache.setSwiftOverlayDependencies(moduleID, swiftOverlayDependencies.getArrayRef());
 }
 
-void ModuleDependencyScanner::discoverCrossImportOverlayDependencies(
-    StringRef mainModuleName, ArrayRef<ModuleDependencyID> allDependencies,
+void ModuleDependencyScanner::resolveCrossImportOverlayDependencies(
+    StringRef mainModuleName,
     ModuleDependenciesCache &cache,
     llvm::function_ref<void(ModuleDependencyID)> action) {
   // Modules explicitly imported. Only these can be secondary module.
   llvm::SetVector<Identifier> newOverlays;
   std::vector<std::pair<std::string, std::string>> overlayFiles;
-  discoverCrossImportOverlayFiles(mainModuleName, allDependencies, cache,
-                                  ScanASTContext, newOverlays, overlayFiles);
+  discoverCrossImportOverlayFiles(mainModuleName, cache, ScanASTContext,
+                                  newOverlays, overlayFiles);
 
   // No new cross-import overlays are found, return.
   if (newOverlays.empty())
@@ -1184,11 +1283,10 @@ void ModuleDependencyScanner::discoverCrossImportOverlayDependencies(
 
   // Record the dummy main module's direct dependencies. The dummy main module
   // only directly depend on these newly discovered overlay modules.
-  if (cache.findDependency(dummyMainName, ModuleDependencyKind::SwiftSource)) {
+  if (cache.findDependency(dummyMainID))
     cache.updateDependency(dummyMainID, dummyMainDependencies);
-  } else {
+  else
     cache.recordDependency(dummyMainName, dummyMainDependencies);
-  }
 
   ModuleDependencyIDSetVector allModules =
     resolveImportedModuleDependencies(dummyMainID, cache);
diff --git a/test/CAS/cross_import.swift b/test/CAS/cross_import.swift
@@ -32,11 +32,11 @@
 // RUN: %{python} %S/Inputs/BuildCommandExtractor.py %t/deps.json Test > %t/MyApp.cmd
 // RUN: %FileCheck %s --input-file=%t/MyApp.cmd --check-prefix CMD
 // CMD: -swift-module-cross-import
-// CMD-NEXT: B
-// CMD-NEXT: A.swiftoverlay
-// CMD-NEXT: -swift-module-cross-import
-// CMD-NEXT: C
-// CMD-NEXT: A.swiftoverlay
+// CMD-NEXT: [[CMI1:[B|C]]]
+// CMD-NEXT: [[CMI1]].swiftcrossimport{{/|\\}}A.swiftoverlay
+// CMD: -swift-module-cross-import
+// CMD-NEXT: [[CMI2:[B|C]]]
+// CMD-NEXT: [[CMI2]].swiftcrossimport{{/|\\}}A.swiftoverlay
 
 // RUN: %target-swift-frontend -emit-module -o %t/Test.swiftmodule  \
 // RUN:   -emit-module-interface-path %t/Test.swiftinterface \
diff --git a/test/ScanDependencies/fine_grained_cross_import_overlays.swift b/test/ScanDependencies/fine_grained_cross_import_overlays.swift
@@ -0,0 +1,26 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/module-cache)
+// RUN: split-file %s %t
+
+// Run a dependency scan on a source file that imports both constituents of a cross-import overlay to ensure the cross-import overlay dependency is registered
+// RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/module-cache %t/C.swift -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -module-name FineGrainedCrossImportTestModule -enable-cross-import-overlays
+// Check the contents of the JSON output
+// RUN: %validate-json %t/deps.json | %FileCheck %s --check-prefix CHECK-TOGETHER
+
+// Run a dependency scan on two source files that separately import constituents of a cross-import overlay and ensure that the cross-import overlay dependency is *not* registered
+// RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/module-cache %t/A.swift %t/B.swift -o %t/deps_disjoint.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -module-name FineGrainedCrossImportTestModule -enable-cross-import-overlays
+// Check the contents of the JSON output
+// RUN: %validate-json %t/deps_disjoint.json | %FileCheck %s --check-prefix CHECK-DISJOINT
+
+//--- A.swift
+import E
+
+//--- B.swift
+import SubE
+
+//--- C.swift
+import E
+import SubE
+
+// CHECK-TOGETHER: "swift": "_cross_import_E"
+// CHECK-DISJOINT-NOT: "swift": "_cross_import_E"
