Skip to content

Commit 99d122c

Browse files
committed
Dependencies: support clang modules when parsing dependency validation info
1 parent 9b6ffb8 commit 99d122c

File tree

4 files changed

+112
-40
lines changed

4 files changed

+112
-40
lines changed

Sources/SWBCore/Dependencies.swift

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,8 +400,18 @@ public struct DependencyValidationInfo: Hashable, Sendable, Codable {
400400
}
401401
}
402402

403+
public struct Include: Hashable, Sendable, Codable {
404+
public let path: Path
405+
public let includeLocations: [Diagnostic.Location]
406+
407+
public init(path: Path, includeLocations: [Diagnostic.Location]) {
408+
self.path = path
409+
self.includeLocations = includeLocations
410+
}
411+
}
412+
403413
public enum Payload: Hashable, Sendable, Codable {
404-
case clangDependencies(imports: [Import], includes: [Path])
414+
case clangDependencies(imports: [Import], includes: [Include])
405415
case swiftDependencies(imports: [Import])
406416
case unsupported
407417
}

Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift

Lines changed: 98 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,6 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
352352
}
353353
}
354354

355-
356-
357355
/// Intended to be called during task dependency setup.
358356
/// If remote caching is enabled along with integrated cache queries, it will request
359357
/// a `ClangCachingMaterializeKeyTaskAction` as task dependency.
@@ -483,29 +481,39 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
483481
isModular: Bool
484482
) throws {
485483
let payload: DependencyValidationInfo.Payload
486-
if let traceFilePath {
487-
let traceFileContent = try fileSystem.read(traceFilePath)
484+
if let traceFilePath,
485+
let traceData = try parseTraceData(Data(fileSystem.read(traceFilePath))) {
488486

489-
let traceData: Array<TraceData>
490-
// clang will emit an empty file instead of an empty array when there's nothing to trace
491-
if traceFileContent.isEmpty {
492-
traceData = []
493-
} else {
494-
do {
495-
traceData = try JSONDecoder().decode(Array<TraceData>.self, from: Data(traceFileContent))
496-
} catch {
497-
throw StubError.error("Failed to decode json trace at \(traceFilePath.str): \(error)")
487+
switch traceData {
488+
case let .V1(traceDataV1):
489+
// mapping each header path to the set of locations that include it
490+
var allFiles = [Path: Set<SWBUtil.Diagnostic.Location>]();
491+
492+
for entry in traceDataV1 {
493+
entry.includes.forEach { allFiles[$0, default: []].insert(.path(entry.source, fileLocation: nil)) }
498494
}
499-
}
500-
501-
var allFiles = Set<Path>()
502-
traceData.forEach { allFiles.formUnion(Set($0.includes)) }
503-
504-
if isModular {
505-
let (imports, includes) = separateImportsFromIncludes(allFiles)
495+
496+
if isModular {
497+
let (imports, includes) = separateImportsFromIncludes(allFiles)
498+
payload = .clangDependencies(imports: imports, includes: includes)
499+
} else {
500+
let includes = allFiles.map { file, locations in DependencyValidationInfo.Include(path: file, includeLocations: Array(locations)) }
501+
payload = .clangDependencies(imports: [], includes: includes)
502+
}
503+
case let .V2(traceDataV2):
504+
var allIncludes = [Path: Set<SWBUtil.Diagnostic.Location>]();
505+
var allImports = [String: Set<SWBUtil.Diagnostic.Location>]();
506+
507+
for entry in traceDataV2.dependencies {
508+
entry.includes.forEach { allIncludes[$0.file, default: []].insert(parseTraceSourceLocation($0.location)) }
509+
if isModular {
510+
entry.imports.forEach { allImports[$0.module, default: []].insert(parseTraceSourceLocation($0.location)) }
511+
}
512+
}
513+
514+
let imports = allImports.map { name, locations in DependencyValidationInfo.Import(dependency: ModuleDependency(name: name, accessLevel: .Private, optional: false), importLocations: Array(locations)) }
515+
let includes = allIncludes.map { file, locations in DependencyValidationInfo.Include(path: file, includeLocations: Array(locations)) }
506516
payload = .clangDependencies(imports: imports, includes: includes)
507-
} else {
508-
payload = .clangDependencies(imports: [], includes: Array(allFiles))
509517
}
510518
} else {
511519
payload = .unsupported
@@ -522,29 +530,27 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
522530
}
523531
}
524532

525-
// Clang's dependency tracing does not currently clearly distinguish modular imports from non-modular includes.
526-
// Until that gets fixed, just guess that if the file is contained in a framework, it comes from a module with
533+
// Clang's dependency tracing V1 did not clearly distinguish modular imports from non-modular includes.
534+
// To keep supporting those trace files, just guess that if the file is contained in a framework, it comes from a module with
527535
// the same name. That is obviously not going to be reliable but it unblocks us from continuing experiments with
528536
// dependency specifications.
529-
private static func separateImportsFromIncludes(_ files: Set<Path>) -> ([DependencyValidationInfo.Import], [Path]) {
537+
private static func separateImportsFromIncludes(_ files: [Path: Set<SWBUtil.Diagnostic.Location>]) -> ([DependencyValidationInfo.Import], [DependencyValidationInfo.Include]) {
530538
func findFrameworkName(_ file: Path) -> String? {
531539
if file.fileExtension == "framework" {
532540
return file.basenameWithoutSuffix
533541
}
534542
return file.dirname.isEmpty || file.dirname.isRoot ? nil : findFrameworkName(file.dirname)
535543
}
536-
var moduleNames: [String] = []
537-
var includeFiles: [Path] = []
538-
for file in files {
544+
var moduleImports: [DependencyValidationInfo.Import] = []
545+
var headerIncludes: [DependencyValidationInfo.Include] = []
546+
for (file, includeLocations) in files {
539547
if let frameworkName = findFrameworkName(file) {
540-
moduleNames.append(frameworkName)
548+
moduleImports.append(DependencyValidationInfo.Import(dependency: ModuleDependency(name: frameworkName, accessLevel: .Private, optional: false), importLocations: Array(includeLocations)))
541549
} else {
542-
includeFiles.append(file)
550+
headerIncludes.append(DependencyValidationInfo.Include(path: file, includeLocations: Array(includeLocations)))
543551
}
544552
}
545-
let moduleDependencies = moduleNames.map { ModuleDependency(name: $0, accessLevel: .Private, optional: false) }
546-
let moduleImports = moduleDependencies.map { DependencyValidationInfo.Import(dependency: $0, importLocations: []) }
547-
return (moduleImports, includeFiles)
553+
return (moduleImports, headerIncludes)
548554
}
549555
}
550556

@@ -610,9 +616,65 @@ public final class ClangNonModularCompileTaskAction: TaskAction {
610616
}
611617
}
612618

619+
fileprivate func parseTraceData(_ data: Data) throws -> TraceData? {
620+
if let jsonObject = try JSONSerialization.jsonObject(with: data) as? [String: Any],
621+
let version = jsonObject["version"] as? String {
622+
if version == "2.0.0" {
623+
return .V2(try JSONDecoder().decode(TraceData.TraceFileV2.self, from: data))
624+
}
625+
return nil
626+
} else {
627+
// The initial unversioned format (v1) of the trace file generated from clang
628+
// is a JSON array so deserializing it as a dictionary will fail.
629+
return .V1(try JSONDecoder().decode(TraceData.TraceFileV1.self, from: data))
630+
}
631+
}
632+
633+
fileprivate func parseTraceSourceLocation(_ locationStr: String) -> SWBUtil.Diagnostic.Location {
634+
guard let match = locationStr.wholeMatch(of: #/([^:]+):(\d+):(\d+)/#) else {
635+
return .unknown
636+
}
637+
let filename = Path(match.1)
638+
let line = Int(match.2)
639+
let column = Int(match.3)
640+
if let line {
641+
return .path(filename, fileLocation: .textual(line: line, column: column))
642+
}
643+
return .unknown
644+
}
645+
613646
// Results from tracing header includes with "direct-per-file" filtering.
614647
// This is used to validate dependencies.
615-
fileprivate struct TraceData: Decodable {
616-
let source: Path
617-
let includes: [Path]
648+
fileprivate enum TraceData: Decodable {
649+
fileprivate struct Include: Decodable {
650+
let location: String
651+
let file: Path
652+
}
653+
654+
fileprivate struct Import: Decodable {
655+
let location: String
656+
let module: String
657+
let file: Path
658+
}
659+
660+
fileprivate struct TraceDataObjectV1: Decodable {
661+
let source: Path
662+
let includes: [Path]
663+
}
664+
665+
fileprivate struct TraceDataObjectV2: Decodable {
666+
let source: Path
667+
let includes: [Include]
668+
let imports: [Import]
669+
}
670+
671+
fileprivate typealias TraceFileV1 = [TraceDataObjectV1]
672+
673+
fileprivate struct TraceFileV2: Decodable {
674+
let version: String
675+
let dependencies: [TraceDataObjectV2]
676+
}
677+
678+
case V1(TraceFileV1)
679+
case V2(TraceFileV2)
618680
}

Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public final class ValidateDependenciesTaskAction: TaskAction {
3636
}
3737

3838
do {
39-
var allClangIncludes = Set<Path>()
39+
var allClangIncludes = Set<DependencyValidationInfo.Include>()
4040
var allClangImports = Set<DependencyValidationInfo.Import>()
4141
var allSwiftImports = Set<DependencyValidationInfo.Import>()
4242
var unsupported = false
@@ -100,7 +100,7 @@ public final class ValidateDependenciesTaskAction: TaskAction {
100100
if unsupported {
101101
diagnostics.append(contentsOf: headerContext.makeDiagnostics(includes: nil))
102102
} else {
103-
diagnostics.append(contentsOf: headerContext.makeDiagnostics(includes: Array(allClangIncludes)))
103+
diagnostics.append(contentsOf: headerContext.makeDiagnostics(includes: allClangIncludes.map { $0.path }))
104104
}
105105
}
106106

Tests/SWBBuildSystemTests/DependencyValidationTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ fileprivate struct DependencyValidationTests: CoreBasedTests {
542542

543543
// Expect complaint about undeclared dependency
544544
try await tester.checkBuild(parameters: BuildParameters(configuration: "Debug"), runDestination: .host, persistent: true) { results in
545-
results.checkError(.contains("Missing entries in MODULE_DEPENDENCIES: Foundation (for task"))
545+
results.checkError(.contains("Missing entry in MODULE_DEPENDENCIES: Foundation (for task"))
546546
}
547547

548548
// Declaring dependencies resolves the problem

0 commit comments

Comments
 (0)