Skip to content

Commit 4e31842

Browse files
mirza-garibovicmirza-garibovic
committed
Dependencies: fire metrics (rdar://150307704) + add unused header validation (rdar://159126160)
1 parent a87ec6a commit 4e31842

File tree

11 files changed

+177
-84
lines changed

11 files changed

+177
-84
lines changed

‎Sources/SWBBuildService/BuildOperationMessages.swift‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -705,12 +705,12 @@ private final class TaskOutputHandler: TaskOutputDelegate {
705705
}
706706
}
707707

708-
func incrementCounter(_ counter: BuildOperationMetrics.Counter) {
709-
self.counters[counter, default: 0] += 1
708+
func incrementCounter(_ counter: BuildOperationMetrics.Counter, by amount: Int) {
709+
self.counters[counter, default: 0] += amount
710710
}
711711

712-
func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) {
713-
self.taskCounters[counter, default: 0] += 1
712+
func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter, by amount: Int) {
713+
self.taskCounters[counter, default: 0] += amount
714714
}
715715

716716
var diagnosticsEngine: DiagnosticProducingDelegateProtocolPrivate<DiagnosticsEngine> {
@@ -932,8 +932,8 @@ private final class DiscardingTaskOutputHandler: TaskOutputDelegate {
932932
func subtaskUpToDate(_ subtask: any ExecutableTask) {}
933933
func previouslyBatchedSubtaskUpToDate(signature: ByteString, target: ConfiguredTarget) {}
934934
func updateResult(_ result: TaskResult) {}
935-
func incrementCounter(_ counter: BuildOperationMetrics.Counter) {}
936-
func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) {}
935+
func incrementCounter(_ counter: BuildOperationMetrics.Counter, by amount: Int) {}
936+
func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter, by amount: Int) {}
937937
}
938938

939939
/// The build output delegate, which sends data back immediately.

‎Sources/SWBCore/Dependencies.swift‎

