[Explicit Module Builds][Incremental Builds] Unify logic used to check if a prior inter-module dep graph is up-to-date with a check to decide which modules to re-build

Using the same 'computeInvalidatedModuleDependencies' routine, which is more thorough and checks module inputs to each dependency as well.

Author: artemcm

Sources/SwiftDriver/ExplicitModuleBuilds/InterModuleDependencies/CommonDependencyOperations.swift

@@ -260,6 +260,7 @@ internal extension InterModuleDependencyGraph {
   /// between it and the root (source module being built by this driver
   /// instance) must also be re-built.
   func computeInvalidatedModuleDependencies(fileSystem: FileSystem,
+                                            forRebuild: Bool,
                                             reporter: IncrementalCompilationState.Reporter? = nil)
   throws -> Set<ModuleDependencyId> {
     let mainModuleInfo = mainModule
@@ -270,10 +271,13 @@ internal extension InterModuleDependencyGraph {
     for dependencyId in mainModuleInfo.directDependencies ?? [] {
       try outOfDateModuleScan(from: dependencyId, visited: &visited,
                               modulesRequiringRebuild: &modulesRequiringRebuild,
-                              fileSystem: fileSystem, reporter: reporter)
+                              fileSystem: fileSystem, forRebuild: forRebuild,
+                              reporter: reporter)
     }
 
-    reporter?.reportExplicitDependencyReBuildSet(Array(modulesRequiringRebuild))
+    if forRebuild {
+      reporter?.reportExplicitDependencyReBuildSet(Array(modulesRequiringRebuild))
+    }
     return modulesRequiringRebuild
   }
 
@@ -284,46 +288,124 @@ internal extension InterModuleDependencyGraph {
                            visited: inout Set<ModuleDependencyId>,
                            modulesRequiringRebuild: inout Set<ModuleDependencyId>,
                            fileSystem: FileSystem,
+                           forRebuild: Bool,
                            reporter: IncrementalCompilationState.Reporter? = nil) throws {
+    let reportOutOfDate = { (name: String, reason: String)  in
+      if forRebuild {
+        reporter?.reportExplicitDependencyWillBeReBuilt(sourceModuleId.moduleNameForDiagnostic, reason: reason)
+      } else {
+        reporter?.reportPriorExplicitDependencyStale(sourceModuleId.moduleNameForDiagnostic, reason: reason)
+      }
+    }
+
     let sourceModuleInfo = try moduleInfo(of: sourceModuleId)
     // Visit the module's dependencies
     var hasOutOfDateModuleDependency = false
-    var mostRecentlyUpdatedDependencyOutput: TimePoint = .zero
     for dependencyId in sourceModuleInfo.directDependencies ?? [] {
       // If we have not already visited this module, recurse.
       if !visited.contains(dependencyId) {
         try outOfDateModuleScan(from: dependencyId, visited: &visited,
                                 modulesRequiringRebuild: &modulesRequiringRebuild,
-                                fileSystem: fileSystem, reporter: reporter)
+                                fileSystem: fileSystem, forRebuild: forRebuild,
+                                reporter: reporter)
       }
       // Even if we're not revisiting a dependency, we must check if it's already known to be out of date.
       hasOutOfDateModuleDependency = hasOutOfDateModuleDependency || modulesRequiringRebuild.contains(dependencyId)
-
-      // Keep track of dependencies' output file time stamp to determine if it is newer than the current module.
-      if let depOutputTimeStamp = try? fileSystem.lastModificationTime(for: VirtualPath.lookup(moduleInfo(of: dependencyId).modulePath.path)),
-         depOutputTimeStamp > mostRecentlyUpdatedDependencyOutput {
-        mostRecentlyUpdatedDependencyOutput = depOutputTimeStamp
-      }
     }
 
     if hasOutOfDateModuleDependency {
-      reporter?.reportExplicitDependencyWillBeReBuilt(sourceModuleId.moduleNameForDiagnostic, reason: "Invalidated by downstream dependency")
-      modulesRequiringRebuild.insert(sourceModuleId)
-    } else if try !IncrementalCompilationState.IncrementalDependencyAndInputSetup.verifyModuleDependencyUpToDate(moduleID: sourceModuleId, moduleInfo: sourceModuleInfo,
-                                                                                                                 fileSystem: fileSystem, reporter: reporter) {
-      reporter?.reportExplicitDependencyWillBeReBuilt(sourceModuleId.moduleNameForDiagnostic, reason: "Out-of-date")
+      reportOutOfDate(sourceModuleId.moduleNameForDiagnostic, "Invalidated by downstream dependency")
       modulesRequiringRebuild.insert(sourceModuleId)
-    } else if let outputModTime = try? fileSystem.lastModificationTime(for: VirtualPath.lookup(sourceModuleInfo.modulePath.path)),
-              outputModTime < mostRecentlyUpdatedDependencyOutput {
-      // If a prior variant of this module dependnecy exists, and is older than any of its direct or transitive
-      // module dependency outputs, it must also be re-built.
-      reporter?.reportExplicitDependencyWillBeReBuilt(sourceModuleId.moduleNameForDiagnostic, reason: "Has newer module dependency inputs")
+    } else if try !verifyModuleDependencyUpToDate(moduleID: sourceModuleId, fileSystem: fileSystem, reporter: reporter) {
+      reportOutOfDate(sourceModuleId.moduleNameForDiagnostic, "Out-of-date")
       modulesRequiringRebuild.insert(sourceModuleId)
     }
 
     // Now that we've determined if this module must be rebuilt, mark it as visited.
     visited.insert(sourceModuleId)
   }
