From c827ecf7405526e7b83b62679366ac0b1d7f5369 Mon Sep 17 00:00:00 2001 From: Mirza Garibovic Date: Mon, 8 Sep 2025 16:41:45 -0700 Subject: [PATCH 1/3] Metrics: Generalize counters, ensure task counters are propagated to clients observing messages --- .../BuildOperationMessages.swift | 21 +++--------------- Sources/SWBBuildSystem/BuildOperation.swift | 2 +- .../SWBProtocol/BuildOperationMessages.swift | 6 ++++- Sources/SWBTaskExecution/Task.swift | 9 +------- .../TaskActions/ClangCompileTaskAction.swift | 6 ++--- .../EmbedSwiftStdLibTaskAction.swift | 10 +-------- .../GenericCachingTaskAction.swift | 16 ++------------ .../SwiftDriverJobTaskAction.swift | 4 ++-- .../SWBTestSupport/BuildOperationTester.swift | 16 ++------------ ...WBServiceConsoleBuildCommandProtocol.swift | 9 ++++---- .../SwiftBuild/SWBuildMessage+Protocol.swift | 5 +---- .../InProcessTaskTestSupport.swift | 22 ++----------------- 12 files changed, 28 insertions(+), 98 deletions(-) diff --git a/Sources/SWBBuildService/BuildOperationMessages.swift b/Sources/SWBBuildService/BuildOperationMessages.swift index c17616f5..2009b22b 100644 --- a/Sources/SWBBuildService/BuildOperationMessages.swift +++ b/Sources/SWBBuildService/BuildOperationMessages.swift @@ -705,20 +705,8 @@ private final class TaskOutputHandler: TaskOutputDelegate { } } - func incrementClangCacheHit() { - self.counters[.clangCacheHits, default: 0] += 1 - } - - func incrementClangCacheMiss() { - self.counters[.clangCacheMisses, default: 0] += 1 - } - - func incrementSwiftCacheHit() { - self.counters[.swiftCacheHits, default: 0] += 1 - } - - func incrementSwiftCacheMiss() { - self.counters[.swiftCacheMisses, default: 0] += 1 + func incrementCounter(_ counter: BuildOperationMetrics.Counter) { + self.counters[counter, default: 0] += 1 } func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) { @@ -944,10 +932,7 @@ private final class DiscardingTaskOutputHandler: TaskOutputDelegate { func subtaskUpToDate(_ subtask: any ExecutableTask) {} func previouslyBatchedSubtaskUpToDate(signature: ByteString, target: ConfiguredTarget) {} func updateResult(_ result: TaskResult) {} - func incrementClangCacheHit() {} - func incrementClangCacheMiss() {} - func incrementSwiftCacheHit() {} - func incrementSwiftCacheMiss() {} + func incrementCounter(_ counter: BuildOperationMetrics.Counter) {} func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) {} } diff --git a/Sources/SWBBuildSystem/BuildOperation.swift b/Sources/SWBBuildSystem/BuildOperation.swift index 8bf3c230..0c4aa1a9 100644 --- a/Sources/SWBBuildSystem/BuildOperation.swift +++ b/Sources/SWBBuildSystem/BuildOperation.swift @@ -806,7 +806,7 @@ package final class BuildOperation: BuildSystemOperation { } // `buildComplete()` should not run within `queue`, otherwise there can be a deadlock during cancelling. - return delegate.buildComplete(self, status: effectiveStatus, delegate: buildOutputDelegate, metrics: .init(counters: aggregatedCounters)) + return delegate.buildComplete(self, status: effectiveStatus, delegate: buildOutputDelegate, metrics: .init(counters: aggregatedCounters, taskCounters: aggregatedTaskCounters)) } func prepareForBuilding() async -> ([String], [String])? { diff --git a/Sources/SWBProtocol/BuildOperationMessages.swift b/Sources/SWBProtocol/BuildOperationMessages.swift index 71d51692..92a84268 100644 --- a/Sources/SWBProtocol/BuildOperationMessages.swift +++ b/Sources/SWBProtocol/BuildOperationMessages.swift @@ -500,8 +500,12 @@ public struct BuildOperationMetrics: Equatable, Codable, Sendable { public let counters: [Counter: Int] - public init(counters: [Counter : Int]) { + /// The key is the first component of task rule info, a.k.a. the rule info type + public let taskCounters: [String: [TaskCounter: Int]] + + public init(counters: [Counter : Int], taskCounters: [String: [TaskCounter: Int]]) { self.counters = counters + self.taskCounters = taskCounters } } diff --git a/Sources/SWBTaskExecution/Task.swift b/Sources/SWBTaskExecution/Task.swift index 745dc5ea..a923dbaf 100644 --- a/Sources/SWBTaskExecution/Task.swift +++ b/Sources/SWBTaskExecution/Task.swift @@ -608,14 +608,7 @@ public protocol TaskOutputDelegate: DiagnosticProducingDelegate /// Report a task which was previously batched as up-to-date. func previouslyBatchedSubtaskUpToDate(signature: ByteString, target: ConfiguredTarget) - func incrementClangCacheHit() - - func incrementClangCacheMiss() - - func incrementSwiftCacheHit() - - func incrementSwiftCacheMiss() - + func incrementCounter(_ counter: BuildOperationMetrics.Counter) func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) var counters: [BuildOperationMetrics.Counter: Int] { get } diff --git a/Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift b/Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift index 6f58f5e9..67c3be65 100644 --- a/Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift +++ b/Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift @@ -414,7 +414,7 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA if enableDiagnosticRemarks { outputDelegate.note("cache miss: \(cacheKey)") } - outputDelegate.incrementClangCacheMiss() + outputDelegate.incrementCounter(.clangCacheMisses) outputDelegate.incrementTaskCounter(.cacheMisses) outputDelegate.emitOutput("Cache miss\n") return false @@ -427,7 +427,7 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA outputDelegate.note("missing CAS output \(output.name): \(output.casID)") outputDelegate.note("cache miss: \(cacheKey)") } - outputDelegate.incrementClangCacheMiss() + outputDelegate.incrementCounter(.clangCacheMisses) outputDelegate.incrementTaskCounter(.cacheMisses) outputDelegate.emitOutput("Cache miss\n") return false @@ -440,7 +440,7 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA outputDelegate.note("using CAS output \(output.name): \(output.casID)") } } - outputDelegate.incrementClangCacheHit() + outputDelegate.incrementCounter(.clangCacheHits) outputDelegate.incrementTaskCounter(.cacheHits) outputDelegate.emitOutput("Cache hit\n") outputDelegate.emitOutput(ByteString(encodingAsUTF8: diagnosticText)) diff --git a/Sources/SWBTaskExecution/TaskActions/EmbedSwiftStdLibTaskAction.swift b/Sources/SWBTaskExecution/TaskActions/EmbedSwiftStdLibTaskAction.swift index 40509fc2..267a85f5 100644 --- a/Sources/SWBTaskExecution/TaskActions/EmbedSwiftStdLibTaskAction.swift +++ b/Sources/SWBTaskExecution/TaskActions/EmbedSwiftStdLibTaskAction.swift @@ -472,15 +472,7 @@ public final class EmbedSwiftStdLibTaskAction: TaskAction { logV(args.joined(separator: " ")) final class CapturingOutputDelegate: TaskOutputDelegate { - func incrementClangCacheHit() { - // TBD - } - - func incrementClangCacheMiss() { - // TBD - } - func incrementSwiftCacheHit() {} - func incrementSwiftCacheMiss() {} + func incrementCounter(_ counter: BuildOperationMetrics.Counter) {} func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) {} var counters: [BuildOperationMetrics.Counter : Int] = [:] diff --git a/Sources/SWBTaskExecution/TaskActions/GenericCachingTaskAction.swift b/Sources/SWBTaskExecution/TaskActions/GenericCachingTaskAction.swift index 466859d9..093c3f33 100644 --- a/Sources/SWBTaskExecution/TaskActions/GenericCachingTaskAction.swift +++ b/Sources/SWBTaskExecution/TaskActions/GenericCachingTaskAction.swift @@ -404,20 +404,8 @@ fileprivate final class CapturingTaskOutputDelegate: TaskOutputDelegate { underlyingTaskOutputDelegate.previouslyBatchedSubtaskUpToDate(signature: signature, target: target) } - func incrementClangCacheHit() { - underlyingTaskOutputDelegate.incrementClangCacheHit() - } - - func incrementClangCacheMiss() { - underlyingTaskOutputDelegate.incrementClangCacheMiss() - } - - func incrementSwiftCacheHit() { - underlyingTaskOutputDelegate.incrementSwiftCacheHit() - } - - func incrementSwiftCacheMiss() { - underlyingTaskOutputDelegate.incrementSwiftCacheMiss() + func incrementCounter(_ counter: BuildOperationMetrics.Counter) { + underlyingTaskOutputDelegate.incrementCounter(counter) } func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) { diff --git a/Sources/SWBTaskExecution/TaskActions/SwiftDriverJobTaskAction.swift b/Sources/SWBTaskExecution/TaskActions/SwiftDriverJobTaskAction.swift index 2ed927e5..3af00355 100644 --- a/Sources/SWBTaskExecution/TaskActions/SwiftDriverJobTaskAction.swift +++ b/Sources/SWBTaskExecution/TaskActions/SwiftDriverJobTaskAction.swift @@ -670,11 +670,11 @@ public final class SwiftDriverJobTaskAction: TaskAction, BuildValueValidatingTas outputDelegate.note("replay cache \(result ? "hit" : "miss")") } if result { - outputDelegate.incrementSwiftCacheHit() + outputDelegate.incrementCounter(.swiftCacheHits) outputDelegate.incrementTaskCounter(.cacheHits) outputDelegate.emitOutput("Cache hit\n") } else { - outputDelegate.incrementSwiftCacheMiss() + outputDelegate.incrementCounter(.swiftCacheMisses) outputDelegate.incrementTaskCounter(.cacheMisses) outputDelegate.emitOutput("Cache miss\n") } diff --git a/Sources/SWBTestSupport/BuildOperationTester.swift b/Sources/SWBTestSupport/BuildOperationTester.swift index 17c93778..82e6c9c4 100644 --- a/Sources/SWBTestSupport/BuildOperationTester.swift +++ b/Sources/SWBTestSupport/BuildOperationTester.swift @@ -1783,20 +1783,8 @@ private final class BuildOperationTesterDelegate: BuildOperationDelegate { } private class TesterTaskOutputDelegate: TaskOutputDelegate { - func incrementClangCacheHit() { - self.counters[.clangCacheHits, default: 0] += 1 - } - - func incrementClangCacheMiss() { - self.counters[.clangCacheMisses, default: 0] += 1 - } - - func incrementSwiftCacheHit() { - self.counters[.swiftCacheHits, default: 0] += 1 - } - - func incrementSwiftCacheMiss() { - self.counters[.swiftCacheMisses, default: 0] += 1 + func incrementCounter(_ counter: BuildOperationMetrics.Counter) { + self.counters[counter, default: 0] += 1 } func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) { diff --git a/Sources/SwiftBuild/ConsoleCommands/SWBServiceConsoleBuildCommandProtocol.swift b/Sources/SwiftBuild/ConsoleCommands/SWBServiceConsoleBuildCommandProtocol.swift index 46f97977..05492729 100644 --- a/Sources/SwiftBuild/ConsoleCommands/SWBServiceConsoleBuildCommandProtocol.swift +++ b/Sources/SwiftBuild/ConsoleCommands/SWBServiceConsoleBuildCommandProtocol.swift @@ -14,6 +14,7 @@ public import Foundation public import SWBProjectModel +public import SWBProtocol import SWBUtil @@ -168,10 +169,10 @@ public enum SwiftBuildMessage { } public struct BuildOperationMetrics { - let clangCacheHits: Int - let clangCacheMisses: Int - let swiftCacheHits: Int - let swiftCacheMisses: Int + let counters: [SWBProtocol.BuildOperationMetrics.Counter: Int] + + /// The key is the first component of task rule info, a.k.a. the rule info type + let taskCounters: [String: [SWBProtocol.BuildOperationMetrics.TaskCounter: Int]] } public struct BuildCompletedInfo { diff --git a/Sources/SwiftBuild/SWBuildMessage+Protocol.swift b/Sources/SwiftBuild/SWBuildMessage+Protocol.swift index f5b58dbd..162e7c26 100644 --- a/Sources/SwiftBuild/SWBuildMessage+Protocol.swift +++ b/Sources/SwiftBuild/SWBuildMessage+Protocol.swift @@ -48,10 +48,7 @@ extension SwiftBuildMessage { init(_ message: BuildOperationEnded) { let metrics: BuildOperationMetrics? if let messageMetrics = message.metrics, !messageMetrics.counters.isEmpty { - metrics = .init(clangCacheHits: messageMetrics.counters[.clangCacheHits] ?? 0, - clangCacheMisses: messageMetrics.counters[.clangCacheMisses] ?? 0, - swiftCacheHits: messageMetrics.counters[.swiftCacheHits] ?? 0, - swiftCacheMisses: messageMetrics.counters[.swiftCacheMisses] ?? 0) + metrics = .init(counters: messageMetrics.counters, taskCounters: messageMetrics.taskCounters) } else { metrics = nil } diff --git a/Tests/SWBTaskExecutionTests/InProcessTaskTestSupport.swift b/Tests/SWBTaskExecutionTests/InProcessTaskTestSupport.swift index 82e321c8..4a2ee5ef 100644 --- a/Tests/SWBTaskExecutionTests/InProcessTaskTestSupport.swift +++ b/Tests/SWBTaskExecutionTests/InProcessTaskTestSupport.swift @@ -71,27 +71,9 @@ final class MockTaskOutputDelegate: TaskOutputDelegate { private let _diagnosticsEngine = DiagnosticsEngine() - func incrementClangCacheHit() { + func incrementCounter(_ counter: BuildOperationMetrics.Counter) { state.state.withLock { state in - state.counters[.clangCacheHits, default: 0] += 1 - } - } - - func incrementClangCacheMiss() { - state.state.withLock { state in - state.counters[.clangCacheMisses, default: 0] += 1 - } - } - - func incrementSwiftCacheHit() { - state.state.withLock { state in - state.counters[.swiftCacheHits, default: 0] += 1 - } - } - - func incrementSwiftCacheMiss() { - state.state.withLock { state in - state.counters[.swiftCacheMisses, default: 0] += 1 + state.counters[counter, default: 0] += 1 } } From 5d161ffac51d96fbecc8bd8c0f718e0d6dc2c9f2 Mon Sep 17 00:00:00 2001 From: Mirza Garibovic Date: Tue, 9 Sep 2025 11:56:43 -0700 Subject: [PATCH 2/3] Dependencies: define metrics --- .../SWBProtocol/BuildOperationMessages.swift | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/Sources/SWBProtocol/BuildOperationMessages.swift b/Sources/SWBProtocol/BuildOperationMessages.swift index 92a84268..e0aac2fc 100644 --- a/Sources/SWBProtocol/BuildOperationMessages.swift +++ b/Sources/SWBProtocol/BuildOperationMessages.swift @@ -486,16 +486,66 @@ public struct TargetDependencyRelationship: Serializable, Codable, Equatable, Se } public struct BuildOperationMetrics: Equatable, Codable, Sendable { + /// These metrics are aggregated across an entire build operation public enum Counter: String, Equatable, Codable, Sendable { case clangCacheHits case clangCacheMisses case swiftCacheHits case swiftCacheMisses + + /// How many dependencies did users declare? + case moduleDependenciesDeclared + + /// How many dependencies were found to be missing? + case moduleDependenciesMissing + + /// How many dependencies were found to be unused? + case moduleDependenciesUnused + + /// How many dependencies warnings were emitted? + case moduleDependenciesWarningsEmitted + + /// How many dependencies errors were emitted? + case moduleDependenciesErrorsEmitted + + /// How many dependencies did users declare? + case headerDependenciesDeclared + + /// How many dependencies were found to be missing? + case headerDependenciesMissing + + /// How many dependencies were found to be unused? + case headerDependenciesUnused + + /// How many dependencies warnings were emitted? + case headerDependenciesWarningsEmitted + + /// How many dependencies errors were emitted? + case headerDependenciesErrorsEmitted } + /// These metrics are aggregated by task rule info type public enum TaskCounter: String, Equatable, Codable, Sendable { case cacheHits case cacheMisses + + /// Number of tasks which could and did validate module dependencies + case moduleDependenciesValidatedTasks + + /// Number of tasks which could but did not validate module dependencies. Together with `moduleDependenciesValidated`, this can be used to track progress towards complete validation. + case moduleDependenciesNotValidatedTasks + + /// Number of unique dependencies scanned / discovered / found by an executing task + case moduleDependenciesScanned + + /// Number of tasks which could and did validate header dependencies + case headerDependenciesValidatedTasks + + /// Number of tasks which could but did not validate header dependencies. Together with `headerDependenciesValidated`, this can be used to track progress towards complete validation. + case headerDependenciesNotValidatedTasks + + /// Number of unique dependencies scanned / discovered / found by an executing task + case headerDependenciesScanned } public let counters: [Counter: Int] From 7bf51fb4975e3e3ccbe8f0c20b85600d59bc4872 Mon Sep 17 00:00:00 2001 From: Mirza Garibovic Date: Tue, 9 Sep 2025 13:51:51 -0700 Subject: [PATCH 3/3] Dependencies: fire metrics (rdar://150307704) + add unused header validation (rdar://159126160) --- .../BuildOperationMessages.swift | 12 +- Sources/SWBCore/Dependencies.swift | 148 +++++++++++------- Sources/SWBCore/Settings/BuiltinMacros.swift | 2 + Sources/SWBTaskExecution/Task.swift | 14 +- .../TaskActions/ClangCompileTaskAction.swift | 21 ++- .../EmbedSwiftStdLibTaskAction.swift | 4 +- .../GenericCachingTaskAction.swift | 8 +- .../TaskActions/SwiftDriverTaskAction.swift | 7 + .../ValidateDependenciesTaskAction.swift | 29 +++- .../SWBTestSupport/BuildOperationTester.swift | 8 +- .../InProcessTaskTestSupport.swift | 8 +- 11 files changed, 177 insertions(+), 84 deletions(-) diff --git a/Sources/SWBBuildService/BuildOperationMessages.swift b/Sources/SWBBuildService/BuildOperationMessages.swift index 2009b22b..729bf33d 100644 --- a/Sources/SWBBuildService/BuildOperationMessages.swift +++ b/Sources/SWBBuildService/BuildOperationMessages.swift @@ -705,12 +705,12 @@ private final class TaskOutputHandler: TaskOutputDelegate { } } - func incrementCounter(_ counter: BuildOperationMetrics.Counter) { - self.counters[counter, default: 0] += 1 + func incrementCounter(_ counter: BuildOperationMetrics.Counter, by amount: Int) { + self.counters[counter, default: 0] += amount } - func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) { - self.taskCounters[counter, default: 0] += 1 + func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter, by amount: Int) { + self.taskCounters[counter, default: 0] += amount } var diagnosticsEngine: DiagnosticProducingDelegateProtocolPrivate { @@ -932,8 +932,8 @@ private final class DiscardingTaskOutputHandler: TaskOutputDelegate { func subtaskUpToDate(_ subtask: any ExecutableTask) {} func previouslyBatchedSubtaskUpToDate(signature: ByteString, target: ConfiguredTarget) {} func updateResult(_ result: TaskResult) {} - func incrementCounter(_ counter: BuildOperationMetrics.Counter) {} - func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) {} + func incrementCounter(_ counter: BuildOperationMetrics.Counter, by amount: Int) {} + func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter, by amount: Int) {} } /// The build output delegate, which sends data back immediately. diff --git a/Sources/SWBCore/Dependencies.swift b/Sources/SWBCore/Dependencies.swift index 94455993..07749710 100644 --- a/Sources/SWBCore/Dependencies.swift +++ b/Sources/SWBCore/Dependencies.swift @@ -74,13 +74,13 @@ public struct ModuleDependency: Hashable, Sendable, SerializableCodable { public struct ModuleDependenciesContext: Sendable, SerializableCodable { public var validate: BooleanWarningLevel public var validateUnused: BooleanWarningLevel - var moduleDependencies: [ModuleDependency] + public var declared: [ModuleDependency] var fixItContext: FixItContext? - init(validate: BooleanWarningLevel, validateUnused: BooleanWarningLevel, moduleDependencies: [ModuleDependency], fixItContext: FixItContext? = nil) { + init(validate: BooleanWarningLevel, validateUnused: BooleanWarningLevel, declared: [ModuleDependency], fixItContext: FixItContext? = nil) { self.validate = validate self.validateUnused = validateUnused - self.moduleDependencies = moduleDependencies + self.declared = declared self.fixItContext = fixItContext } @@ -90,7 +90,7 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable { guard validate != .no || validateUnused != .no else { return nil } let downgrade = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_DEPENDENCIES_DOWNGRADE_ERRORS) let fixItContext = validate != .no ? ModuleDependenciesContext.FixItContext(settings: settings) : nil - self.init(validate: downgrade ? .yes : validate, validateUnused: validateUnused, moduleDependencies: settings.moduleDependencies, fixItContext: fixItContext) + self.init(validate: downgrade ? .yes : validate, validateUnused: validateUnused, declared: settings.moduleDependencies, fixItContext: fixItContext) } public func makeUnsupportedToolchainDiagnostic() -> Diagnostic { @@ -111,14 +111,14 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable { if fromSwift && $0.importLocations.isEmpty { return false } // TODO: if the difference is just the access modifier, we emit a new entry, but ultimately our fixit should update the existing entry or emit an error about a conflict - if moduleDependencies.contains($0.0) { return false } + if declared.contains($0.0) { return false } return true } } public func computeUnusedDependencies(usedModuleNames: Set) -> [ModuleDependency] { guard validateUnused != .no else { return [] } - return moduleDependencies.filter { !$0.optional && !usedModuleNames.contains($0.name) } + return declared.filter { !$0.optional && !usedModuleNames.contains($0.name) } } /// Make diagnostics for missing module dependencies. @@ -231,6 +231,7 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable { public struct HeaderDependency: Hashable, Sendable, SerializableCodable { public let name: String public let accessLevel: AccessLevel + public let optional: Bool public enum AccessLevel: String, Hashable, Sendable, CaseIterable, Codable, Serializable { case Private = "private" @@ -245,29 +246,25 @@ public struct HeaderDependency: Hashable, Sendable, SerializableCodable { } } - public init(name: String, accessLevel: AccessLevel) { + public init(name: String, accessLevel: AccessLevel, optional: Bool) { self.name = name self.accessLevel = accessLevel + self.optional = optional } public init(entry: String) throws { - var it = entry.split(separator: " ").makeIterator() - switch (it.next(), it.next(), it.next()) { - case (let .some(name), nil, nil): - self.name = String(name) - self.accessLevel = .Private - - case (let .some(accessLevel), let .some(name), nil): - self.name = String(name) - self.accessLevel = try AccessLevel(String(accessLevel)) - - default: - throw StubError.error("expected 1 or 2 space-separated components in: \(entry)") + let re = #/((?private|package|public)\s+)?(?.+)(?\?)?/# + guard let match = entry.wholeMatch(of: re) else { + throw StubError.error("Invalid header dependency format: \(entry), expected [private|package|public] [?]") } + + self.name = String(match.output.name) + self.accessLevel = try match.output.accessLevel.map { try AccessLevel(String($0)) } ?? .Private + self.optional = match.output.optional != nil } public var asBuildSettingEntry: String { - "\(accessLevel == .Private ? "" : "\(accessLevel.rawValue) ")\(name)" + "\(accessLevel == .Private ? "" : "\(accessLevel.rawValue) ")\(name)\(optional ? "?" : "")" } public var asBuildSettingEntryQuotedIfNeeded: String { @@ -278,63 +275,104 @@ public struct HeaderDependency: Hashable, Sendable, SerializableCodable { public struct HeaderDependenciesContext: Sendable, SerializableCodable { public var validate: BooleanWarningLevel - var headerDependencies: [HeaderDependency] + public var validateUnused: BooleanWarningLevel + public var declared: [HeaderDependency] var fixItContext: FixItContext? - init(validate: BooleanWarningLevel, headerDependencies: [HeaderDependency], fixItContext: FixItContext? = nil) { + init(validate: BooleanWarningLevel, validateUnused: BooleanWarningLevel, declared: [HeaderDependency], fixItContext: FixItContext? = nil) { self.validate = validate - self.headerDependencies = headerDependencies + self.validateUnused = validateUnused + self.declared = declared self.fixItContext = fixItContext } public init?(settings: Settings) { let validate = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_HEADER_DEPENDENCIES) - guard validate != .no else { return nil } + let validateUnused = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_UNUSED_HEADER_DEPENDENCIES) + guard validate != .no || validateUnused != .no else { return nil } let downgrade = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_DEPENDENCIES_DOWNGRADE_ERRORS) let fixItContext = HeaderDependenciesContext.FixItContext(settings: settings) - self.init(validate: downgrade ? .yes : validate, headerDependencies: settings.headerDependencies, fixItContext: fixItContext) + self.init(validate: downgrade ? .yes : validate, validateUnused: validateUnused, declared: settings.headerDependencies, fixItContext: fixItContext) + } + + public func makeUnsupportedToolchainDiagnostic() -> Diagnostic { + Diagnostic( + behavior: .warning, + location: .unknown, + data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_HEADER_DEPENDENCIES.name)")) + } + + /// Compute missing module dependencies. + public func computeMissingAndUnusedDependencies(includes: [Path]) -> (missing: [HeaderDependency], unused: [HeaderDependency]) { + let declaredNames = Set(declared.map { $0.name }) + + let missing: [HeaderDependency] + if validate != .no { + missing = includes.filter { file in + return !declaredNames.contains(where: { file.ends(with: $0) }) + }.map { + // TODO: What if the basename doesn't uniquely identify the header? + HeaderDependency(name: $0.basename, accessLevel: .Private, optional: false) + } + } + else { + missing = [] + } + + let unused: [HeaderDependency] + if validateUnused != .no { + unused = declared.filter { !$0.optional && !declaredNames.contains($0.name) } + } + else { + unused = [] + } + + return (missing, unused) } /// Make diagnostics for missing header dependencies. /// /// The compiler tracing information does not provide the include locations or whether they are public imports /// (which depends on whether the import is in an installed header file). - /// If `includes` is nil, the current toolchain does support the feature to trace imports. - public func makeDiagnostics(includes: [Path]?) -> [Diagnostic] { - guard validate != .no else { return [] } - guard let includes else { - return [Diagnostic( - behavior: .warning, - location: .unknown, - data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_HEADER_DEPENDENCIES.name)"))] - } - - let headerDependencyNames = headerDependencies.map { $0.name } - let missingDeps = includes.filter { file in - return !headerDependencyNames.contains(where: { file.ends(with: $0) }) - }.map { - // TODO: What if the basename doesn't uniquely identify the header? - HeaderDependency(name: $0.basename, accessLevel: .Private) - } + public func makeDiagnostics(missingDependencies: [HeaderDependency], unusedDependencies: [HeaderDependency]) -> [Diagnostic] { + let missingDiagnostics: [Diagnostic] + if !missingDependencies.isEmpty { + let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning - guard !missingDeps.isEmpty else { return [] } + let fixIt = fixItContext?.makeFixIt(newHeaders: missingDependencies) + let fixIts = fixIt.map { [$0] } ?? [] - let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning + let message = "Missing entries in \(BuiltinMacros.HEADER_DEPENDENCIES.name): \(missingDependencies.map { $0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))" - let fixIt = fixItContext?.makeFixIt(newHeaders: missingDeps) - let fixIts = fixIt.map { [$0] } ?? [] + let location: Diagnostic.Location = fixIt.map { + Diagnostic.Location.path($0.sourceRange.path, line: $0.sourceRange.endLine, column: $0.sourceRange.endColumn) + } ?? Diagnostic.Location.buildSetting(BuiltinMacros.HEADER_DEPENDENCIES) - let message = "Missing entries in \(BuiltinMacros.HEADER_DEPENDENCIES.name): \(missingDeps.map { $0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))" + missingDiagnostics = [Diagnostic( + behavior: behavior, + location: location, + data: DiagnosticData(message), + fixIts: fixIts)] + } + else { + missingDiagnostics = [] + } - let location: Diagnostic.Location = fixIt.map { - Diagnostic.Location.path($0.sourceRange.path, line: $0.sourceRange.endLine, column: $0.sourceRange.endColumn) - } ?? Diagnostic.Location.buildSetting(BuiltinMacros.HEADER_DEPENDENCIES) + let unusedDiagnostics: [Diagnostic] + if !unusedDependencies.isEmpty { + let message = "Unused entries in \(BuiltinMacros.HEADER_DEPENDENCIES.name): \(unusedDependencies.map { $0.name }.sorted().joined(separator: " "))" + // TODO location & fixit + unusedDiagnostics = [Diagnostic( + behavior: validateUnused == .yesError ? .error : .warning, + location: .unknown, + data: DiagnosticData(message), + fixIts: [])] + } + else { + unusedDiagnostics = [] + } - return [Diagnostic( - behavior: behavior, - location: location, - data: DiagnosticData(message), - fixIts: fixIts)] + return missingDiagnostics + unusedDiagnostics } struct FixItContext: Sendable, SerializableCodable { diff --git a/Sources/SWBCore/Settings/BuiltinMacros.swift b/Sources/SWBCore/Settings/BuiltinMacros.swift index 156cee04..e7507729 100644 --- a/Sources/SWBCore/Settings/BuiltinMacros.swift +++ b/Sources/SWBCore/Settings/BuiltinMacros.swift @@ -1151,6 +1151,7 @@ public final class BuiltinMacros { public static let VALIDATE_DEPENDENCIES_DOWNGRADE_ERRORS = BuiltinMacros.declareBooleanMacro("VALIDATE_DEPENDENCIES_DOWNGRADE_ERRORS") public static let VALIDATE_DEVELOPMENT_ASSET_PATHS = BuiltinMacros.declareEnumMacro("VALIDATE_DEVELOPMENT_ASSET_PATHS") as EnumMacroDeclaration public static let VALIDATE_HEADER_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_HEADER_DEPENDENCIES") as EnumMacroDeclaration + public static let VALIDATE_UNUSED_HEADER_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_UNUSED_HEADER_DEPENDENCIES") as EnumMacroDeclaration public static let VALIDATE_MODULE_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_MODULE_DEPENDENCIES") as EnumMacroDeclaration public static let VALIDATE_UNUSED_MODULE_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_UNUSED_MODULE_DEPENDENCIES") as EnumMacroDeclaration public static let VECTOR_SUFFIX = BuiltinMacros.declareStringMacro("VECTOR_SUFFIX") @@ -2388,6 +2389,7 @@ public final class BuiltinMacros { VALIDATE_DEVELOPMENT_ASSET_PATHS, VALIDATE_HEADER_DEPENDENCIES, VALIDATE_MODULE_DEPENDENCIES, + VALIDATE_UNUSED_HEADER_DEPENDENCIES, VALIDATE_UNUSED_MODULE_DEPENDENCIES, VALID_ARCHS, VECTOR_SUFFIX, diff --git a/Sources/SWBTaskExecution/Task.swift b/Sources/SWBTaskExecution/Task.swift index a923dbaf..97e5c8ab 100644 --- a/Sources/SWBTaskExecution/Task.swift +++ b/Sources/SWBTaskExecution/Task.swift @@ -608,8 +608,8 @@ public protocol TaskOutputDelegate: DiagnosticProducingDelegate /// Report a task which was previously batched as up-to-date. func previouslyBatchedSubtaskUpToDate(signature: ByteString, target: ConfiguredTarget) - func incrementCounter(_ counter: BuildOperationMetrics.Counter) - func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) + func incrementCounter(_ counter: BuildOperationMetrics.Counter, by amount: Int) + func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter, by amount: Int) var counters: [BuildOperationMetrics.Counter: Int] { get } var taskCounters: [BuildOperationMetrics.TaskCounter: Int] { get } @@ -634,6 +634,16 @@ package extension TaskOutputDelegate func emitNote(_ message: String) { note(message) } + + /// Convenience method for incrementing a counter by 1. + func incrementCounter(_ counter: BuildOperationMetrics.Counter) { + incrementCounter(counter, by: 1) + } + + /// Convenience method for incrementing a task counter by 1. + func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) { + incrementTaskCounter(counter, by: 1) + } } /// Convenience function for writing inline text output. diff --git a/Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift b/Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift index 67c3be65..74982139 100644 --- a/Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift +++ b/Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift @@ -341,7 +341,8 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA traceFilePath: traceFilePath, dependencyValidationOutputPath: dependencyValidationOutputPath, fileSystem: executionDelegate.fs, - isModular: true + isModular: true, + outputDelegate: outputDelegate ) } @@ -480,7 +481,8 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA traceFilePath: Path?, dependencyValidationOutputPath: Path?, fileSystem: any FSProxy, - isModular: Bool + isModular: Bool, + outputDelegate: any TaskOutputDelegate ) throws { let payload: DependencyValidationInfo.Payload if let traceFilePath { @@ -500,14 +502,24 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA var allFiles = Set() traceData.forEach { allFiles.formUnion(Set($0.includes)) } - + + outputDelegate.incrementTaskCounter(.headerDependenciesValidatedTasks) + if isModular { let (imports, includes) = separateImportsFromIncludes(allFiles) + outputDelegate.incrementTaskCounter(.moduleDependenciesValidatedTasks) + outputDelegate.incrementTaskCounter(.moduleDependenciesScanned, by: imports.count) + outputDelegate.incrementTaskCounter(.headerDependenciesScanned, by: includes.count) payload = .clangDependencies(imports: imports, includes: includes) } else { + outputDelegate.incrementTaskCounter(.headerDependenciesScanned, by: allFiles.count) payload = .clangDependencies(imports: [], includes: Array(allFiles)) } } else { + outputDelegate.incrementTaskCounter(.headerDependenciesNotValidatedTasks) + if isModular { + outputDelegate.incrementTaskCounter(.moduleDependenciesNotValidatedTasks) + } payload = .unsupported } @@ -598,7 +610,8 @@ public final class ClangNonModularCompileTaskAction: TaskAction { traceFilePath: traceFilePath, dependencyValidationOutputPath: dependencyValidationOutputPath, fileSystem: executionDelegate.fs, - isModular: false + isModular: false, + outputDelegate: outputDelegate ) } diff --git a/Sources/SWBTaskExecution/TaskActions/EmbedSwiftStdLibTaskAction.swift b/Sources/SWBTaskExecution/TaskActions/EmbedSwiftStdLibTaskAction.swift index 267a85f5..38501d22 100644 --- a/Sources/SWBTaskExecution/TaskActions/EmbedSwiftStdLibTaskAction.swift +++ b/Sources/SWBTaskExecution/TaskActions/EmbedSwiftStdLibTaskAction.swift @@ -472,8 +472,8 @@ public final class EmbedSwiftStdLibTaskAction: TaskAction { logV(args.joined(separator: " ")) final class CapturingOutputDelegate: TaskOutputDelegate { - func incrementCounter(_ counter: BuildOperationMetrics.Counter) {} - func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) {} + func incrementCounter(_ counter: BuildOperationMetrics.Counter, by amount: Int) {} + func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter, by amount: Int) {} var counters: [BuildOperationMetrics.Counter : Int] = [:] var taskCounters: [BuildOperationMetrics.TaskCounter : Int] = [:] diff --git a/Sources/SWBTaskExecution/TaskActions/GenericCachingTaskAction.swift b/Sources/SWBTaskExecution/TaskActions/GenericCachingTaskAction.swift index 093c3f33..d68eb864 100644 --- a/Sources/SWBTaskExecution/TaskActions/GenericCachingTaskAction.swift +++ b/Sources/SWBTaskExecution/TaskActions/GenericCachingTaskAction.swift @@ -404,12 +404,12 @@ fileprivate final class CapturingTaskOutputDelegate: TaskOutputDelegate { underlyingTaskOutputDelegate.previouslyBatchedSubtaskUpToDate(signature: signature, target: target) } - func incrementCounter(_ counter: BuildOperationMetrics.Counter) { - underlyingTaskOutputDelegate.incrementCounter(counter) + func incrementCounter(_ counter: BuildOperationMetrics.Counter, by amount: Int) { + underlyingTaskOutputDelegate.incrementCounter(counter, by: amount) } - func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) { - underlyingTaskOutputDelegate.incrementTaskCounter(counter) + func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter, by amount: Int) { + underlyingTaskOutputDelegate.incrementTaskCounter(counter, by: amount) } var counters: [BuildOperationMetrics.Counter : Int] { diff --git a/Sources/SWBTaskExecution/TaskActions/SwiftDriverTaskAction.swift b/Sources/SWBTaskExecution/TaskActions/SwiftDriverTaskAction.swift index 082945bd..85f7a182 100644 --- a/Sources/SWBTaskExecution/TaskActions/SwiftDriverTaskAction.swift +++ b/Sources/SWBTaskExecution/TaskActions/SwiftDriverTaskAction.swift @@ -16,6 +16,7 @@ import SWBUtil import Foundation internal import SwiftDriver internal import SWBMacro +internal import SWBProtocol final public class SwiftDriverTaskAction: TaskAction, BuildValueValidatingTaskAction { public override class var toolIdentifier: String { @@ -101,8 +102,11 @@ final public class SwiftDriverTaskAction: TaskAction, BuildValueValidatingTaskAc let payload: DependencyValidationInfo.Payload if let imports = try await dependencyGraph.mainModuleImportModuleDependencies(for: driverPayload.uniqueID) { payload = .swiftDependencies(imports: imports.map { .init(dependency: $0.0, importLocations: $0.importLocations) }) + outputDelegate.incrementTaskCounter(.moduleDependenciesValidatedTasks) + outputDelegate.incrementTaskCounter(.moduleDependenciesScanned, by: imports.count) } else { payload = .unsupported + outputDelegate.incrementTaskCounter(.moduleDependenciesNotValidatedTasks) } let validationInfo = DependencyValidationInfo(payload: payload) _ = try executionDelegate.fs.writeIfChanged( @@ -112,6 +116,9 @@ final public class SwiftDriverTaskAction: TaskAction, BuildValueValidatingTaskAc ) ) } + else { + outputDelegate.incrementTaskCounter(.moduleDependenciesNotValidatedTasks) + } if driverPayload.reportRequiredTargetDependencies != .no && driverPayload.explicitModulesEnabled, let target = task.forTarget { let dependencyModuleNames = try await dependencyGraph.queryTransitiveDependencyModuleNames(for: driverPayload.uniqueID) diff --git a/Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift b/Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift index 9844389c..9a09bf69 100644 --- a/Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift +++ b/Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift @@ -13,6 +13,7 @@ import Foundation public import SWBCore import SWBUtil +internal import SWBProtocol public final class ValidateDependenciesTaskAction: TaskAction { public override class var toolIdentifier: String { @@ -35,6 +36,14 @@ public final class ValidateDependenciesTaskAction: TaskAction { return .failed } + if let ctx = payload.moduleDependenciesContext { + outputDelegate.incrementCounter(.moduleDependenciesDeclared, by: ctx.declared.count) + } + + if let ctx = payload.headerDependenciesContext { + outputDelegate.incrementCounter(.moduleDependenciesDeclared, by: ctx.declared.count) + } + do { var allClangIncludes = Set() var allClangImports = Set() @@ -91,16 +100,30 @@ public final class ValidateDependenciesTaskAction: TaskAction { let swiftImports = Set(allSwiftImports.map { $0.dependency.name }) let uniqueClangMissingDeps = clangMissingDeps.filter { !swiftImports.contains($0.0.name) } + let missingDeps = uniqueClangMissingDeps + updatedSwiftMissingDeps + outputDelegate.incrementCounter(.moduleDependenciesMissing, by: missingDeps.count) + let unusedDeps = moduleContext.computeUnusedDependencies(usedModuleNames: Set(allClangImports.map { $0.dependency.name } + allSwiftImports.map { $0.dependency.name })) + outputDelegate.incrementCounter(.moduleDependenciesUnused, by: unusedDeps.count) - diagnostics.append(contentsOf: moduleContext.makeDiagnostics(missingDependencies: uniqueClangMissingDeps + updatedSwiftMissingDeps, unusedDependencies: unusedDeps)) + let diags = moduleContext.makeDiagnostics(missingDependencies: missingDeps, unusedDependencies: unusedDeps) + outputDelegate.incrementCounter(.moduleDependenciesWarningsEmitted, by: diags.lazy.filter { $0.behavior == .warning }.count) + outputDelegate.incrementCounter(.moduleDependenciesErrorsEmitted, by: diags.lazy.filter { $0.behavior == .error }.count) + diagnostics.append(contentsOf: diags) } } if let headerContext = payload.headerDependenciesContext { if unsupported { - diagnostics.append(contentsOf: headerContext.makeDiagnostics(includes: nil)) + diagnostics.append(contentsOf: [headerContext.makeUnsupportedToolchainDiagnostic()]) } else { - diagnostics.append(contentsOf: headerContext.makeDiagnostics(includes: Array(allClangIncludes))) + let (missingDeps, unusedDeps) = headerContext.computeMissingAndUnusedDependencies(includes: Array(allClangIncludes)) + outputDelegate.incrementCounter(.headerDependenciesMissing, by: missingDeps.count) + outputDelegate.incrementCounter(.headerDependenciesUnused, by: unusedDeps.count) + + let diags = headerContext.makeDiagnostics(missingDependencies: missingDeps, unusedDependencies: unusedDeps) + outputDelegate.incrementCounter(.headerDependenciesWarningsEmitted, by: diags.lazy.filter { $0.behavior == .warning }.count) + outputDelegate.incrementCounter(.headerDependenciesErrorsEmitted, by: diags.lazy.filter { $0.behavior == .error }.count) + diagnostics.append(contentsOf: diags) } } diff --git a/Sources/SWBTestSupport/BuildOperationTester.swift b/Sources/SWBTestSupport/BuildOperationTester.swift index 82e6c9c4..735bdf07 100644 --- a/Sources/SWBTestSupport/BuildOperationTester.swift +++ b/Sources/SWBTestSupport/BuildOperationTester.swift @@ -1783,12 +1783,12 @@ private final class BuildOperationTesterDelegate: BuildOperationDelegate { } private class TesterTaskOutputDelegate: TaskOutputDelegate { - func incrementCounter(_ counter: BuildOperationMetrics.Counter) { - self.counters[counter, default: 0] += 1 + func incrementCounter(_ counter: BuildOperationMetrics.Counter, by amount: Int) { + self.counters[counter, default: 0] += amount } - func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) { - self.taskCounters[counter, default: 0] += 1 + func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter, by amount: Int) { + self.taskCounters[counter, default: 0] += amount } var counters: [BuildOperationMetrics.Counter : Int] = [.clangCacheHits: 0, .clangCacheMisses: 0, .swiftCacheHits: 0, .swiftCacheMisses: 0] diff --git a/Tests/SWBTaskExecutionTests/InProcessTaskTestSupport.swift b/Tests/SWBTaskExecutionTests/InProcessTaskTestSupport.swift index 4a2ee5ef..ba41b03b 100644 --- a/Tests/SWBTaskExecutionTests/InProcessTaskTestSupport.swift +++ b/Tests/SWBTaskExecutionTests/InProcessTaskTestSupport.swift @@ -71,15 +71,15 @@ final class MockTaskOutputDelegate: TaskOutputDelegate { private let _diagnosticsEngine = DiagnosticsEngine() - func incrementCounter(_ counter: BuildOperationMetrics.Counter) { + func incrementCounter(_ counter: BuildOperationMetrics.Counter, by amount: Int) { state.state.withLock { state in - state.counters[counter, default: 0] += 1 + state.counters[counter, default: 0] += amount } } - func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) { + func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter, by amount: Int) { state.state.withLock { state in - state.taskCounters[counter, default: 0] += 1 + state.taskCounters[counter, default: 0] += amount } }