Validate MODULE_DEPENDENCIES using tracing from Clang

This currently only checks Clang compilations with modules because there
were some issues with introducing a new task action for non-modular Clang
compilations.

rdar://150313957

Author: bob-wilson

Sources/SWBCore/Dependencies.swift

@@ -80,7 +80,60 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable {
         self.init(validate: validate, moduleDependencies: settings.moduleDependencies, fixItContext: fixItContext)
     }
 
-    /// Nil `imports` means the current toolchain doesn't have the features to gather imports. This is temporarily required to support running against older toolchains.
+    /// Make diagnostics for missing module dependencies from Clang imports.
+    ///
+    /// The compiler tracing information does not provide the import locations or whether they are public imports
+    /// (which depends on whether the import is in an installed header file).
+    /// If `files` is nil, the current toolchain does support the feature to trace imports.
+    public func makeDiagnostics(files: [Path]?) -> [Diagnostic] {
+        guard validate != .no else { return [] }
+        guard let files else {
+            return [Diagnostic(
+                behavior: .warning,
+                location: .unknown,
+                data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_MODULE_DEPENDENCIES.name)"))]
+        }
+
+        // The following is a provisional/incomplete mechanism for resolving a module dependency from a file path.
+        // For now, just grab the framework name and assume there is a module with the same name.
+        func findFrameworkName(_ file: Path) -> String? {
+            if file.fileExtension == "framework" {
+                return file.basenameWithoutSuffix
+            }
+            return file.dirname.isEmpty || file.dirname.isRoot ? nil : findFrameworkName(file.dirname)
+        }
+
+        let moduleDependencyNames = moduleDependencies.map { $0.name }
+        let fileNames = files.compactMap { findFrameworkName($0) }
+        let missingDeps = fileNames.filter {
+            return !moduleDependencyNames.contains($0)
+        }.map {
+            ModuleDependency(name: $0, accessLevel: .Private)
+        }
+
+        guard !missingDeps.isEmpty else { return [] }
+
+        let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning
+
+        let fixIt = fixItContext?.makeFixIt(newModules: missingDeps)
+        let fixIts = fixIt.map { [$0] } ?? []
+
+        let message = "Missing entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(missingDeps.map { $0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))"
+
+        let location: Diagnostic.Location = fixIt.map {
+            Diagnostic.Location.path($0.sourceRange.path, line: $0.sourceRange.endLine, column: $0.sourceRange.endColumn)
+        } ?? Diagnostic.Location.buildSetting(BuiltinMacros.MODULE_DEPENDENCIES)
+
+        return [Diagnostic(
+            behavior: behavior,
+            location: location,
+            data: DiagnosticData(message),
+            fixIts: fixIts)]
+    }
+
+    /// Make diagnostics for missing module dependencies from Swift imports.
+    ///
+    /// If `imports` is nil, the current toolchain does not support the features to gather imports.
     public func makeDiagnostics(imports: [(ModuleDependency, importLocations: [Diagnostic.Location])]?) -> [Diagnostic] {
         guard validate != .no else { return [] }
         guard let imports else {

Sources/SWBCore/SpecImplementations/Tools/CCompiler.swift

@@ -432,7 +432,10 @@ public struct ClangTaskPayload: ClangModuleVerifierPayloadType, DependencyInfoEd
 
     public let fileNameMapPath: Path?
 
-    fileprivate init(serializedDiagnosticsPath: Path?, indexingPayload: ClangIndexingPayload?, explicitModulesPayload: ClangExplicitModulesPayload? = nil, outputObjectFilePath: Path? = nil, fileNameMapPath: Path? = nil, developerPathString: String? = nil) {
+    public let moduleDependenciesContext: ModuleDependenciesContext?
+    public let traceFilePath: Path?
+
+    fileprivate init(serializedDiagnosticsPath: Path?, indexingPayload: ClangIndexingPayload?, explicitModulesPayload: ClangExplicitModulesPayload? = nil, outputObjectFilePath: Path? = nil, fileNameMapPath: Path? = nil, developerPathString: String? = nil, moduleDependenciesContext: ModuleDependenciesContext? = nil, traceFilePath: Path? = nil) {
         if let developerPathString, explicitModulesPayload == nil {
             self.dependencyInfoEditPayload = .init(removablePaths: [], removableBasenames: [], developerPath: Path(developerPathString))
         } else {
@@ -443,27 +446,33 @@ public struct ClangTaskPayload: ClangModuleVerifierPayloadType, DependencyInfoEd
         self.explicitModulesPayload = explicitModulesPayload
         self.outputObjectFilePath = outputObjectFilePath
         self.fileNameMapPath = fileNameMapPath
+        self.moduleDependenciesContext = moduleDependenciesContext
+        self.traceFilePath = traceFilePath
     }
 
     public func serialize<T: Serializer>(to serializer: T) {
-        serializer.serializeAggregate(6) {
+        serializer.serializeAggregate(8) {
             serializer.serialize(serializedDiagnosticsPath)
             serializer.serialize(indexingPayload)
             serializer.serialize(explicitModulesPayload)
             serializer.serialize(outputObjectFilePath)
             serializer.serialize(fileNameMapPath)
             serializer.serialize(dependencyInfoEditPayload)
+            serializer.serialize(moduleDependenciesContext)
+            serializer.serialize(traceFilePath)
         }
     }
 
     public init(from deserializer: any Deserializer) throws {
-        try deserializer.beginAggregate(6)
+        try deserializer.beginAggregate(8)
         self.serializedDiagnosticsPath = try deserializer.deserialize()
         self.indexingPayload = try deserializer.deserialize()
         self.explicitModulesPayload = try deserializer.deserialize()
         self.outputObjectFilePath = try deserializer.deserialize()
         self.fileNameMapPath = try deserializer.deserialize()
         self.dependencyInfoEditPayload = try deserializer.deserialize()
+        self.moduleDependenciesContext = try deserializer.deserialize()
+        self.traceFilePath = try deserializer.deserialize()
     }
 }
 
@@ -1156,6 +1165,22 @@ public class ClangCompilerSpec : CompilerSpec, SpecIdentifierType, GCCCompatible
             dependencyData = nil
         }
 
+        let moduleDependenciesContext = cbc.producer.moduleDependenciesContext
+        let traceFilePath: Path?
+        if clangInfo?.hasFeature("print-headers-direct-per-file") ?? false,
+            (moduleDependenciesContext?.validate ?? .defaultValue) != .no {
+            let file = Path(outputNode.path.str + ".trace.json")
+            commandLine += [
+                "-Xclang", "-header-include-file",
+                "-Xclang", file.str,
+                "-Xclang", "-header-include-filtering=direct-per-file",
+                "-Xclang", "-header-include-format=json"
+            ]
+            traceFilePath = file
+        } else {
+            traceFilePath = nil
+        }
+
         // Add the diagnostics serialization flag.  We currently place the diagnostics file right next to the output object file.
         let diagFilePath: Path?
         if let serializedDiagnosticsOptions = self.serializedDiagnosticsOptions(scope: cbc.scope, outputPath: outputNode.path) {
@@ -1266,7 +1291,9 @@ public class ClangCompilerSpec : CompilerSpec, SpecIdentifierType, GCCCompatible
             explicitModulesPayload: explicitModulesPayload,
             outputObjectFilePath: shouldGenerateRemarks ? outputNode.path : nil,
             fileNameMapPath: verifierPayload?.fileNameMapPath,
-            developerPathString: recordSystemHeaderDepsOutsideSysroot ? cbc.scope.evaluate(BuiltinMacros.DEVELOPER_DIR).str : nil
+            developerPathString: recordSystemHeaderDepsOutsideSysroot ? cbc.scope.evaluate(BuiltinMacros.DEVELOPER_DIR).str : nil,
+            moduleDependenciesContext: moduleDependenciesContext,
+            traceFilePath: traceFilePath
         )
 
         var inputNodes: [any PlannedNode] = inputDeps.map { delegate.createNode($0) }
@@ -1316,6 +1343,15 @@ public class ClangCompilerSpec : CompilerSpec, SpecIdentifierType, GCCCompatible
             extraInputs = []
         }
 
+        if let moduleDependenciesContext {
+            guard let jsonData = try? JSONEncoder(outputFormatting: [.prettyPrinted, .sortedKeys, .withoutEscapingSlashes]).encode(moduleDependenciesContext),
+               let signature = String(data: jsonData, encoding: .utf8) else {
+                delegate.error("failed to serialize 'MODULE_DEPENDENCIES' context information")
+                return
+            }
+            additionalSignatureData += "|\(signature)"
+        }
+
         // Finally, create the task.
         delegate.createTask(type: self, dependencyData: dependencyData, payload: payload, ruleInfo: ruleInfo, additionalSignatureData: additionalSignatureData, commandLine: commandLine, additionalOutput: additionalOutput, environment: environmentBindings, workingDirectory: compilerWorkingDirectory(cbc), inputs: inputNodes + extraInputs, outputs: [outputNode], action: action ?? delegate.taskActionCreationDelegate.createDeferredExecutionTaskActionIfRequested(userPreferences: cbc.producer.userPreferences), execDescription: resolveExecutionDescription(cbc, delegate), enableSandboxing: enableSandboxing, additionalTaskOrderingOptions: [.compilationForIndexableSourceFile], usesExecutionInputs: usesExecutionInputs, showEnvironment: true, priority: .preferred)
 

Sources/SWBCore/ToolInfo/ClangToolInfo.swift

@@ -38,6 +38,7 @@ public struct DiscoveredClangToolSpecInfo: DiscoveredCommandLineToolSpecInfo {
         case wSystemHeadersInModule = "Wsystem-headers-in-module"
         case extractAPISupportsCPlusPlus = "extract-api-supports-cpp"
         case deploymentTargetEnvironmentVariables = "deployment-target-environment-variables"
+        case printHeadersDirectPerFile = "print-headers-direct-per-file"
     }
     public var toolFeatures: ToolFeatures<FeatureFlag>
     public func hasFeature(_ feature: String) -> Bool {
@@ -46,6 +47,10 @@ public struct DiscoveredClangToolSpecInfo: DiscoveredCommandLineToolSpecInfo {
         if feature == FeatureFlag.extractAPISupportsCPlusPlus.rawValue {
             return clangVersion > Version(17)
         }
+        // FIXME: Remove once the feature flag is added to clang.
+        if feature == FeatureFlag.printHeadersDirectPerFile.rawValue, let clangVersion {
+            return clangVersion >= Version(1700, 3, 10, 2)
+        }
         return toolFeatures.has(feature)
     }
 

Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift

@@ -248,6 +248,23 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
                 casDBs = nil
             }
 
+            // Check if verifying dependencies from trace data is enabled.
+            let traceFilePath: Path?
+            let moduleDependenciesContext: ModuleDependenciesContext?
+            if let payload = task.payload as? ClangTaskPayload {
+                traceFilePath = payload.traceFilePath
+                moduleDependenciesContext = payload.moduleDependenciesContext
+            } else {
+                traceFilePath = nil
+                moduleDependenciesContext = nil
+            }
+            if let traceFilePath {
+                // Remove the trace output file if it already exists.
+                if executionDelegate.fs.exists(traceFilePath) {
+                    try executionDelegate.fs.remove(traceFilePath)
+                }
+            }
+
             var lastResult: CommandResult? = nil
             for command in dependencyInfo.commands {
                 if let casDBs {
@@ -304,6 +321,30 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
                     return lastResult ?? .failed
                 }
             }
+
+            if let moduleDependenciesContext, lastResult == .succeeded {
+                // Verify the dependencies from the trace data.
+                let files: [Path]?
+                if let traceFilePath {
+                    let fs = executionDelegate.fs
+                    let traceData = try JSONDecoder().decode(Array<TraceData>.self, from: Data(fs.read(traceFilePath)))
+
+                    var allFiles = Set<Path>()
+                    traceData.forEach { allFiles.formUnion(Set($0.includes)) }
+                    files = Array(allFiles)
+                } else {
+                    files = nil
+                }
+                let diagnostics = moduleDependenciesContext.makeDiagnostics(files: files)
+                for diagnostic in diagnostics {
+                    outputDelegate.emit(diagnostic)
+                }
+
+                if diagnostics.contains(where: { $0.behavior == .error }) {
+                    return .failed
+                }
+            }
+
             return lastResult ?? .failed
         } catch {
             outputDelegate.emitError("\(error)")
@@ -431,3 +472,10 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
         )
     }
 }
