From c11413058f7696e73f99fccf4d5621eeea1ec249 Mon Sep 17 00:00:00 2001 From: Artem Chikin Date: Thu, 10 Apr 2025 11:06:48 -0700 Subject: [PATCH] [Explicit Module Builds] Centralize computing *all* dependencies of a given 'ModuleInfo' and use throughout This is in preparation of dependency scanner turning the 'directDependencies' field to contain only actually-directly-imported modules. --- .../ExplicitDependencyBuildPlanner.swift | 2 +- .../CommonDependencyOperations.swift | 53 ++++--------------- .../InterModuleDependencyGraph.swift | 34 +++++++++++- .../SwiftDriver/Jobs/PrebuiltModulesJob.swift | 51 +++++++++--------- .../DOTModuleDependencyGraphSerializer.swift | 5 +- .../SwiftDriverTests/CachingBuildTests.swift | 4 +- .../ExplicitModuleBuildTests.swift | 14 ++--- 7 files changed, 75 insertions(+), 88 deletions(-) diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift index 5f098143e..35f25da84 100644 --- a/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift +++ b/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift @@ -495,7 +495,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 index ec5c5a4eb..9ea3b5e1b 100644 --- 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 = [] // 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 index 5a64acfd6..85b9b04c2 100644 --- 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 = [] + 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 index bff6dc64f..e8a557670 100644 --- 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(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 index 602bbe944..ba89b1689 100644 --- 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 index 97c7435e1..bfad7cdca 100644 --- 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 index 88b74c2a5..4d420fc59 100644 --- 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()) } } }