Skip to content

Dependencies: support clang modules when parsing dependency validation info #778

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 4 commits into from
Sep 12, 2025
Merged

Dependencies: support clang modules when parsing dependency validation info #778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5349aac
Dependencies: support clang modules when parsing dependency validatio…
sina-mahdavi Sep 5, 2025
a43af46
Dependencies: Use better Regex/JSON decoding for parsing dependency t…
sina-mahdavi Sep 5, 2025
c5f62f1
Dependencies: Handle empty dependency trace file in parsing
sina-mahdavi Sep 5, 2025
6a44f6a
Dependencies: Fix deduplication issue when parsing dependency trace v1
sina-mahdavi Sep 5, 2025
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
12 changes: 11 additions & 1 deletion Sources/SWBCore/Dependencies.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,8 +438,18 @@ public struct DependencyValidationInfo: Hashable, Sendable, Codable {
}
}

public struct Include: Hashable, Sendable, Codable {
public let path: Path
public let includeLocations: [Diagnostic.Location]

public init(path: Path, includeLocations: [Diagnostic.Location]) {
self.path = path
self.includeLocations = includeLocations
}
}

public enum Payload: Hashable, Sendable, Codable {
case clangDependencies(imports: [Import], includes: [Path])
case clangDependencies(imports: [Import], includes: [Include])
case swiftDependencies(imports: [Import])
case unsupported
}
Expand Down
153 changes: 114 additions & 39 deletions Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,6 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
}
}



