Skip to content

[Dependency Scanning] Remove obsolete placeholder module concept #1917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 11, 2025
Merged

[Dependency Scanning] Remove obsolete placeholder module concept #1917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions Sources/CSwiftScan/include/swiftscan_header.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ typedef struct {
typedef enum {
SWIFTSCAN_DEPENDENCY_INFO_SWIFT_TEXTUAL = 0,
SWIFTSCAN_DEPENDENCY_INFO_SWIFT_BINARY = 1,
SWIFTSCAN_DEPENDENCY_INFO_SWIFT_PLACEHOLDER = 2,
SWIFTSCAN_DEPENDENCY_INFO_CLANG = 3
} swiftscan_dependency_info_kind_t;

Expand Down Expand Up @@ -170,14 +169,6 @@ typedef struct {
swiftscan_string_set_t *
(*swiftscan_swift_binary_detail_get_header_dependencies)(swiftscan_module_details_t);

//=== Swift Placeholder Module Details query APIs -------------------------===//
swiftscan_string_ref_t
(*swiftscan_swift_placeholder_detail_get_compiled_module_path)(swiftscan_module_details_t);
swiftscan_string_ref_t
(*swiftscan_swift_placeholder_detail_get_module_doc_path)(swiftscan_module_details_t);
swiftscan_string_ref_t
(*swiftscan_swift_placeholder_detail_get_module_source_info_path)(swiftscan_module_details_t);

//=== Clang Module Details query APIs -------------------------------------===//
swiftscan_string_ref_t
(*swiftscan_clang_detail_get_module_map_path)(swiftscan_module_details_t);
Expand Down
42 changes: 29 additions & 13 deletions Sources/SwiftDriver/Driver/Driver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,10 +638,6 @@ public struct Driver {
/// is shared across many targets; otherwise, a new instance is created by the driver itself.
@_spi(Testing) public let interModuleDependencyOracle: InterModuleDependencyOracle

/// A dictionary of external targets that are a part of the same build, mapping to filesystem paths
/// of their module files
@_spi(Testing) public var externalTargetModuleDetailsMap: ExternalTargetModuleDetailsMap? = nil

/// A collection of all the flags the selected toolchain's `swift-frontend` supports
public let supportedFrontendFlags: Set<String>

Expand Down Expand Up @@ -768,6 +764,7 @@ public struct Driver {
}

@available(*, deprecated, renamed: "init(args:env:diagnosticsOutput:fileSystem:executor:integratedDriver:compilerExecutableDir:externalTargetModuleDetailsMap:interModuleDependencyOracle:)")
@_disfavoredOverload
public init(
args: [String],
env: [String: String] = ProcessEnv.vars,
Expand All @@ -788,12 +785,12 @@ public struct Driver {
integratedDriver: integratedDriver,
compilerIntegratedTooling: false,
compilerExecutableDir: compilerExecutableDir,
externalTargetModuleDetailsMap: externalTargetModuleDetailsMap,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing a deprecated public init doesn't quite make sense? If you change it, you might as well just delete it.

Or we can just silently ignore the field rather than deleting the parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is changing the underlying init that the deprecated public init itself is calling, rather than the signature of the deprecated public init. So in effect we are silently ignoring the field.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my bad. The way GitHub website fold the code makes it looks like a change to the function signature.

interModuleDependencyOracle: interModuleDependencyOracle
)
}

@available(*, deprecated, renamed: "init(args:env:diagnosticsOutput:fileSystem:executor:integratedDriver:compilerIntegratedTooling:compilerExecutableDir:externalTargetModuleDetailsMap:interModuleDependencyOracle:)")
@_disfavoredOverload
public init(
args: [String],
env: [String: String] = ProcessEnv.vars,
Expand All @@ -814,7 +811,33 @@ public struct Driver {
integratedDriver: integratedDriver,
compilerIntegratedTooling: false,
compilerExecutableDir: compilerExecutableDir,
externalTargetModuleDetailsMap: externalTargetModuleDetailsMap,
interModuleDependencyOracle: interModuleDependencyOracle
)
}

@available(*, deprecated, renamed: "init(args:env:diagnosticsOutput:fileSystem:executor:integratedDriver:compilerExecutableDir:interModuleDependencyOracle:)")
@_disfavoredOverload
public init(
args: [String],
env: [String: String] = ProcessEnv.vars,
diagnosticsOutput: DiagnosticsOutput = .engine(DiagnosticsEngine(handlers: [Driver.stderrDiagnosticsHandler])),
fileSystem: FileSystem = localFileSystem,
executor: DriverExecutor,
integratedDriver: Bool = true,
compilerIntegratedTooling: Bool = false,
compilerExecutableDir: AbsolutePath? = nil,
externalTargetModuleDetailsMap: ExternalTargetModuleDetailsMap? = nil,
interModuleDependencyOracle: InterModuleDependencyOracle? = nil
) throws {
try self.init(
args: args,
env: env,
diagnosticsOutput: diagnosticsOutput,
fileSystem: fileSystem,
executor: executor,
integratedDriver: integratedDriver,
compilerIntegratedTooling: false,
compilerExecutableDir: compilerExecutableDir,
interModuleDependencyOracle: interModuleDependencyOracle
)
}
Expand All @@ -837,9 +860,6 @@ public struct Driver {
/// Swift compiler image which contains symbols normally queried from a libSwiftScan instance.
/// - Parameter compilerExecutableDir: Directory that contains the compiler executable to be used.
/// Used when in `integratedDriver` mode as a substitute for the driver knowing its executable path.
/// - Parameter externalTargetModuleDetailsMap: A dictionary of external targets that are a part of
/// the same build, mapping to a details value which includes a filesystem path of their
/// `.swiftmodule` and a flag indicating whether the external target is a framework.
/// - Parameter interModuleDependencyOracle: An oracle for querying inter-module dependencies,
/// shared across different module builds by a build system.
public init(
Expand All @@ -851,7 +871,6 @@ public struct Driver {
integratedDriver: Bool = true,
compilerIntegratedTooling: Bool = false,
compilerExecutableDir: AbsolutePath? = nil,
externalTargetModuleDetailsMap: ExternalTargetModuleDetailsMap? = nil,
interModuleDependencyOracle: InterModuleDependencyOracle? = nil
) throws {
self.env = env
Expand All @@ -869,7 +888,6 @@ public struct Driver {
self.diagnosticEngine = diagnosticsEngine

self.executor = executor
self.externalTargetModuleDetailsMap = externalTargetModuleDetailsMap

if case .subcommand = try Self.invocationRunMode(forArgs: args).mode {
throw Error.subcommandPassedToDriver
Expand Down Expand Up @@ -1775,8 +1793,6 @@ extension Driver {
pathString = pathString + "[" + moduleName + "]"
case .clang(let moduleName):
pathString = pathString + "[" + moduleName + "](ObjC)"
case .swiftPlaceholder(_):
fatalError("Unexpected unresolved Placeholder module")
}
if index < path.count - 1 {
pathString = pathString + " -> "
Expand Down
3 changes: 1 addition & 2 deletions Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public struct ChainedBridgingHeaderFile {
let content: String
}

// Deprecated
public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalTargetModuleDetails]

/// In Explicit Module Build mode, this planner is responsible for generating and providing
Expand Down Expand Up @@ -363,8 +364,6 @@ public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalT
headerDependencies: prebuiltModuleDetails.headerDependencyPaths,
isFramework: isFramework,
moduleCacheKey: prebuiltModuleDetails.moduleCacheKey))
case .swiftPlaceholder:
fatalError("Unresolved placeholder dependencies at planning stage: \(dependencyId) of \(moduleId)")
}
}

Expand Down
53 changes: 0 additions & 53 deletions ...SwiftDriver/ExplicitModuleBuilds/InterModuleDependencies/CommonDependencyOperations.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,49 +13,6 @@
import func TSCBasic.topologicalSort
import protocol TSCBasic.FileSystem

@_spi(Testing) public extension InterModuleDependencyGraph {
/// For targets that are built alongside the driver's current module, the scanning action will report them as
/// textual targets to be built from source. Because we can rely on these targets to have been built prior
/// to the driver's current target, we resolve such external targets as prebuilt binary modules, in the graph.
mutating func resolveExternalDependencies(for externalTargetModuleDetailsMap: ExternalTargetModuleDetailsMap)
throws {
for (externalModuleId, externalModuleDetails) in externalTargetModuleDetailsMap {
let externalModulePath = externalModuleDetails.path
// Replace the occurrence of a Swift module to-be-built from source-file
// to an info that describes a pre-built binary module.
let swiftModuleId: ModuleDependencyId = .swift(externalModuleId.moduleName)
let prebuiltModuleId: ModuleDependencyId = .swiftPrebuiltExternal(externalModuleId.moduleName)
if let currentInfo = modules[swiftModuleId],
externalModuleId.moduleName != mainModuleName {
let newExternalModuleDetails =
SwiftPrebuiltExternalModuleDetails(compiledModulePath:
TextualVirtualPath(path: VirtualPath.absolute(externalModulePath).intern()),
isFramework: externalModuleDetails.isFramework)
let newInfo = ModuleInfo(modulePath: TextualVirtualPath(path: VirtualPath.absolute(externalModulePath).intern()),
sourceFiles: [],
directDependencies: currentInfo.directDependencies,
linkLibraries: currentInfo.linkLibraries,
details: .swiftPrebuiltExternal(newExternalModuleDetails))
Self.replaceModule(originalId: swiftModuleId, replacementId: prebuiltModuleId,
replacementInfo: newInfo, in: &modules)
} else if let currentPrebuiltInfo = modules[prebuiltModuleId] {
// Just update the isFramework bit on this prebuilt module dependency
let newExternalModuleDetails =
SwiftPrebuiltExternalModuleDetails(compiledModulePath:
TextualVirtualPath(path: VirtualPath.absolute(externalModulePath).intern()),
isFramework: externalModuleDetails.isFramework)
let newInfo = ModuleInfo(modulePath: TextualVirtualPath(path: VirtualPath.absolute(externalModulePath).intern()),
sourceFiles: [],
directDependencies: currentPrebuiltInfo.directDependencies,
linkLibraries: currentPrebuiltInfo.linkLibraries,
details: .swiftPrebuiltExternal(newExternalModuleDetails))
Self.replaceModule(originalId: prebuiltModuleId, replacementId: prebuiltModuleId,
replacementInfo: newInfo, in: &modules)
}
}
}
}

extension InterModuleDependencyGraph {
var topologicalSorting: [ModuleDependencyId] {
get throws {
Expand Down Expand Up @@ -114,10 +71,6 @@ extension InterModuleDependencyGraph {
in moduleInfoMap: inout ModuleInfoMap) {
for moduleId in moduleInfoMap.keys {
var moduleInfo = moduleInfoMap[moduleId]!
// Skip over placeholders, they do not have dependencies
if case .swiftPlaceholder(_) = moduleId {
continue
}
if let originalModuleIndex = moduleInfo.directDependencies?.firstIndex(of: originalId) {
moduleInfo.directDependencies![originalModuleIndex] = replacementId;
moduleInfoMap[moduleId] = moduleInfo
Expand Down Expand Up @@ -246,9 +199,6 @@ internal extension InterModuleDependencyGraph {
return try casOutputMissing(clangDetails.moduleCacheKey)
case .swiftPrebuiltExternal(_):
return false;
case .swiftPlaceholder(_):
// TODO: This should never ever happen. Hard error?
return true;
}
}

Expand Down Expand Up @@ -340,9 +290,6 @@ internal extension InterModuleDependencyGraph {
}
case .swiftPrebuiltExternal(_):
return true;
case .swiftPlaceholder(_):
// TODO: This should never ever happen. Hard error?
return false;
}

return true
Expand Down
51 changes: 9 additions & 42 deletions ...SwiftDriver/ExplicitModuleBuilds/InterModuleDependencies/InterModuleDependencyGraph.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ public typealias ModuleInfoMap = [ModuleDependencyId: ModuleInfo]

public enum ModuleDependencyId: Hashable {
case swift(String)
case swiftPlaceholder(String)
case swiftPrebuiltExternal(String)
case clang(String)

public var moduleName: String {
switch self {
case .swift(let name): return name
case .swiftPlaceholder(let name): return name
case .swiftPrebuiltExternal(let name): return name
case .clang(let name): return name
}
Expand All @@ -34,7 +32,6 @@ public enum ModuleDependencyId: Hashable {
internal var moduleNameForDiagnostic: String {
switch self {
case .swift(let name): return name
case .swiftPlaceholder(let name): return name + "(placeholder)"
case .swiftPrebuiltExternal(let name): return name + "(swiftmodule)"
case .clang(let name): return name + "(pcm)"
}
Expand All @@ -44,7 +41,6 @@ public enum ModuleDependencyId: Hashable {
extension ModuleDependencyId: Codable {
enum CodingKeys: CodingKey {
case swift
case swiftPlaceholder
case swiftPrebuiltExternal
case clang
}
Expand All @@ -56,16 +52,11 @@ extension ModuleDependencyId: Codable {
self = .swift(moduleName)
} catch {
do {
let moduleName = try container.decode(String.self, forKey: .swiftPlaceholder)
self = .swiftPlaceholder(moduleName)
let moduleName = try container.decode(String.self, forKey: .swiftPrebuiltExternal)
self = .swiftPrebuiltExternal(moduleName)
} catch {
do {
let moduleName = try container.decode(String.self, forKey: .swiftPrebuiltExternal)
self = .swiftPrebuiltExternal(moduleName)
} catch {
let moduleName = try container.decode(String.self, forKey: .clang)
self = .clang(moduleName)
}
let moduleName = try container.decode(String.self, forKey: .clang)
self = .clang(moduleName)
}
}
}
Expand All @@ -75,8 +66,6 @@ extension ModuleDependencyId: Codable {
switch self {
case .swift(let moduleName):
try container.encode(moduleName, forKey: .swift)
case .swiftPlaceholder(let moduleName):
try container.encode(moduleName, forKey: .swiftPlaceholder)
case .swiftPrebuiltExternal(let moduleName):
try container.encode(moduleName, forKey: .swiftPrebuiltExternal)
case .clang(let moduleName):
Expand Down Expand Up @@ -147,15 +136,6 @@ public struct SwiftModuleDetails: Codable, Hashable {
public var chainedBridgingHeaderContent: String?
}

/// Details specific to Swift placeholder dependencies.
public struct SwiftPlaceholderModuleDetails: Codable, Hashable {
/// The path to the .swiftModuleDoc file.
var moduleDocPath: TextualVirtualPath?

/// The path to the .swiftSourceInfo file.
var moduleSourceInfoPath: TextualVirtualPath?
}

/// Details specific to Swift externally-pre-built modules.
public struct SwiftPrebuiltExternalModuleDetails: Codable, Hashable {
/// The path to the already-compiled module that must be used instead of
Expand Down Expand Up @@ -221,10 +201,6 @@ public struct ModuleInfo: Codable, Hashable {
/// a bridging header.
case swift(SwiftModuleDetails)

/// Swift placeholder modules carry additional details that specify their
/// module doc path and source info paths.
case swiftPlaceholder(SwiftPlaceholderModuleDetails)

/// Swift externally-prebuilt modules must communicate the path to pre-built binary artifacts
case swiftPrebuiltExternal(SwiftPrebuiltExternalModuleDetails)

Expand All @@ -248,7 +224,6 @@ public struct ModuleInfo: Codable, Hashable {
extension ModuleInfo.Details: Codable {
enum CodingKeys: CodingKey {
case swift
case swiftPlaceholder
case swiftPrebuiltExternal
case clang
}
Expand All @@ -260,18 +235,12 @@ extension ModuleInfo.Details: Codable {
self = .swift(details)
} catch {
do {
let details = try container.decode(SwiftPlaceholderModuleDetails.self,
forKey: .swiftPlaceholder)
self = .swiftPlaceholder(details)
let details = try container.decode(SwiftPrebuiltExternalModuleDetails.self,
forKey: .swiftPrebuiltExternal)
self = .swiftPrebuiltExternal(details)
} catch {
do {
let details = try container.decode(SwiftPrebuiltExternalModuleDetails.self,
forKey: .swiftPrebuiltExternal)
self = .swiftPrebuiltExternal(details)
} catch {
let details = try container.decode(ClangModuleDetails.self, forKey: .clang)
self = .clang(details)
}
let details = try container.decode(ClangModuleDetails.self, forKey: .clang)
self = .clang(details)
}
}
}
Expand All @@ -281,8 +250,6 @@ extension ModuleInfo.Details: Codable {
switch self {
case .swift(let details):
try container.encode(details, forKey: .swift)
case .swiftPlaceholder(let details):
try container.encode(details, forKey: .swiftPlaceholder)
case .swiftPrebuiltExternal(let details):
try container.encode(details, forKey: .swiftPrebuiltExternal)
case .clang(let details):
Expand Down
14 changes: 1 addition & 13 deletions ...wiftDriver/ExplicitModuleBuilds/InterModuleDependencies/InterModuleDependencyOracle.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,7 @@ import Dispatch
/// An abstraction of a cache and query-engine of inter-module dependencies
public class InterModuleDependencyOracle {
/// Allow external clients to instantiate the oracle
/// - Parameter scannerRequiresPlaceholderModules: Configures this driver's/oracle's scanner invocations to
/// specify external module dependencies to be treated as placeholders. This is required in contexts
/// where the dependency scanning action is invoked for a module which depends on another module
/// that is part of the same build but has not yet been built. Treating it as a placeholder
/// will allow the scanning action to not fail when it fails to detect this dependency on
/// the filesystem. For example, SwiftPM plans all targets belonging to a package before *any* of them
/// are built. So this setting is meant to be used there. In contexts where planning a module
/// necessarily means all of its dependencies have already been built this is not necessary.
public init(scannerRequiresPlaceholderModules: Bool = false) {
self.scannerRequiresPlaceholderModules = scannerRequiresPlaceholderModules
}
public init() {}

@_spi(Testing) public func getDependencies(workingDirectory: AbsolutePath,
moduleAliases: [String: String]? = nil,
Expand Down Expand Up @@ -193,8 +183,6 @@ public class InterModuleDependencyOracle {
/// A reference to an instance of the compiler's libSwiftScan shared library
private var swiftScanLibInstance: SwiftScan? = nil

internal let scannerRequiresPlaceholderModules: Bool

internal struct CASConfig: Hashable, Equatable {
static func == (lhs: InterModuleDependencyOracle.CASConfig, rhs: InterModuleDependencyOracle.CASConfig) -> Bool {
return lhs.onDiskPath == rhs.onDiskPath &&
Expand Down
Loading