[Explicit Module Builds] Treat binary Swift module explicit dependencies 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 swiftlang#1674.

Author: artemcm

Sources/SwiftDriver/ExplicitModuleBuilds/InterModuleDependencies/CommonDependencyOperations.swift

@@ -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)
@@ -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?

TestInputs/ExplicitModuleBuilds/Swift/G.swiftinterface

@@ -5,3 +5,4 @@
 import Swift
 public func overlayFuncG() { }
 #endif
+import D

Tests/SwiftDriverTests/CachingBuildTests.swift

@@ -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)
@@ -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)
@@ -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

Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift

@@ -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)
@@ -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)
@@ -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)
@@ -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

Tests/SwiftDriverTests/IncrementalCompilationTests.swift

@@ -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,
@@ -563,7 +568,6 @@ extension IncrementalCompilationTests {
       reading(deps: "other")
       schedulingPostCompileJobs
       linking
-
     }
   }
 }