+
+// 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]
+}

Sources/SWBTestSupport/CoreBasedTests.swift

@@ -147,6 +147,10 @@ extension CoreBasedTests {
             if clangInfo.clangVersion > Version(17) {
                 realToolFeatures.insert(.extractAPISupportsCPlusPlus)
             }
+            if let clangVersion = clangInfo.clangVersion, clangVersion >= Version(1700, 3, 10, 2) {
+                realToolFeatures.insert(.printHeadersDirectPerFile)
+            }
+
             return ToolFeatures(realToolFeatures)
         }
     }

Tests/SWBBuildSystemTests/DependencyValidationTests.swift

@@ -328,7 +328,7 @@ fileprivate struct DependencyValidationTests: CoreBasedTests {
     }
 
     @Test(.requireSDKs(.host))
-    func validateModuleDependencies() async throws {
+    func validateModuleDependenciesSwift() async throws {
         try await withTemporaryDirectory { tmpDir in
             let testWorkspace = try await TestWorkspace(
                 "Test",
@@ -458,4 +458,71 @@ fileprivate struct DependencyValidationTests: CoreBasedTests {
             }
         }
     }
+
+    @Test(.requireSDKs(.host), .requireClangFeatures(.printHeadersDirectPerFile))
+    func validateModuleDependenciesClang() async throws {
+        try await withTemporaryDirectory { tmpDir async throws -> Void in
+            let testWorkspace = TestWorkspace(
+                "Test",
+                sourceRoot: tmpDir.join("Test"),
+                projects: [
+                    TestProject(
+                        "aProject",
+                        groupTree: TestGroup(
+                            "Sources", path: "Sources",
+                            children: [
+                                TestFile("CoreFoo.m")
+                            ]),
+                        buildConfigurations: [
+                            TestBuildConfiguration(
+                                "Debug",
+                                buildSettings: [
+                                    "PRODUCT_NAME": "$(TARGET_NAME)",
+                                    "CLANG_ENABLE_MODULES": "YES",
+                                    "CLANG_ENABLE_EXPLICIT_MODULES": "YES",
+                                    "GENERATE_INFOPLIST_FILE": "YES",
+                                    "MODULE_DEPENDENCIES": "Foundation",
+                                    "VALIDATE_MODULE_DEPENDENCIES": "YES_ERROR",
+                                    "SDKROOT": "$(HOST_PLATFORM)",
+                                    "SUPPORTED_PLATFORMS": "$(HOST_PLATFORM)",
+                                    "DSTROOT": tmpDir.join("dstroot").str,
+                                ]
+                            )
+                        ],
+                        targets: [
+                            TestStandardTarget(
+                                "CoreFoo", type: .framework,
+                                buildPhases: [
+                                    TestSourcesBuildPhase(["CoreFoo.m"]),
+                                    TestFrameworksBuildPhase()
+                                ])
+                        ])
+                ]
+            )
+
+            let tester = try await BuildOperationTester(getCore(), testWorkspace, simulated: false)
+            let SRCROOT = testWorkspace.sourceRoot.join("aProject")
+
+            // Write the source files.
+            try await tester.fs.writeFileContents(SRCROOT.join("Sources/CoreFoo.m")) { contents in
+                contents <<< """
+                    #include <Foundation/Foundation.h>
+                    #include <Accelerate/Accelerate.h>
+
+                    void f0(void) { };
+                """
+            }
+
+            // 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: Accelerate"))
+            }
+
+            // Declaring dependencies resolves the problem
+            try await tester.checkBuild(parameters: BuildParameters(configuration: "Debug", overrides: ["MODULE_DEPENDENCIES": "Foundation Accelerate"]), runDestination: .host, persistent: true) { results in
+                results.checkNoErrors()
+            }
+        }
+    }
+
 }