Lines changed: 93 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,13 @@ public struct ModuleDependency: Hashable, Sendable, SerializableCodable {
7474
public struct ModuleDependenciesContext: Sendable, SerializableCodable {
7575
public var validate: BooleanWarningLevel
7676
public var validateUnused: BooleanWarningLevel
77-
var moduleDependencies: [ModuleDependency]
77+
public var declared: [ModuleDependency]
7878
var fixItContext: FixItContext?
7979

80-
init(validate: BooleanWarningLevel, validateUnused: BooleanWarningLevel, moduleDependencies: [ModuleDependency], fixItContext: FixItContext? = nil) {
80+
init(validate: BooleanWarningLevel, validateUnused: BooleanWarningLevel, declared: [ModuleDependency], fixItContext: FixItContext? = nil) {
8181
self.validate = validate
8282
self.validateUnused = validateUnused
83-
self.moduleDependencies = moduleDependencies
83+
self.declared = declared
8484
self.fixItContext = fixItContext
8585
}
8686

@@ -90,7 +90,7 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable {
9090
guard validate != .no || validateUnused != .no else { return nil }
9191
let downgrade = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_DEPENDENCIES_DOWNGRADE_ERRORS)
9292
let fixItContext = validate != .no ? ModuleDependenciesContext.FixItContext(settings: settings) : nil
93-
self.init(validate: downgrade ? .yes : validate, validateUnused: validateUnused, moduleDependencies: settings.moduleDependencies, fixItContext: fixItContext)
93+
self.init(validate: downgrade ? .yes : validate, validateUnused: validateUnused, declared: settings.moduleDependencies, fixItContext: fixItContext)
9494
}
9595

9696
public func makeUnsupportedToolchainDiagnostic() -> Diagnostic {
@@ -111,14 +111,14 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable {
111111
if fromSwift && $0.importLocations.isEmpty { return false }
112112

113113
// 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
114-
if moduleDependencies.contains($0.0) { return false }
114+
if declared.contains($0.0) { return false }
115115
return true
116116
}
117117
}
118118

119119
public func computeUnusedDependencies(usedModuleNames: Set<String>) -> [ModuleDependency] {
120120
guard validateUnused != .no else { return [] }
121-
return moduleDependencies.filter { !$0.optional && !usedModuleNames.contains($0.name) }
121+
return declared.filter { !$0.optional && !usedModuleNames.contains($0.name) }
122122
}
123123

124124
/// Make diagnostics for missing module dependencies.
@@ -231,6 +231,7 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable {
231231
public struct HeaderDependency: Hashable, Sendable, SerializableCodable {
232232
public let name: String
233233
public let accessLevel: AccessLevel
234+
public let optional: Bool
234235

235236
public enum AccessLevel: String, Hashable, Sendable, CaseIterable, Codable, Serializable {
236237
case Private = "private"
@@ -245,29 +246,25 @@ public struct HeaderDependency: Hashable, Sendable, SerializableCodable {
245246
}
246247
}
247248

248-
public init(name: String, accessLevel: AccessLevel) {
249+
public init(name: String, accessLevel: AccessLevel, optional: Bool) {
249250
self.name = name
250251
self.accessLevel = accessLevel
252+
self.optional = optional
251253
}
252254

253255
public init(entry: String) throws {
254-
var it = entry.split(separator: " ").makeIterator()
255-
switch (it.next(), it.next(), it.next()) {
256-
case (let .some(name), nil, nil):
257-
self.name = String(name)
258-
self.accessLevel = .Private
259-
260-
case (let .some(accessLevel), let .some(name), nil):
261-
self.name = String(name)
262-
self.accessLevel = try AccessLevel(String(accessLevel))
263-
264-
default:
265-
throw StubError.error("expected 1 or 2 space-separated components in: \(entry)")
256+
let re = #/((?<accessLevel>private|package|public)\s+)?(?<name>.+)(?<optional>\?)?/#
257+
guard let match = entry.wholeMatch(of: re) else {
258+
throw StubError.error("Invalid header dependency format: \(entry), expected [private|package|public] <name>[?]")
266259
}
260+
261+
self.name = String(match.output.name)
262+
self.accessLevel = try match.output.accessLevel.map { try AccessLevel(String($0)) } ?? .Private
263+
self.optional = match.output.optional != nil
267264
}
268265

269266
public var asBuildSettingEntry: String {
270-
"\(accessLevel == .Private ? "" : "\(accessLevel.rawValue) ")\(name)"
267+
"\(accessLevel == .Private ? "" : "\(accessLevel.rawValue) ")\(name)\(optional ? "?" : "")"
271268
}
272269

273270
public var asBuildSettingEntryQuotedIfNeeded: String {
@@ -278,63 +275,104 @@ public struct HeaderDependency: Hashable, Sendable, SerializableCodable {
278275

279276
public struct HeaderDependenciesContext: Sendable, SerializableCodable {
280277
public var validate: BooleanWarningLevel
281-
var headerDependencies: [HeaderDependency]
278+
public var validateUnused: BooleanWarningLevel
279+
public var declared: [HeaderDependency]
282280
var fixItContext: FixItContext?
283281

284-
init(validate: BooleanWarningLevel, headerDependencies: [HeaderDependency], fixItContext: FixItContext? = nil) {
282+
init(validate: BooleanWarningLevel, validateUnused: BooleanWarningLevel, declared: [HeaderDependency], fixItContext: FixItContext? = nil) {
285283
self.validate = validate
286-
self.headerDependencies = headerDependencies
284+
self.validateUnused = validateUnused
285+
self.declared = declared
287286
self.fixItContext = fixItContext
288287
}
289288

290289
public init?(settings: Settings) {
291290
let validate = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_HEADER_DEPENDENCIES)
292-
guard validate != .no else { return nil }
291+
let validateUnused = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_UNUSED_HEADER_DEPENDENCIES)
292+
guard validate != .no || validateUnused != .no else { return nil }
293293
let downgrade = settings.globalScope.evaluate(BuiltinMacros.VALIDATE_DEPENDENCIES_DOWNGRADE_ERRORS)
294294
let fixItContext = HeaderDependenciesContext.FixItContext(settings: settings)
295-
self.init(validate: downgrade ? .yes : validate, headerDependencies: settings.headerDependencies, fixItContext: fixItContext)
295+
self.init(validate: downgrade ? .yes : validate, validateUnused: validateUnused, declared: settings.headerDependencies, fixItContext: fixItContext)
296+
}
297+
298+
public func makeUnsupportedToolchainDiagnostic() -> Diagnostic {
299+
Diagnostic(
300+
behavior: .warning,
301+
location: .unknown,
302+
data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_HEADER_DEPENDENCIES.name)"))
303+
}
304+
305+
/// Compute missing module dependencies.
306+
public func computeMissingAndUnusedDependencies(includes: [Path]) -> (missing: [HeaderDependency], unused: [HeaderDependency]) {
307+
let declaredNames = Set(declared.map { $0.name })
308+
309+
let missing: [HeaderDependency]
310+
if validate != .no {
311+
missing = includes.filter { file in
312+
return !declaredNames.contains(where: { file.ends(with: $0) })
313+
}.map {
314+
// TODO: What if the basename doesn't uniquely identify the header?
315+
HeaderDependency(name: $0.basename, accessLevel: .Private, optional: false)
316+
}
317+
}
318+
else {
319+
missing = []
320+
}
321+
322+
let unused: [HeaderDependency]
323+
if validateUnused != .no {
324+
unused = declared.filter { !$0.optional && !declaredNames.contains($0.name) }
325+
}
326+
else {
327+
unused = []
328+
}
329+
330+
return (missing, unused)
296331
}
297332

298333
/// Make diagnostics for missing header dependencies.
299334
///
300335
/// The compiler tracing information does not provide the include locations or whether they are public imports
301336
/// (which depends on whether the import is in an installed header file).
302-
/// If `includes` is nil, the current toolchain does support the feature to trace imports.
303-
public func makeDiagnostics(includes: [Path]?) -> [Diagnostic] {
304-
guard validate != .no else { return [] }
305-
guard let includes else {
306-
return [Diagnostic(
307-
behavior: .warning,
308-
location: .unknown,
309-
data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_HEADER_DEPENDENCIES.name)"))]
310-
}
311-
312-
let headerDependencyNames = headerDependencies.map { $0.name }
313-
let missingDeps = includes.filter { file in
314-
return !headerDependencyNames.contains(where: { file.ends(with: $0) })
315-
}.map {
316-
// TODO: What if the basename doesn't uniquely identify the header?
317-
HeaderDependency(name: $0.basename, accessLevel: .Private)
318-
}
337+
public func makeDiagnostics(missingDependencies: [HeaderDependency], unusedDependencies: [HeaderDependency]) -> [Diagnostic] {
338+
let missingDiagnostics: [Diagnostic]
339+
if !missingDependencies.isEmpty {
340+
let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning
319341

320-
guard !missingDeps.isEmpty else { return [] }
342+
let fixIt = fixItContext?.makeFixIt(newHeaders: missingDependencies)
343+
let fixIts = fixIt.map { [$0] } ?? []
321344

322-
let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning
345+
let message = "Missing entries in \(BuiltinMacros.HEADER_DEPENDENCIES.name): \(missingDependencies.map { $0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))"
323346

324-
let fixIt = fixItContext?.makeFixIt(newHeaders: missingDeps)
325-
let fixIts = fixIt.map { [$0] } ?? []
347+
let location: Diagnostic.Location = fixIt.map {
348+
Diagnostic.Location.path($0.sourceRange.path, line: $0.sourceRange.endLine, column: $0.sourceRange.endColumn)
349+
} ?? Diagnostic.Location.buildSetting(BuiltinMacros.HEADER_DEPENDENCIES)
326350

327-
let message = "Missing entries in \(BuiltinMacros.HEADER_DEPENDENCIES.name): \(missingDeps.map { $0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))"
351+
missingDiagnostics = [Diagnostic(
352+
behavior: behavior,
353+
location: location,
354+
data: DiagnosticData(message),
355+
fixIts: fixIts)]
356+
}
357+
else {
358+
missingDiagnostics = []
359+
}
328360

329-
let location: Diagnostic.Location = fixIt.map {
330-
Diagnostic.Location.path($0.sourceRange.path, line: $0.sourceRange.endLine, column: $0.sourceRange.endColumn)
331-
} ?? Diagnostic.Location.buildSetting(BuiltinMacros.HEADER_DEPENDENCIES)
361+
let unusedDiagnostics: [Diagnostic]
362+
if !unusedDependencies.isEmpty {
363+
let message = "Unused entries in \(BuiltinMacros.HEADER_DEPENDENCIES.name): \(unusedDependencies.map { $0.name }.sorted().joined(separator: " "))"
364+
// TODO location & fixit
365+
unusedDiagnostics = [Diagnostic(
366+
behavior: validateUnused == .yesError ? .error : .warning,
367+
location: .unknown,
368+
data: DiagnosticData(message),
369+
fixIts: [])]
370+
}
371+
else {
372+
unusedDiagnostics = []
373+
}
332374

333-
return [Diagnostic(
334-
behavior: behavior,
335-
location: location,
336-
data: DiagnosticData(message),
337-
fixIts: fixIts)]
375+
return missingDiagnostics + unusedDiagnostics
338376
}
339377

340378
struct FixItContext: Sendable, SerializableCodable {

‎Sources/SWBCore/Settings/BuiltinMacros.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,6 +1153,7 @@ public final class BuiltinMacros {
11531153
public static let VALIDATE_DEPENDENCIES_DOWNGRADE_ERRORS = BuiltinMacros.declareBooleanMacro("VALIDATE_DEPENDENCIES_DOWNGRADE_ERRORS")
11541154
public static let VALIDATE_DEVELOPMENT_ASSET_PATHS = BuiltinMacros.declareEnumMacro("VALIDATE_DEVELOPMENT_ASSET_PATHS") as EnumMacroDeclaration<BooleanWarningLevel>
11551155
public static let VALIDATE_HEADER_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_HEADER_DEPENDENCIES") as EnumMacroDeclaration<BooleanWarningLevel>
1156+
public static let VALIDATE_UNUSED_HEADER_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_UNUSED_HEADER_DEPENDENCIES") as EnumMacroDeclaration<BooleanWarningLevel>
11561157
public static let VALIDATE_MODULE_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_MODULE_DEPENDENCIES") as EnumMacroDeclaration<BooleanWarningLevel>
11571158
public static let VALIDATE_UNUSED_MODULE_DEPENDENCIES = BuiltinMacros.declareEnumMacro("VALIDATE_UNUSED_MODULE_DEPENDENCIES") as EnumMacroDeclaration<BooleanWarningLevel>
11581159
public static let VECTOR_SUFFIX = BuiltinMacros.declareStringMacro("VECTOR_SUFFIX")
@@ -2392,6 +2393,7 @@ public final class BuiltinMacros {
23922393
VALIDATE_DEVELOPMENT_ASSET_PATHS,
23932394
VALIDATE_HEADER_DEPENDENCIES,
23942395
VALIDATE_MODULE_DEPENDENCIES,
2396+
VALIDATE_UNUSED_HEADER_DEPENDENCIES,
23952397
VALIDATE_UNUSED_MODULE_DEPENDENCIES,
23962398
VALID_ARCHS,
23972399
VECTOR_SUFFIX,

‎Sources/SWBTaskExecution/Task.swift‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -608,8 +608,8 @@ public protocol TaskOutputDelegate: DiagnosticProducingDelegate
608608
/// Report a task which was previously batched as up-to-date.
609609
func previouslyBatchedSubtaskUpToDate(signature: ByteString, target: ConfiguredTarget)
610610

611-
func incrementCounter(_ counter: BuildOperationMetrics.Counter)
612-
func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter)
611+
func incrementCounter(_ counter: BuildOperationMetrics.Counter, by amount: Int)
612+
func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter, by amount: Int)
613613

614614
var counters: [BuildOperationMetrics.Counter: Int] { get }
615615
var taskCounters: [BuildOperationMetrics.TaskCounter: Int] { get }
@@ -634,6 +634,16 @@ package extension TaskOutputDelegate
634634
func emitNote(_ message: String) {
635635
note(message)
636636
}
637+
638+
/// Convenience method for incrementing a counter by 1.
639+
func incrementCounter(_ counter: BuildOperationMetrics.Counter) {
640+
incrementCounter(counter, by: 1)
641+
}
642+
643+
/// Convenience method for incrementing a task counter by 1.
644+
func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) {
645+
incrementTaskCounter(counter, by: 1)
646+
}
637647
}
638648

639649
/// Convenience function for writing inline text output.

‎Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,8 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
341341
traceFilePath: traceFilePath,
342342
dependencyValidationOutputPath: dependencyValidationOutputPath,
343343
fileSystem: executionDelegate.fs,
344-
isModular: true
344+
isModular: true,
345+
outputDelegate: outputDelegate
345346
)
346347
}
347348

@@ -480,7 +481,8 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
480481
traceFilePath: Path?,
481482
dependencyValidationOutputPath: Path?,
482483
fileSystem: any FSProxy,
483-
isModular: Bool
484+
isModular: Bool,
485+
outputDelegate: any TaskOutputDelegate
484486
) throws {
485487
let payload: DependencyValidationInfo.Payload
486488
if let traceFilePath {
@@ -500,14 +502,24 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
500502

501503
var allFiles = Set<Path>()
502504
traceData.forEach { allFiles.formUnion(Set($0.includes)) }
503-
505+
506+
outputDelegate.incrementTaskCounter(.headerDependenciesValidatedTasks)
507+
504508
if isModular {
505509
let (imports, includes) = separateImportsFromIncludes(allFiles)
510+
outputDelegate.incrementTaskCounter(.moduleDependenciesValidatedTasks)
511+
outputDelegate.incrementTaskCounter(.moduleDependenciesScanned, by: imports.count)
512+
outputDelegate.incrementTaskCounter(.headerDependenciesScanned, by: includes.count)
506513
payload = .clangDependencies(imports: imports, includes: includes)
507514
} else {
515+
outputDelegate.incrementTaskCounter(.headerDependenciesScanned, by: allFiles.count)
508516
payload = .clangDependencies(imports: [], includes: Array(allFiles))
509517
}
510518
} else {
519+
outputDelegate.incrementTaskCounter(.headerDependenciesNotValidatedTasks)
520+
if isModular {
521+
outputDelegate.incrementTaskCounter(.moduleDependenciesNotValidatedTasks)
522+
}
511523
payload = .unsupported
512524
}
513525

@@ -598,7 +610,8 @@ public final class ClangNonModularCompileTaskAction: TaskAction {
598610
traceFilePath: traceFilePath,
599611
dependencyValidationOutputPath: dependencyValidationOutputPath,
600612
fileSystem: executionDelegate.fs,
601-
isModular: false
613+
isModular: false,
614+
outputDelegate: outputDelegate
602615
)
603616
}
604617

‎Sources/SWBTaskExecution/TaskActions/EmbedSwiftStdLibTaskAction.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,8 +472,8 @@ public final class EmbedSwiftStdLibTaskAction: TaskAction {
472472
logV(args.joined(separator: " "))
473473

474474
final class CapturingOutputDelegate: TaskOutputDelegate {
475-
func incrementCounter(_ counter: BuildOperationMetrics.Counter) {}
476-
func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) {}
475+
func incrementCounter(_ counter: BuildOperationMetrics.Counter, by amount: Int) {}
476+
func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter, by amount: Int) {}
477477

478478
var counters: [BuildOperationMetrics.Counter : Int] = [:]
479479
var taskCounters: [BuildOperationMetrics.TaskCounter : Int] = [:]

‎Sources/SWBTaskExecution/TaskActions/GenericCachingTaskAction.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -404,12 +404,12 @@ fileprivate final class CapturingTaskOutputDelegate: TaskOutputDelegate {
404404
underlyingTaskOutputDelegate.previouslyBatchedSubtaskUpToDate(signature: signature, target: target)
405405
}
406406

407-
func incrementCounter(_ counter: BuildOperationMetrics.Counter) {
408-
underlyingTaskOutputDelegate.incrementCounter(counter)
407+
func incrementCounter(_ counter: BuildOperationMetrics.Counter, by amount: Int) {
408+
underlyingTaskOutputDelegate.incrementCounter(counter, by: amount)
409409
}
410410

411-
func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter) {
412-
underlyingTaskOutputDelegate.incrementTaskCounter(counter)
411+
func incrementTaskCounter(_ counter: BuildOperationMetrics.TaskCounter, by amount: Int) {
412+
underlyingTaskOutputDelegate.incrementTaskCounter(counter, by: amount)
413413
}
414414

415415
var counters: [BuildOperationMetrics.Counter : Int] {

0 commit comments

Comments
 (0)