+
+  func verifyModuleDependencyUpToDate(moduleID: ModuleDependencyId,
+                                      fileSystem: FileSystem,
+                                      reporter: IncrementalCompilationState.Reporter?) throws -> Bool {
+    let checkedModuleInfo = try moduleInfo(of: moduleID)
+    // Verify that the specified input exists and is older than the specified output
+    let verifyInputOlderThanOutputModTime: (String, VirtualPath, TimePoint) -> Bool =
+    { moduleName, inputPath, outputModTime in
+      guard let inputModTime =
+              try? fileSystem.lastModificationTime(for: inputPath) else {
+        reporter?.report("Unable to 'stat' \(inputPath.description)")
+        return false
+      }
+      if inputModTime > outputModTime {
+        reporter?.reportExplicitDependencyOutOfDate(moduleName,
+                                                    inputPath: inputPath.description)
+        return false
+      }
+      return true
+    }
+
+    // Check if the output file exists
+    guard let outputModTime = try? fileSystem.lastModificationTime(for: VirtualPath.lookup(checkedModuleInfo.modulePath.path)) else {
+      reporter?.report("Module output not found: '\(moduleID.moduleNameForDiagnostic)'")
+      return false
+    }
+
+    // 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)
+      if !verifyInputOlderThanOutputModTime(moduleID.moduleName,
+                                            VirtualPath.lookup(dependencyInfo.modulePath.path),
+                                            outputModTime) {
+        return false
+      }
+    }
+
+    // Check if any of the textual sources of this module are newer than this module
+    switch checkedModuleInfo.details {
+    case .swift(let swiftDetails):
+      if let moduleInterfacePath = swiftDetails.moduleInterfacePath {
+        if !verifyInputOlderThanOutputModTime(moduleID.moduleName,
+                                              VirtualPath.lookup(moduleInterfacePath.path),
+                                              outputModTime) {
+          return false
+        }
+      }
+      if let bridgingHeaderPath = swiftDetails.bridgingHeaderPath {
+        if !verifyInputOlderThanOutputModTime(moduleID.moduleName,
+                                              VirtualPath.lookup(bridgingHeaderPath.path),
+                                              outputModTime) {
+          return false
+        }
+      }
+      for bridgingSourceFile in swiftDetails.bridgingSourceFiles ?? [] {
+        if !verifyInputOlderThanOutputModTime(moduleID.moduleName,
+                                              VirtualPath.lookup(bridgingSourceFile.path),
+                                              outputModTime) {
+          return false
+        }
+      }
+    case .clang(_):
+      for inputSourceFile in checkedModuleInfo.sourceFiles ?? [] {
+        if !verifyInputOlderThanOutputModTime(moduleID.moduleName,
+                                              try VirtualPath(path: inputSourceFile),
+                                              outputModTime) {
+          return false
+        }
+      }
+    case .swiftPrebuiltExternal(_):
+      // TODO: We have to give-up here until we have a way to verify the timestamp of the binary module.
+      // We can do better here by knowing if this module hasn't changed - which would allows us to not
+      // invalidate any of the dependencies that depend on it.
+      reporter?.report("Unable to verify binary module dependency up-to-date: \(moduleID.moduleNameForDiagnostic)")
+      return false;
+    case .swiftPlaceholder(_):
+      // TODO: This should never ever happen. Hard error?
+      return false;
+    }
+
+    return true
+  }
 }
 
 internal extension InterModuleDependencyGraph {

Sources/SwiftDriver/IncrementalCompilation/FirstWaveComputer.swift

@@ -155,7 +155,9 @@ extension IncrementalCompilationState.FirstWaveComputer {
 
     // Determine which module pre-build jobs must be re-run
     let modulesRequiringReBuild =
-      try moduleDependencyGraph.computeInvalidatedModuleDependencies(fileSystem: fileSystem, reporter: reporter)
+      try moduleDependencyGraph.computeInvalidatedModuleDependencies(fileSystem: fileSystem,
+                                                                     forRebuild: true,
+                                                                     reporter: reporter)
 
     // Filter the `.generatePCM` and `.compileModuleFromInterface` jobs for 
     // modules which do *not* need re-building.

Sources/SwiftDriver/IncrementalCompilation/IncrementalCompilationState+Extensions.swift

@@ -308,6 +308,11 @@ extension IncrementalCompilationState {
       report("Dependency module '\(moduleOutputPath)' will be re-built: \(reason)")
     }
 
+    func reportPriorExplicitDependencyStale(_ moduleOutputPath: String,
+                                               reason: String) {
+      report("Dependency module '\(moduleOutputPath)' info is stale: \(reason)")
+    }
+
     func reportExplicitDependencyReBuildSet(_ modules: [ModuleDependencyId]) {
       report("Following explicit module dependencies will be re-built: [\(modules.map { $0.moduleNameForDiagnostic }.sorted().joined(separator: ", "))]")
     }

Sources/SwiftDriver/IncrementalCompilation/IncrementalDependencyAndInputSetup.swift

@@ -120,109 +120,16 @@ extension IncrementalCompilationState.IncrementalDependencyAndInputSetup {
     }
 
     // Verify that each dependnecy is up-to-date with respect to its inputs
-    guard try verifyInterModuleDependenciesUpToDate(in: priorInterModuleDependencyGraph,
-                                                    buildRecordInfo: buildRecordInfo,
-                                                    reporter: reporter) else {
+    guard try priorInterModuleDependencyGraph.computeInvalidatedModuleDependencies(fileSystem: buildRecordInfo.fileSystem,
+                                                                                   forRebuild: false,
+                                                                                   reporter: reporter).isEmpty else {
       reporter?.reportExplicitBuildMustReScan("Not all dependencies are up-to-date.")
       return nil
     }
 
     reporter?.report("Confirmed prior inter-module dependency graph is up-to-date at: \(buildRecordInfo.interModuleDependencyGraphPath)")
     return priorInterModuleDependencyGraph
   }
-
-  static func verifyModuleDependencyUpToDate(moduleID: ModuleDependencyId, moduleInfo: ModuleInfo,
-                                             fileSystem: FileSystem,
-                                             reporter: IncrementalCompilationState.Reporter?) throws -> Bool {
-    // Verify that the specified input exists and is older than the specified output
-    let verifyInputOlderThanOutputModTime: (String, VirtualPath, VirtualPath, TimePoint) -> Bool =
-    { moduleName, inputPath, outputPath, outputModTime in
-      guard let inputModTime =
-              try? fileSystem.lastModificationTime(for: inputPath) else {
-        reporter?.report("Unable to 'stat' \(inputPath.description)")
-        return false
-      }
-      if inputModTime > outputModTime {
-        reporter?.reportExplicitDependencyOutOfDate(moduleName,
-                                                    inputPath: inputPath.description)
-        return false
-      }
-      return true
-    }
-
-    switch moduleInfo.details {
-    case .swift(let swiftDetails):
-      guard let outputModTime = try? fileSystem.lastModificationTime(for: VirtualPath.lookup(moduleInfo.modulePath.path)) else {
-        reporter?.report("Module output not found: '\(moduleID.moduleNameForDiagnostic)'")
-        return false
-      }
-      if let moduleInterfacePath = swiftDetails.moduleInterfacePath {
-        if !verifyInputOlderThanOutputModTime(moduleID.moduleName,
-                                              VirtualPath.lookup(moduleInterfacePath.path),
-                                              VirtualPath.lookup(moduleInfo.modulePath.path),
-                                              outputModTime) {
-          return false
-        }
-      }
-      if let bridgingHeaderPath = swiftDetails.bridgingHeaderPath {
-        if !verifyInputOlderThanOutputModTime(moduleID.moduleName,
-                                              VirtualPath.lookup(bridgingHeaderPath.path),
-                                              VirtualPath.lookup(moduleInfo.modulePath.path),
-                                              outputModTime) {
-          return false
-        }
-      }
-      for bridgingSourceFile in swiftDetails.bridgingSourceFiles ?? [] {
-        if !verifyInputOlderThanOutputModTime(moduleID.moduleName,
-                                              VirtualPath.lookup(bridgingSourceFile.path),
-                                              VirtualPath.lookup(moduleInfo.modulePath.path),
-                                              outputModTime) {
-          return false
-        }
-      }
-    case .clang(_):
-      guard let outputModTime = try? fileSystem.lastModificationTime(for: VirtualPath.lookup(moduleInfo.modulePath.path)) else {
-        reporter?.report("Module output not found: '\(moduleID.moduleNameForDiagnostic)'")
-        return false
-      }
-      for inputSourceFile in moduleInfo.sourceFiles ?? [] {
-        if !verifyInputOlderThanOutputModTime(moduleID.moduleName,
-                                              try VirtualPath(path: inputSourceFile),
-                                              VirtualPath.lookup(moduleInfo.modulePath.path),
-                                              outputModTime) {
-          return false
-        }
-      }
-    case .swiftPrebuiltExternal(_):
-      // TODO: We have to give-up here until we have a way to verify the timestamp of the binary module.
-      // We can do better here by knowing if this module hasn't change - which would allows us to not
-      // invalidate any of the dependencies that depend on it.
-      reporter?.report("Unable to verify binary module dependency up-to-date: \(moduleID.moduleNameForDiagnostic)")
-      return false;
-    case .swiftPlaceholder(_):
-      // TODO: This should never ever happen. Hard error?
-      return false;
-    }
-    return true
-  }
-
-  /// For each direct and transitive module dependency, check if any of the inputs are newer than the output
-  static func verifyInterModuleDependenciesUpToDate(in graph: InterModuleDependencyGraph,
-                                                    buildRecordInfo: BuildRecordInfo,
-                                                    reporter: IncrementalCompilationState.Reporter?) throws -> Bool {
-    for module in graph.modules {
-      if module.key == .swift(graph.mainModuleName) {
-        continue
-      }
-      if try !verifyModuleDependencyUpToDate(moduleID: module.key, 
-                                             moduleInfo: module.value,
-                                             fileSystem: buildRecordInfo.fileSystem,
-                                             reporter: reporter) {
-        return false
-      }
-    }
-    return true
-  }
 }
 
 /// Builds the `InitialState`