Skip to content

Commit

Permalink
[Explicit Module Builds] Treat binary Swift module explicit dependenc…
Browse files Browse the repository at this point in the history
…ies as always up-to-date

The driver does not have the capability to rebuild them, so there is no point to invalidating these dependency nodes. Crucially, if these dependencies are newer than some other dependency between the source module and it, it will still cause invalidation of dependencies which can and must be re-built.

This change fixes the remaining case left by the initial attempt at this in #1674.
  • Loading branch information
artemcm committed Nov 20, 2024
1 parent 68c1b1f commit 8a0f4f2
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,15 @@ internal extension InterModuleDependencyGraph {
return false
}

// We do not verify the binary module itself being out-of-date if we do not have a textual
// interface it was built from, but we can safely treat it as up-to-date, particularly
// because if it is newer than any of the modules they depend on it, they will
// still get invalidated in the check below for whether a module has
// any dependencies newer than it.
if case .swiftPrebuiltExternal(_) = moduleID {
return true
}

// Check if a dependency of this module has a newer output than this module
for dependencyId in checkedModuleInfo.directDependencies ?? [] {
let dependencyInfo = try moduleInfo(of: dependencyId)
Expand Down Expand Up @@ -400,11 +409,6 @@ internal extension InterModuleDependencyGraph {
}
}
case .swiftPrebuiltExternal(_):
// We do not verify the binary module itself being out-of-date if we do not have a textual
// interface it was built from, but we can safely treat it as up-to-date, particularly
// because if it is newer than any of the modules they depend on it, they will
// still get invalidated in the check above for whether a module has
// any dependencies newer than it.
return true;
case .swiftPlaceholder(_):
// TODO: This should never ever happen. Hard error?
Expand Down
1 change: 1 addition & 0 deletions TestInputs/ExplicitModuleBuilds/Swift/G.swiftinterface
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@
import Swift
public func overlayFuncG() { }
#endif
import D
14 changes: 11 additions & 3 deletions Tests/SwiftDriverTests/CachingBuildTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,10 @@ final class CachingBuildTests: XCTestCase {
try checkCachingBuildJob(job: job, moduleId: .clang("G"),
dependencyGraph: dependencyGraph)
}
else if relativeOutputPathFileName.starts(with: "D-") {
try checkCachingBuildJob(job: job, moduleId: .clang("D"),
dependencyGraph: dependencyGraph)
}
else if relativeOutputPathFileName.starts(with: "F-") {
try checkCachingBuildJob(job: job, moduleId: .clang("F"),
dependencyGraph: dependencyGraph)
Expand Down Expand Up @@ -555,6 +559,10 @@ final class CachingBuildTests: XCTestCase {
try checkCachingBuildJob(job: job, moduleId: .clang("C"),
dependencyGraph: dependencyGraph)
}
else if relativeOutputPathFileName.starts(with: "D-") {
try checkCachingBuildJob(job: job, moduleId: .clang("D"),
dependencyGraph: dependencyGraph)
}
else if relativeOutputPathFileName.starts(with: "G-") {
try checkCachingBuildJob(job: job, moduleId: .clang("G"),
dependencyGraph: dependencyGraph)
Expand Down Expand Up @@ -775,11 +783,11 @@ final class CachingBuildTests: XCTestCase {
let expectedNumberOfDependencies: Int
if driver.hostTriple.isMacOSX,
driver.hostTriple.version(for: .macOS) < Triple.Version(11, 0, 0) {
expectedNumberOfDependencies = 12
expectedNumberOfDependencies = 13
} else if driver.targetTriple.isWindows {
expectedNumberOfDependencies = 14
expectedNumberOfDependencies = 15
} else {
expectedNumberOfDependencies = 11
expectedNumberOfDependencies = 12
}

// Dispatch several iterations in parallel
Expand Down
18 changes: 15 additions & 3 deletions Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,10 @@ final class ExplicitModuleBuildTests: XCTestCase {
try checkExplicitModuleBuildJob(job: job, moduleId: .clang("C"),
dependencyGraph: dependencyGraph)
}
else if relativeOutputPathFileName.starts(with: "D-") {
try checkExplicitModuleBuildJob(job: job, moduleId: .clang("D"),
dependencyGraph: dependencyGraph)
}
else if relativeOutputPathFileName.starts(with: "G-") {
try checkExplicitModuleBuildJob(job: job, moduleId: .clang("G"),
dependencyGraph: dependencyGraph)
Expand Down Expand Up @@ -719,6 +723,10 @@ final class ExplicitModuleBuildTests: XCTestCase {
try checkExplicitModuleBuildJob(job: job, moduleId: .clang("C"),
dependencyGraph: dependencyGraph)
}
else if relativeOutputPathFileName.starts(with: "D-") {
try checkExplicitModuleBuildJob(job: job, moduleId: .clang("D"),
dependencyGraph: dependencyGraph)
}
else if relativeOutputPathFileName.starts(with: "G-") {
try checkExplicitModuleBuildJob(job: job, moduleId: .clang("G"),
dependencyGraph: dependencyGraph)
Expand Down Expand Up @@ -846,6 +854,10 @@ final class ExplicitModuleBuildTests: XCTestCase {
try checkExplicitModuleBuildJob(job: job, moduleId: .clang("C"),
dependencyGraph: dependencyGraph)
}
else if relativeOutputPathFileName.starts(with: "D-") {
try checkExplicitModuleBuildJob(job: job, moduleId: .clang("D"),
dependencyGraph: dependencyGraph)
}
else if relativeOutputPathFileName.starts(with: "G-") {
try checkExplicitModuleBuildJob(job: job, moduleId: .clang("G"),
dependencyGraph: dependencyGraph)
Expand Down Expand Up @@ -1736,11 +1748,11 @@ final class ExplicitModuleBuildTests: XCTestCase {
let expectedNumberOfDependencies: Int
if hostTriple.isMacOSX,
hostTriple.version(for: .macOS) < Triple.Version(11, 0, 0) {
expectedNumberOfDependencies = 12
expectedNumberOfDependencies = 13
} else if driver.targetTriple.isWindows {
expectedNumberOfDependencies = 14
expectedNumberOfDependencies = 15
} else {
expectedNumberOfDependencies = 11
expectedNumberOfDependencies = 12
}

// Dispatch several iterations in parallel
Expand Down
10 changes: 7 additions & 3 deletions Tests/SwiftDriverTests/IncrementalCompilationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -533,10 +533,15 @@ extension IncrementalCompilationTests {
try! localFileSystem.removeFileTree(try AbsolutePath(validating: explicitSwiftDependenciesPath.appending(component: "G.swiftinterface").pathString))
try buildInitialState(checkDiagnostics: false, explicitModuleBuild: true)

// Touch one of the inputs to actually trigger the incremental build, so that we can ensure
// no module deps get re-built
// Touch one of the inputs to actually trigger the incremental build
touch(inputPath(basename: "other"))

// Touch the output of a dependency of 'G', to ensure that it is newer than 'G', but 'G' still does not
// get "invalidated",
let nameOfDModule = try XCTUnwrap(modCacheEntries.first { $0.hasPrefix("D") && $0.hasSuffix(".pcm")})
let pathToDModule = explicitModuleCacheDir.appending(component: nameOfDModule)
touch(pathToDModule)

try doABuild(
"Unchanged binary dependency (G)",
checkDiagnostics: true,
Expand All @@ -563,7 +568,6 @@ extension IncrementalCompilationTests {
reading(deps: "other")
schedulingPostCompileJobs
linking

}
}
}
Expand Down

0 comments on commit 8a0f4f2

Please sign in to comment.