[Explicit Module Builds] Centralize computing *all* dependencies of a given 'ModuleInfo' and use throughout

artemcm · artemcm · commit 0c2ef2161390 · 2025-04-14T13:45:19.000-07:00
This is in preparation of dependency scanner turning the 'directDependencies' field to contain only actually-directly-imported modules.
diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift
@@ -510,7 +510,7 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT
                                 clangDependencyArtifacts: &clangDependencyArtifacts,
                                 swiftDependencyArtifacts: &swiftDependencyArtifacts)
       let depInfo = try dependencyGraph.moduleInfo(of: bridgingHeaderDepID)
-      dependenciesWorklist.append(contentsOf: depInfo.directDependencies ?? [])
+      dependenciesWorklist.append(contentsOf: depInfo.allDependencies)
     }
 
     // Clang module dependencies are specified on the command line explicitly
diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/InterModuleDependencies/CommonDependencyOperations.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/InterModuleDependencies/CommonDependencyOperations.swift
@@ -59,16 +59,7 @@ import protocol TSCBasic.FileSystem
 extension InterModuleDependencyGraph {
   var topologicalSorting: [ModuleDependencyId] {
     get throws {
-      try topologicalSort(Array(modules.keys),
-                          successors: {
-        var dependencies: [ModuleDependencyId] = []
-        let moduleInfo = try moduleInfo(of: $0)
-        dependencies.append(contentsOf: moduleInfo.directDependencies ?? [])
-        if case .swift(let swiftModuleDetails) = moduleInfo.details {
-          dependencies.append(contentsOf: swiftModuleDetails.swiftOverlayDependencies ?? [])
-        }
-        return dependencies
-      })
+      try topologicalSort(Array(modules.keys), successors: { try Array(moduleInfo(of: $0).allDependencies) })
     }
   }
 
@@ -91,22 +82,9 @@ extension InterModuleDependencyGraph {
       }
     // Traverse the set of modules in reverse topological order, assimilating transitive closures
     for moduleId in topologicalIdList.reversed() {
-      let moduleInfo = try moduleInfo(of: moduleId)
-      for dependencyId in moduleInfo.directDependencies! {
+      for dependencyId in try moduleInfo(of: moduleId).allDependencies {
         transitiveClosureMap[moduleId]!.formUnion(transitiveClosureMap[dependencyId]!)
       }
-      // For Swift dependencies, their corresponding Swift Overlay dependencies
-      // and bridging header dependencies are equivalent to direct dependencies.
-      if case .swift(let swiftModuleDetails) = moduleInfo.details {
-        let swiftOverlayDependencies = swiftModuleDetails.swiftOverlayDependencies ?? []
-        for dependencyId in swiftOverlayDependencies {
-          transitiveClosureMap[moduleId]!.formUnion(transitiveClosureMap[dependencyId]!)
-        }
-        let bridgingHeaderDependencies = swiftModuleDetails.bridgingHeaderDependencies ?? []
-        for dependencyId in bridgingHeaderDependencies {
-          transitiveClosureMap[moduleId]!.formUnion(transitiveClosureMap[dependencyId]!)
-        }
-      }
     }
     // For ease of use down-the-line, remove the node's self from its set of reachable nodes
     for (key, _) in transitiveClosureMap {
@@ -167,7 +145,7 @@ internal extension InterModuleDependencyGraph {
     var visited: Set<ModuleDependencyId> = []
     // Scan from the main module's dependencies to avoid reporting
     // the main module itself in the results.
-    for dependencyId in mainModuleInfo.directDependencies ?? [] {
+    for dependencyId in mainModuleInfo.allDependencies {
       try outOfDateModuleScan(from: dependencyId, visited: &visited,
                               modulesRequiringRebuild: &modulesRequiringRebuild,
                               fileSystem: fileSystem, cas: cas, forRebuild: forRebuild,
@@ -225,7 +203,7 @@ internal extension InterModuleDependencyGraph {
     let sourceModuleInfo = try moduleInfo(of: sourceModuleId)
     // Visit the module's dependencies
     var hasOutOfDateModuleDependency = false
-    for dependencyId in sourceModuleInfo.directDependencies ?? [] {
+    for dependencyId in sourceModuleInfo.allDependencies {
       // If we have not already visited this module, recurse.
       if !visited.contains(dependencyId) {
         try outOfDateModuleScan(from: dependencyId, visited: &visited,
@@ -319,7 +297,7 @@ internal extension InterModuleDependencyGraph {
     }
 
     // Check if a dependency of this module has a newer output than this module
-    for dependencyId in checkedModuleInfo.directDependencies ?? [] {
+    for dependencyId in checkedModuleInfo.allDependencies {
       let dependencyInfo = try moduleInfo(of: dependencyId)
       if !verifyInputOlderThanOutputModTime(moduleID.moduleName,
                                             VirtualPath.lookup(dependencyInfo.modulePath.path),
@@ -409,21 +387,14 @@ internal extension InterModuleDependencyGraph {
       // depends on a corresponding Clang module with the same name.
       // If it does, add it to the path as well.
       var completePath = pathSoFar
-      if let dependencies = sourceInfo.directDependencies,
-         dependencies.contains(.clang(source.moduleName)) {
+      if sourceInfo.allDependencies.contains(.clang(source.moduleName)) {
         completePath.append(.clang(source.moduleName))
       }
       result = completePath
       return true
     }
 
-    var allDependencies = sourceInfo.directDependencies ?? []
-    if case .swift(let swiftModuleDetails) = sourceInfo.details,
-          let overlayDependencies = swiftModuleDetails.swiftOverlayDependencies {
-      allDependencies.append(contentsOf: overlayDependencies)
-    }
-
-    for dependency in allDependencies {
+    for dependency in sourceInfo.allDependencies {
       if !visited.contains(dependency),
          try findAPath(source: dependency,
                        pathSoFar: pathSoFar + [dependency],
@@ -446,20 +417,14 @@ internal extension InterModuleDependencyGraph {
       // depends on a corresponding Clang module with the same name.
       // If it does, add it to the path as well.
       var completePath = pathSoFar
-      if let dependencies = sourceInfo.directDependencies,
-         dependencies.contains(.clang(source.moduleName)) {
+      if sourceInfo.allDependencies.contains(.clang(source.moduleName)) {
         completePath.append(.clang(source.moduleName))
       }
       results.insert(completePath)
       return
     }
 
-    var allDependencies = sourceInfo.directDependencies ?? []
-    if case .swift(let swiftModuleDetails) = sourceInfo.details,
-          let overlayDependencies = swiftModuleDetails.swiftOverlayDependencies {
-      allDependencies.append(contentsOf: overlayDependencies)
-    }
-    for dependency in allDependencies {
+    for dependency in sourceInfo.allDependencies {
       try findAllPaths(source: dependency,
                        pathSoFar: pathSoFar + [dependency],
                        results: &results,
diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/InterModuleDependencies/InterModuleDependencyGraph.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/InterModuleDependencies/InterModuleDependencyGraph.swift
@@ -203,7 +203,10 @@ public struct ModuleInfo: Codable, Hashable {
   /// The source files used to build this module.
   public var sourceFiles: [String]?
 
-  /// The set of direct module dependencies of this module.
+  /// The set of directly-imported module dependencies of this module.
+  /// For the complete set of all module dependencies of this module,
+  /// including Swift overlay dependencies and bridging header dependenceis,
+  /// use the `allDependencies` computed property.
   public var directDependencies: [ModuleDependencyId]?
 
   /// The set of libraries that need to be linked
@@ -301,6 +304,35 @@ extension ModuleInfo {
   }
 }
 
+public extension ModuleInfo {
+  // Directly-imported dependencies plus additional dependency
+  // kinds for Swift modules:
+  // - Swift overlay dependencies
+  // - Bridging Header dependencies
+  var allDependencies: [ModuleDependencyId] {
+    var result: [ModuleDependencyId] = directDependencies ?? []
+    if case .swift(let swiftModuleDetails) = details {
+      // Ensure the dependnecies emitted are unique and follow a predictable ordering:
+      // 1. directDependencies in the order reported by the scanner
+      // 2. swift overlay dependencies
+      // 3. briding header dependencies
+      var addedSoFar: Set<ModuleDependencyId> = []
+      addedSoFar.formUnion(directDependencies ?? [])
+      for depId in swiftModuleDetails.swiftOverlayDependencies ?? [] {
+        if addedSoFar.insert(depId).inserted {
+          result.append(depId)
+        }
+      }
+      for depId in swiftModuleDetails.bridgingHeaderDependencies ?? [] {
+        if addedSoFar.insert(depId).inserted {
+          result.append(depId)
+        }
+      }
+    }
+    return result
+  }
+}
+
 /// Describes the complete set of dependencies for a Swift module, including
 /// all of the Swift and C modules and source files it depends on.
 public struct InterModuleDependencyGraph: Codable {
diff --git a/Sources/SwiftDriver/Jobs/PrebuiltModulesJob.swift b/Sources/SwiftDriver/Jobs/PrebuiltModulesJob.swift
@@ -633,7 +633,7 @@ extension InterModuleDependencyGraph {
           if !includingPCM && isPCM(key) {
             break
           }
-          modules[key]!.directDependencies?.forEach { dep in
+          modules[key]!.allDependencies.forEach { dep in
             if !includingPCM && isPCM(dep) {
               return
             }
@@ -814,7 +814,8 @@ extension Driver {
 
     func getSwiftDependencies(for module: String) -> [String] {
       let info = dependencyGraph.modules[.swift(module)]!
-      guard let dependencies = info.directDependencies else {
+      let dependencies = info.allDependencies
+      guard !dependencies.isEmpty else {
         return []
       }
       return collectSwiftModuleNames(dependencies)
@@ -859,31 +860,29 @@ extension Driver {
     }
     // Keep track of modules we haven't handled.
     var unhandledModules = Set<String>(inputMap.keys)
-    if let importedModules = dependencyGraph.mainModule.directDependencies {
-      // Start from those modules explicitly imported into the file under scanning
-      var openModules = collectSwiftModuleNames(importedModules)
-      var idx = 0
-      while idx != openModules.count {
-        let module = openModules[idx]
-        let dependencies = getSwiftDependencies(for: module)
-        try forEachInputOutputPair(module) { input, output in
-          jobs.append(contentsOf: try generateSingleModuleBuildingJob(module,
-            prebuiltModuleDir, input, output,
-            try getOutputPaths(withName: dependencies, loadableFor: input.arch),
-            currentABIDir, baselineABIDir))
-        }
-        // For each dependency, add to the list to handle if the list doesn't
-        // contain this dependency.
-        dependencies.forEach({ newModule in
-          if !openModules.contains(newModule) {
-            diagnosticEngine.emit(.note("\(newModule) is discovered."),
-                                  location: nil)
-            openModules.append(newModule)
-          }
-        })
-        unhandledModules.remove(module)
-        idx += 1
+    // Start from those modules explicitly imported into the file under scanning
+    var openModules = collectSwiftModuleNames(dependencyGraph.mainModule.allDependencies)
+    var idx = 0
+    while idx != openModules.count {
+      let module = openModules[idx]
+      let dependencies = getSwiftDependencies(for: module)
+      try forEachInputOutputPair(module) { input, output in
+        jobs.append(contentsOf: try generateSingleModuleBuildingJob(module,
+          prebuiltModuleDir, input, output,
+          try getOutputPaths(withName: dependencies, loadableFor: input.arch),
+          currentABIDir, baselineABIDir))
       }
+      // For each dependency, add to the list to handle if the list doesn't
+      // contain this dependency.
+      dependencies.forEach({ newModule in
+        if !openModules.contains(newModule) {
+          diagnosticEngine.emit(.note("\(newModule) is discovered."),
+                                location: nil)
+          openModules.append(newModule)
+        }
+      })
+      unhandledModules.remove(module)
+      idx += 1
     }
 
     // We are done if we don't need to handle all inputs exhaustively.
diff --git a/Sources/SwiftDriver/Utilities/DOTModuleDependencyGraphSerializer.swift b/Sources/SwiftDriver/Utilities/DOTModuleDependencyGraphSerializer.swift
@@ -64,10 +64,7 @@ import TSCBasic
     stream.write("digraph Modules {\n")
     for (moduleId, moduleInfo) in graph.modules {
       stream.write(outputNode(for: moduleId))
-      guard let dependencies = moduleInfo.directDependencies else {
-        continue
-      }
-      for dependencyId in dependencies {
+      for dependencyId in moduleInfo.allDependencies {
         stream.write("  \(quoteName(label(for: moduleId))) -> \(quoteName(label(for: dependencyId))) [color=black];\n")
       }
     }
diff --git a/Tests/SwiftDriverTests/CachingBuildTests.swift b/Tests/SwiftDriverTests/CachingBuildTests.swift
@@ -172,7 +172,7 @@ private func checkCachingBuildJobDependencies(job: Job,
     XCTAssertTrue(job.commandLine.contains(.flag(String(cacheKey))))
   }
 
-  for dependencyId in moduleInfo.directDependencies! {
+  for dependencyId in moduleInfo.allDependencies {
     let dependencyInfo = try dependencyGraph.moduleInfo(of: dependencyId)
     switch dependencyInfo.details {
       case .swift(let swiftDependencyDetails):
@@ -186,7 +186,7 @@ private func checkCachingBuildJobDependencies(job: Job,
     }
 
     // Ensure all transitive dependencies got added as well.
-    for transitiveDependencyId in dependencyInfo.directDependencies! {
+    for transitiveDependencyId in dependencyInfo.allDependencies {
       try checkCachingBuildJobDependencies(job: job,
                                            moduleInfo: try dependencyGraph.moduleInfo(of: transitiveDependencyId),
                                            dependencyGraph: dependencyGraph)
diff --git a/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift b/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift
@@ -88,7 +88,7 @@ private func checkExplicitModuleBuildJobDependencies(job: Job,
     XCTAssertJobInvocationMatches(job, .flag("-fmodule-file=\(dependencyId.moduleName)=\(clangDependencyModulePathString)"))
   }
 
-  for dependencyId in moduleInfo.directDependencies! {
+  for dependencyId in moduleInfo.allDependencies {
     let dependencyInfo = try dependencyGraph.moduleInfo(of: dependencyId)
     switch dependencyInfo.details {
       case .swift(_):
@@ -102,7 +102,7 @@ private func checkExplicitModuleBuildJobDependencies(job: Job,
     }
 
     // Ensure all transitive dependencies got added as well.
-    for transitiveDependencyId in dependencyInfo.directDependencies! {
+    for transitiveDependencyId in dependencyInfo.allDependencies {
       try checkExplicitModuleBuildJobDependencies(job: job,
                                                   moduleInfo: try dependencyGraph.moduleInfo(of: transitiveDependencyId),
                                                   dependencyGraph: dependencyGraph)
@@ -2442,13 +2442,10 @@ final class ExplicitModuleBuildTests: XCTestCase {
             main.nativePathString(escaped: true)
         ] + sdkArgumentsForTesting) { driver, diagnostics in
           diagnostics.forbidUnexpected(.error, .warning, .note, .remark)
-
-          let jobs = try driver.planBuild()
-          try driver.run(jobs: jobs)
-
           diagnostics.expect(.remark("Module 'testTraceDependency' depends on 'A'"))
           diagnostics.expect(.note("[testTraceDependency] -> [A] -> [A](ObjC)"))
           diagnostics.expect(.note("[testTraceDependency] -> [C](ObjC) -> [B](ObjC) -> [A](ObjC)"))
+          try driver.run(jobs: driver.planBuild())
         }
       }
 
@@ -2466,13 +2463,10 @@ final class ExplicitModuleBuildTests: XCTestCase {
             main.nativePathString(escaped: true)
         ] + sdkArgumentsForTesting) { driver, diagnostics in
           diagnostics.forbidUnexpected(.error, .warning, .note, .remark)
-
-          let jobs = try driver.planBuild()
-          try driver.run(jobs: jobs)
-
           diagnostics.expect(.remark("Module 'testTraceDependency' depends on 'A'"))
           diagnostics.expect(.note("[testTraceDependency] -> [A] -> [A](ObjC)"),
                              alternativeMessage: .note("[testTraceDependency] -> [C](ObjC) -> [B](ObjC) -> [A](ObjC)"))
+          try driver.run(jobs: driver.planBuild())
         }
       }
     }

Original file line number	Diff line number	Diff line change
`@@ -510,7 +510,7 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT`
`510`	`510`	`clangDependencyArtifacts: &clangDependencyArtifacts,`
`511`	`511`	`swiftDependencyArtifacts: &swiftDependencyArtifacts)`
`512`	`512`	`let depInfo = try dependencyGraph.moduleInfo(of: bridgingHeaderDepID)`
`513`		`- dependenciesWorklist.append(contentsOf: depInfo.directDependencies ?? [])`
	`513`	`+ dependenciesWorklist.append(contentsOf: depInfo.allDependencies)`
`514`	`514`	`}`
`515`	`515`
`516`	`516`	`// Clang module dependencies are specified on the command line explicitly`
Original file line number	Diff line number	Diff line change
`@@ -64,10 +64,7 @@ import TSCBasic`
`64`	`64`	`stream.write("digraph Modules {\n")`
`65`	`65`	`for (moduleId, moduleInfo) in graph.modules {`
`66`	`66`	`stream.write(outputNode(for: moduleId))`
`67`		`- guard let dependencies = moduleInfo.directDependencies else {`
`68`		`- continue`
`69`		`- }`
`70`		`- for dependencyId in dependencies {`
	`67`	`+ for dependencyId in moduleInfo.allDependencies {`
`71`	`68`	`stream.write(" \(quoteName(label(for: moduleId))) -> \(quoteName(label(for: dependencyId))) [color=black];\n")`
`72`	`69`	`}`
`73`	`70`	`}`