Skip to content

Commit 9871d5a

Browse files
committed
Handle cross-language duplications for missing dependencies
If a missing C dependency is also a dependency of the Swift code in a module, we won't report the dependency as missing from the C context.
1 parent f6d3d4f commit 9871d5a

File tree

4 files changed

+106
-11
lines changed

4 files changed

+106
-11
lines changed

Sources/SWBCore/Dependencies.swift

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,18 +80,15 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable {
8080
self.init(validate: validate, moduleDependencies: settings.moduleDependencies, fixItContext: fixItContext)
8181
}
8282

83-
/// Make diagnostics for missing module dependencies from Clang imports.
83+
/// Compute missing module dependencies from Clang imports.
8484
///
8585
/// The compiler tracing information does not provide the import locations or whether they are public imports
8686
/// (which depends on whether the import is in an installed header file).
8787
/// If `files` is nil, the current toolchain does support the feature to trace imports.
88-
public func makeDiagnostics(files: [Path]?) -> [Diagnostic] {
88+
public func computeMissingDependencies(files: [Path]?) -> [ModuleDependency]? {
8989
guard validate != .no else { return [] }
9090
guard let files else {
91-
return [Diagnostic(
92-
behavior: .warning,
93-
location: .unknown,
94-
data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_MODULE_DEPENDENCIES.name)"))]
91+
return nil
9592
}
9693

9794
// The following is a provisional/incomplete mechanism for resolving a module dependency from a file path.
@@ -111,14 +108,26 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable {
111108
ModuleDependency(name: $0, accessLevel: .Private)
112109
})
113110

114-
guard !missingDeps.isEmpty else { return [] }
111+
return Array(missingDeps)
112+
}
113+
114+
/// Make diagnostics for missing module dependencies from Clang imports.
115+
public func makeDiagnostics(missingDependencies: [ModuleDependency]?) -> [Diagnostic] {
116+
guard let missingDependencies else {
117+
return [Diagnostic(
118+
behavior: .warning,
119+
location: .unknown,
120+
data: DiagnosticData("The current toolchain does not support \(BuiltinMacros.VALIDATE_MODULE_DEPENDENCIES.name)"))]
121+
}
122+
123+
guard !missingDependencies.isEmpty else { return [] }
115124

116125
let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning
117126

118-
let fixIt = fixItContext?.makeFixIt(newModules: Array(missingDeps))
127+
let fixIt = fixItContext?.makeFixIt(newModules: Array(missingDependencies))
119128
let fixIts = fixIt.map { [$0] } ?? []
120129

121-
let message = "Missing entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(missingDeps.map { $0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))"
130+
let message = "Missing entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(missingDependencies.map { $0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))"
122131

123132
let location: Diagnostic.Location = fixIt.map {
124133
Diagnostic.Location.path($0.sourceRange.path, line: $0.sourceRange.endLine, column: $0.sourceRange.endColumn)

Sources/SWBTaskExecution/TaskActions/ValidateDependenciesTaskAction.swift

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,15 @@ public final class ValidateDependenciesTaskAction: TaskAction {
6161
var diagnostics: [Diagnostic] = []
6262

6363
if unsupported {
64-
diagnostics.append(contentsOf: context.makeDiagnostics(files: nil))
64+
diagnostics.append(contentsOf: context.makeDiagnostics(missingDependencies: nil))
6565
} else {
66-
diagnostics.append(contentsOf: context.makeDiagnostics(files: allFiles.map { Path($0) }))
66+
// Filter missing C dependencies by known Swift dependencies to avoid duplicate diagnostics between the two.
67+
let swiftImports = allImports.map { $0.dependency.name }
68+
let missingDependencies = context.computeMissingDependencies(files: allFiles.map { Path($0) })?.filter {
69+
!swiftImports.contains($0.name)
70+
}
71+
72+
diagnostics.append(contentsOf: context.makeDiagnostics(missingDependencies: missingDependencies))
6773
diagnostics.append(contentsOf: context.makeDiagnostics(imports: allImports.map { ($0.dependency, $0.importLocations) }))
6874
}
6975

Sources/SWBTestSupport/CoreBasedTests.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,14 @@ extension CoreBasedTests {
155155
}
156156
}
157157

158+
/// The path to clang in the default toolchain.
159+
public var defaultClangPath: Path {
160+
get async throws {
161+
let clangInfo = try await clangInfo
162+
return clangInfo.toolPath
163+
}
164+
}
165+
158166
/// The path to the libClang.dylib compiler in the default toolchain.
159167
package var libClangPath: Path {
160168
get async throws {

Tests/SWBBuildSystemTests/DependencyValidationTests.swift

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,4 +529,76 @@ fileprivate struct DependencyValidationTests: CoreBasedTests {
529529
}
530530
}
531531

532+
@Test(.requireSDKs(.host), .requireClangFeatures(.printHeadersDirectPerFile), .skipHostOS(.windows, "toolchain too old"), .skipHostOS(.linux, "toolchain too old"))
533+
func validateModuleDependenciesMixedSource() async throws {
534+
try await withTemporaryDirectory { tmpDir async throws -> Void in
535+
let testWorkspace = try await TestWorkspace(
536+
"Test",
537+
sourceRoot: tmpDir.join("Test"),
538+
projects: [
539+
TestProject(
540+
"aProject",
541+
groupTree: TestGroup(
542+
"Sources", path: "Sources",
543+
children: [
544+
TestFile("CoreFoo.m"),
545+
TestFile("Swift.swift"),
546+
]),
547+
buildConfigurations: [
548+
TestBuildConfiguration(
549+
"Debug",
550+
buildSettings: [
551+
"PRODUCT_NAME": "$(TARGET_NAME)",
552+
"CLANG_ENABLE_MODULES": "YES",
553+
"CLANG_ENABLE_EXPLICIT_MODULES": "YES",
554+
"SWIFT_ENABLE_EXPLICIT_MODULES": "YES",
555+
"SWIFT_UPCOMING_FEATURE_INTERNAL_IMPORTS_BY_DEFAULT": "YES",
556+
"SWIFT_VERSION": swiftVersion,
557+
"GENERATE_INFOPLIST_FILE": "YES",
558+
"VALIDATE_MODULE_DEPENDENCIES": "YES_ERROR",
559+
"SDKROOT": "$(HOST_PLATFORM)",
560+
"SUPPORTED_PLATFORMS": "$(HOST_PLATFORM)",
561+
"DSTROOT": tmpDir.join("dstroot").str,
562+
563+
// Temporarily override to use the latest toolchain in CI because we depend on swift and swift-driver changes which aren't in the baseline tools yet
564+
"TOOLCHAINS": "swift",
565+
// We still want to use the default clang since that is used to gate the test
566+
"CC": defaultClangPath.str,
567+
]
568+
)
569+
],
570+
targets: [
571+
TestStandardTarget(
572+
"CoreFoo", type: .framework,
573+
buildPhases: [
574+
TestSourcesBuildPhase(["CoreFoo.m", "Swift.swift"]),
575+
TestFrameworksBuildPhase()
576+
])
577+
])
578+
]
579+
)
580+
581+
let tester = try await BuildOperationTester(getCore(), testWorkspace, simulated: false)
582+
let SRCROOT = testWorkspace.sourceRoot.join("aProject")
583+
584+
try await tester.fs.writeFileContents(SRCROOT.join("Sources/Swift.swift")) { stream in
585+
stream <<< """
586+
import Foundation
587+
"""
588+
}
589+
590+
try await tester.fs.writeFileContents(SRCROOT.join("Sources/CoreFoo.m")) { contents in
591+
contents <<< """
592+
#include <Foundation/Foundation.h>
593+
594+
void f(void) { };
595+
"""
596+
}
597+
598+
try await tester.checkBuild(parameters: BuildParameters(configuration: "Debug"), runDestination: .host, persistent: true) { results in
599+
results.checkError(.contains("Missing entries in MODULE_DEPENDENCIES: Foundation"))
600+
}
601+
}
602+
}
603+
532604
}

0 commit comments

Comments
 (0)