/// Intended to be called during task dependency setup.
/// If remote caching is enabled along with integrated cache queries, it will request
/// a `ClangCachingMaterializeKeyTaskAction` as task dependency.
Expand Down Expand Up @@ -485,35 +483,51 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
outputDelegate: any TaskOutputDelegate
) throws {
let payload: DependencyValidationInfo.Payload
if let traceFilePath {
let traceFileContent = try fileSystem.read(traceFilePath)

let traceData: Array<TraceData>
// clang will emit an empty file instead of an empty array when there's nothing to trace
if traceFileContent.isEmpty {
traceData = []
} else {
do {
traceData = try JSONDecoder().decode(Array<TraceData>.self, from: Data(traceFileContent))
} catch {
throw StubError.error("Failed to decode json trace at \(traceFilePath.str): \(error)")
}
}

var allFiles = Set<Path>()
traceData.forEach { allFiles.formUnion(Set($0.includes)) }
if let traceFilePath,
let traceData = try parseTraceData(Data(fileSystem.read(traceFilePath))) {

outputDelegate.incrementTaskCounter(.headerDependenciesValidatedTasks)

if isModular {
let (imports, includes) = separateImportsFromIncludes(allFiles)
outputDelegate.incrementTaskCounter(.moduleDependenciesValidatedTasks)
outputDelegate.incrementTaskCounter(.moduleDependenciesScanned, by: imports.count)
}

switch traceData {
case .empty:
payload = .clangDependencies(imports: [], includes: [])
case let .v1(traceDataV1):
// mapping each header path to the set of locations that include it
var allFiles = [Path: Set<SWBUtil.Diagnostic.Location>]();

for entry in traceDataV1 {
entry.includes.forEach { allFiles[$0, default: []].insert(.path(entry.source, fileLocation: nil)) }
}

if isModular {
let (imports, includes) = separateImportsFromIncludes(allFiles)
outputDelegate.incrementTaskCounter(.moduleDependenciesScanned, by: imports.count)
outputDelegate.incrementTaskCounter(.headerDependenciesScanned, by: includes.count)
payload = .clangDependencies(imports: imports, includes: includes)
} else {
let includes = allFiles.map { file, locations in DependencyValidationInfo.Include(path: file, includeLocations: Array(locations)) }
outputDelegate.incrementTaskCounter(.headerDependenciesScanned, by: includes.count)
payload = .clangDependencies(imports: [], includes: includes)
}
case let .v2(traceDataV2):
var allIncludes = [Path: Set<SWBUtil.Diagnostic.Location>]();
var allImports = [String: Set<SWBUtil.Diagnostic.Location>]();

for entry in traceDataV2.dependencies {
entry.includes.forEach { allIncludes[$0.file, default: []].insert(parseTraceSourceLocation($0.location)) }
if isModular {
entry.imports.forEach { allImports[$0.module, default: []].insert(parseTraceSourceLocation($0.location)) }
}
}

let imports = allImports.map { name, locations in DependencyValidationInfo.Import(dependency: ModuleDependency(name: name, accessLevel: .Private, optional: false), importLocations: Array(locations)) }
let includes = allIncludes.map { file, locations in DependencyValidationInfo.Include(path: file, includeLocations: Array(locations)) }
outputDelegate.incrementTaskCounter(.headerDependenciesScanned, by: includes.count)
outputDelegate.incrementTaskCounter(.moduleDependenciesScanned, by: imports.count)
payload = .clangDependencies(imports: imports, includes: includes)
} else {
outputDelegate.incrementTaskCounter(.headerDependenciesScanned, by: allFiles.count)
payload = .clangDependencies(imports: [], includes: Array(allFiles))
}
} else {
outputDelegate.incrementTaskCounter(.headerDependenciesNotValidatedTasks)
Expand All @@ -534,29 +548,28 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
}
}

// Clang's dependency tracing does not currently clearly distinguish modular imports from non-modular includes.
// Until that gets fixed, just guess that if the file is contained in a framework, it comes from a module with
// Clang's dependency tracing V1 did not clearly distinguish modular imports from non-modular includes.
// To keep supporting those trace files, just guess that if the file is contained in a framework, it comes from a module with
// the same name. That is obviously not going to be reliable but it unblocks us from continuing experiments with
// dependency specifications.
private static func separateImportsFromIncludes(_ files: Set<Path>) -> ([DependencyValidationInfo.Import], [Path]) {
private static func separateImportsFromIncludes(_ files: [Path: Set<SWBUtil.Diagnostic.Location>]) -> ([DependencyValidationInfo.Import], [DependencyValidationInfo.Include]) {
func findFrameworkName(_ file: Path) -> String? {
if file.fileExtension == "framework" {
return file.basenameWithoutSuffix
}
return file.dirname.isEmpty || file.dirname.isRoot ? nil : findFrameworkName(file.dirname)
}
var moduleNames: [String] = []
var includeFiles: [Path] = []
for file in files {
var moduleImportsByName = [String: Set<SWBUtil.Diagnostic.Location>]()
var headerIncludes: [DependencyValidationInfo.Include] = []
for (file, includeLocations) in files {
if let frameworkName = findFrameworkName(file) {
moduleNames.append(frameworkName)
moduleImportsByName[frameworkName, default: []].formUnion(includeLocations)
} else {
includeFiles.append(file)
headerIncludes.append(DependencyValidationInfo.Include(path: file, includeLocations: Array(includeLocations)))
}
}
let moduleDependencies = moduleNames.map { ModuleDependency(name: $0, accessLevel: .Private, optional: false) }
let moduleImports = moduleDependencies.map { DependencyValidationInfo.Import(dependency: $0, importLocations: []) }
return (moduleImports, includeFiles)
let moduleImports = moduleImportsByName.map { name, locations in DependencyValidationInfo.Import(dependency: ModuleDependency(name: name, accessLevel: .Private, optional: false), importLocations: Array(locations)) }
return (moduleImports, headerIncludes)
}
}

Expand Down Expand Up @@ -623,9 +636,71 @@ public final class ClangNonModularCompileTaskAction: TaskAction {
}
}

fileprivate func parseTraceData(_ data: Data) throws -> TraceData? {
// clang will emit an empty file instead of an empty array when there's nothing to trace
if data.isEmpty {
return .empty
}

let jsonObject = try PropertyList.fromJSONData(data)
if let version = jsonObject.dictValue?["version"]?.stringValue {
if version == "2.0.0" {
return .v2(try JSONDecoder().decode(TraceData.TraceFileV2.self, from: data))
}
return nil
} else {
// The initial unversioned format (v1) of the trace file generated from clang
// is a JSON array so deserializing it as a dictionary will fail.
return .v1(try JSONDecoder().decode(TraceData.TraceFileV1.self, from: data))
}
}

fileprivate func parseTraceSourceLocation(_ locationStr: String) -> SWBUtil.Diagnostic.Location {
guard let match = locationStr.wholeMatch(of: #/(?<filename>.+):(?<line>\d+):(?<column>\d+)/#) else {
return .unknown
}
let filename = Path(match.filename)
let line = Int(match.line)
let column = Int(match.column)
if let line {
return .path(filename, fileLocation: .textual(line: line, column: column))
}
return .unknown
}

// Results from tracing header includes with "direct-per-file" filtering.
// This is used to validate dependencies.
fileprivate struct TraceData: Decodable {
let source: Path
let includes: [Path]
fileprivate enum TraceData: Decodable {
fileprivate struct Include: Decodable {
let location: String
let file: Path
}

fileprivate struct Import: Decodable {
let location: String
let module: String
let file: Path
}

fileprivate struct TraceDataObjectV1: Decodable {
let source: Path
let includes: [Path]
}

fileprivate struct TraceDataObjectV2: Decodable {
let source: Path
let includes: [Include]
let imports: [Import]
}

fileprivate typealias TraceFileV1 = [TraceDataObjectV1]

fileprivate struct TraceFileV2: Decodable {
let version: String
let dependencies: [TraceDataObjectV2]
}

case empty
case v1(TraceFileV1)
case v2(TraceFileV2)
}
4 changes: 2 additions & 2 deletions Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public final class ValidateDependenciesTaskAction: TaskAction {
}

do {
var allClangIncludes = Set<Path>()
var allClangIncludes = Set<DependencyValidationInfo.Include>()
var allClangImports = Set<DependencyValidationInfo.Import>()
var allSwiftImports = Set<DependencyValidationInfo.Import>()
var unsupported = false
Expand Down Expand Up @@ -116,7 +116,7 @@ public final class ValidateDependenciesTaskAction: TaskAction {
if unsupported {
diagnostics.append(contentsOf: [headerContext.makeUnsupportedToolchainDiagnostic()])
} else {
let (missingDeps, unusedDeps) = headerContext.computeMissingAndUnusedDependencies(includes: Array(allClangIncludes))
let (missingDeps, unusedDeps) = headerContext.computeMissingAndUnusedDependencies(includes: allClangIncludes.map { $0.path })
outputDelegate.incrementCounter(.headerDependenciesMissing, by: missingDeps.count)
outputDelegate.incrementCounter(.headerDependenciesUnused, by: unusedDeps.count)

Expand Down
2 changes: 1 addition & 1 deletion Tests/SWBBuildSystemTests/DependencyValidationTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ fileprivate struct DependencyValidationTests: CoreBasedTests {

// Expect complaint about undeclared dependency
try await tester.checkBuild(parameters: BuildParameters(configuration: "Debug"), runDestination: .host, persistent: true) { results in
results.checkError(.contains("Missing entries in MODULE_DEPENDENCIES: Foundation (for task"))
results.checkError(.contains("Missing entry in MODULE_DEPENDENCIES: Foundation (for task"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm why would this change? I thought the top level diag was always pluralized.

Copy link
Contributor Author

@sina-mahdavi sina-mahdavi Sep 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The test fails for me locally with "entries", I think it's because the "(for task" substring is at the end of the diagnostic which will have the singular messages. So the whole diagnostic is like:

Missing entries in MODULE_DEPENDENCIES: Foundation Foundation
/private/var/folders/5h/9y9ls7ms0fs8gdmlfwklkwpm0000gn/T/swbuild.tmp.x8Tuqtfi/Data.noindex/Test/aProject/Sources/CoreFoo.m: Missing entry in MODULE_DEPENDENCIES: Foundation
/private/var/folders/5h/9y9ls7ms0fs8gdmlfwklkwpm0000gn/T/swbuild.tmp.x8Tuqtfi/Data.noindex/Test/aProject/Sources/CoreBar.m: Missing entry in MODULE_DEPENDENCIES: Foundation (for task: [\"ValidateDependencies\"])

If you want I could change this to look for Missing entries in MODULE_DEPENDENCIES: Foundation Foundation\n?

Copy link
Contributor

Choose a reason for hiding this comment

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

The missing entries are supposed to be de-duplicated (which is why the test checks for "(for task" at the end). It sounds like that's not working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I think there's an issue with the deduplication in my implementation of parsing v1 traces that I need to fix. However, even after fixing that the diagnostic looks like this:

Missing entries in MODULE_DEPENDENCIES: Foundation
Missing entry in MODULE_DEPENDENCIES: Foundation (for task: [\"ValidateDependencies\"])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I think my last commit should fix the deduplication issue, the first line of the diagnostic has only one mention of Foundation. However, Foundation (for task is still only in the last line which has singular entry.

}

// Declaring dependencies resolves the problem
Expand Down
Loading