Merge pull request #400 from davidungar/honor-always-rebuild-deps

[Incremental] Honor -driver-always-rebuild-dependents

Author: David Ungar

Sources/SwiftDriver/Incremental Compilation/DependencyKey.swift

@@ -2,7 +2,7 @@
 import Foundation
 
 /// A filename from another module
-struct ExternalDependency: Hashable, CustomStringConvertible {
+struct ExternalDependency: Hashable, Comparable, CustomStringConvertible {
   let fileName: String
 
   var file: VirtualPath? {
@@ -14,6 +14,10 @@ struct ExternalDependency: Hashable, CustomStringConvertible {
   public var description: String {
     fileName.description
   }
+
+  public static func < (lhs: Self, rhs: Self) -> Bool {
+    lhs.fileName < rhs.fileName
+  }
 }
 
 
@@ -31,7 +35,7 @@ public struct DependencyKey: Hashable, CustomStringConvertible {
   /// implementations white. Each node holds an instance variable describing which
   /// aspect of the entity it represents.
 
-  enum DeclAspect {
+  enum DeclAspect: Comparable {
     case interface, implementation
   }
 
@@ -112,3 +116,15 @@ var correspondingImplementation: Self? {
   }
 }
 
+// MARK: - Comparing
+/// Needed to sort nodes to make tracing deterministic to test against emitted diagnostics
+extension DependencyKey: Comparable {
+  public static func < (lhs: Self, rhs: Self) -> Bool {
+    lhs.aspect != rhs.aspect ? lhs.aspect < rhs.aspect :
+      lhs.designator < rhs.designator
+  }
+}
+
+extension DependencyKey.Designator: Comparable {
+}
+

Sources/SwiftDriver/Incremental Compilation/IncrementalCompilationState.swift

@@ -118,6 +118,7 @@ public class IncrementalCompilationState {
       buildRecordInfo: buildRecordInfo,
       moduleDependencyGraph: moduleDependencyGraph,
       outOfDateBuildRecord: outOfDateBuildRecord,
+      alwaysRebuildDependents: parsedOptions.contains(.driverAlwaysRebuildDependents),
       reportIncrementalDecision: reportIncrementalDecision)
 
     self.moduleDependencyGraph = moduleDependencyGraph
@@ -208,6 +209,7 @@ extension IncrementalCompilationState {
     buildRecordInfo: BuildRecordInfo,
     moduleDependencyGraph: ModuleDependencyGraph,
     outOfDateBuildRecord: BuildRecord,
+    alwaysRebuildDependents: Bool,
     reportIncrementalDecision: ((String, TypedVirtualPath?) -> Void)?
   ) -> Set<TypedVirtualPath> {
 
@@ -238,6 +240,7 @@ extension IncrementalCompilationState {
     let speculativeInputs = computeSpeculativeInputs(
       changedInputs: changedInputs,
       moduleDependencyGraph: moduleDependencyGraph,
+      alwaysRebuildDependents: alwaysRebuildDependents,
       reportIncrementalDecision: reportIncrementalDecision)
       .subtracting(definitelyRequiredInputs)
 
@@ -330,26 +333,32 @@ extension IncrementalCompilationState {
   private static func computeSpeculativeInputs(
     changedInputs: [(TypedVirtualPath, InputInfo.Status)],
     moduleDependencyGraph: ModuleDependencyGraph,
+    alwaysRebuildDependents: Bool,
     reportIncrementalDecision: ((String, TypedVirtualPath?) -> Void)?
     ) -> Set<TypedVirtualPath> {
     // Collect the files that will be compiled whose dependents should be schedule
     let cascadingFiles: [TypedVirtualPath] = changedInputs.compactMap { input, status in
       let basename = input.file.basename
-      switch status {
-      case .needsCascadingBuild:
+      switch (status, alwaysRebuildDependents) {
+
+       case (_, true):
+        reportIncrementalDecision?(
+          "scheduling dependents of \(basename); -driver-always-rebuild-dependents", nil)
+        return input
+      case (.needsCascadingBuild, false):
         reportIncrementalDecision?(
           "scheduling dependents of \(basename); needed cascading build", nil)
         return input
 
-      case .upToDate: // Must be building because it changed
+      case (.upToDate, false): // was up to date, but changed
         reportIncrementalDecision?(
           "not scheduling dependents of \(basename); unknown changes", nil)
         return nil
-       case .newlyAdded:
+       case (.newlyAdded, false):
         reportIncrementalDecision?(
           "not scheduling dependents of \(basename): no entry in build record or dependency graph", nil)
         return nil
-      case .needsNonCascadingBuild:
+      case (.needsNonCascadingBuild, false):
         reportIncrementalDecision?(
           "not scheduling dependents of \(basename): does not need cascading build", nil)
         return nil

Sources/SwiftDriver/Incremental Compilation/ModuleDependencyGraph Parts/Node.swift

@@ -80,6 +80,22 @@ extension ModuleDependencyGraph.Node: Equatable, Hashable {
   }
 }
 
+extension ModuleDependencyGraph.Node: Comparable {
+  static func < (lhs: ModuleDependencyGraph.Node, rhs: ModuleDependencyGraph.Node) -> Bool {
+    func lt<T: Comparable> (_ a: T?, _ b: T?) -> Bool {
+      switch (a, b) {
+      case let (x?, y?): return x < y
+      case (nil, nil): return false
+      case (nil, _?): return true
+      case (_?, nil): return false
+      }
+    }
+    return lhs.dependencyKey != rhs.dependencyKey ? lhs.dependencyKey < rhs.dependencyKey :
+      lhs.swiftDeps != rhs.swiftDeps ? lt(lhs.swiftDeps, rhs.swiftDeps)
+      : lt(lhs.fingerprint, rhs.fingerprint)
+  }
+}
+
 
 extension ModuleDependencyGraph.Node: CustomStringConvertible {
   public var description: String {

Sources/SwiftDriver/Incremental Compilation/ModuleDependencyGraph Parts/NodeFinder.swift

@@ -71,6 +71,14 @@ extension ModuleDependencyGraph.NodeFinder {
       .map(fnVerifyingSwiftDeps)
   }
 
+  func forEachUseInOrder(of def: Graph.Node, _ fn: (Graph.Node, Graph.SwiftDeps) -> Void) {
+    var uses = [(Graph.Node, Graph.SwiftDeps)]()
+    forEachUse(of: def) {
+      uses.append(($0, $1))
+    }
+    uses.sorted {$0.0 < $1.0} .forEach { fn($0.0, $0.1) }
+  }
+
   func mappings(of n: Graph.Node) -> [(Graph.SwiftDeps?, DependencyKey)]
   {
     nodeMap.compactMap {

Sources/SwiftDriver/Incremental Compilation/ModuleDependencyGraph Parts/SwiftDeps.swift

@@ -46,3 +46,9 @@ extension ModuleDependencyGraph.SwiftDeps {
     file.name
   }
 }
+// MARK: - comparing
+extension ModuleDependencyGraph.SwiftDeps: Comparable {
+  static func < (lhs: Self, rhs: Self) -> Bool {
+    lhs.file.name < rhs.file.name
+  }
+}

Sources/SwiftDriver/Incremental Compilation/ModuleDependencyGraph Parts/Tracer.swift

@@ -57,7 +57,8 @@ extension ModuleDependencyGraph.Tracer {
   where Nodes.Element == ModuleDependencyGraph.Node
   {
     self.graph = graph
-    self.startingPoints = Array(defs)
+    // Sort so "Tracing" diagnostics are deterministically ordered
+    self.startingPoints = defs.sorted()
     self.currentPathIfTracing = graph.reportIncrementalDecision != nil ? [] : nil
     self.diagnosticEngine = diagnosticEngine
   }
@@ -83,7 +84,7 @@ extension ModuleDependencyGraph.Tracer {
     let pathLengthAfterArrival = traceArrival(at: definition);
 
     // If this use also provides something, follow it
-    graph.nodeFinder.forEachUse(of: definition) { use, _ in
+    graph.nodeFinder.forEachUseInOrder(of: definition) { use, _ in
       findNextPreviouslyUntracedDependent(of: use)
     }
     traceDeparture(pathLengthAfterArrival);

Sources/SwiftDriver/Incremental Compilation/ModuleDependencyGraph.swift

@@ -194,7 +194,7 @@ extension ModuleDependencyGraph {
     // These nodes will depend on the *interface* of the external Decl.
     let key = DependencyKey(interfaceFor: externalSwiftDeps)
     let node = Node(key: key, fingerprint: nil, swiftDeps: nil)
-    nodeFinder.forEachUse(of: node) { use, useSwiftDeps in
+    nodeFinder.forEachUseInOrder(of: node) { use, useSwiftDeps in
       if isUntraced(use) {
         fn(useSwiftDeps)
       }

Tests/SwiftDriverTests/IncrementalCompilationTests.swift

@@ -362,6 +362,14 @@ final class IncrementalCompilationTests: XCTestCase {
     #endif
   }
 
+  // FIXME: Expect failure in Linux in CI just as testIncrementalDiagnostics
+  func testAlwaysRebuildDependents() throws {
+    #if !os(Linux)
+    tryInitial(true)
+    tryTouchingMainAlwaysRebuildDependents(true)
+    #endif
+  }
+
   func testIncremental() throws {
     try testIncremental(checkDiagnostics: false)
   }
@@ -500,6 +508,36 @@ final class IncrementalCompilationTests: XCTestCase {
       whenAutolinking: autolinkLifecycleExpectations)
   }
 
+  func tryTouchingMainAlwaysRebuildDependents(_ checkDiagnostics: Bool) {
+    touch("main")
+    let extraArgument = "-driver-always-rebuild-dependents"
+    try! doABuild(
+      "non-propagating but \(extraArgument)",
+      checkDiagnostics: checkDiagnostics,
+      extraArguments: [extraArgument],
+      expectingRemarks: [
+        "Incremental compilation: Scheduing changed input {compile: main.o <= main.swift}",
+        "Incremental compilation: May skip current input: {compile: other.o <= other.swift}",
+        "Incremental compilation: Queuing (initial): {compile: main.o <= main.swift}",
+        "Incremental compilation: scheduling dependents of main.swift; -driver-always-rebuild-dependents",
+        "Incremental compilation: Traced: interface of top-level name foo from: main.swift -> implementation of other.swiftdeps from: other.swift",
+        "Incremental compilation: Found dependent of main.swift: {compile: other.o <= other.swift}",
+        "Incremental compilation: Immediately scheduling dependent on main.swift {compile: other.o <= other.swift}",
+        "Incremental compilation: Queuing (dependent): {compile: other.o <= other.swift}",
+        "Found 2 batchable jobs",
+        "Forming into 1 batch",
+        "Adding {compile: main.swift} to batch 0",
+        "Adding {compile: other.swift} to batch 0",
+        "Forming batch job from 2 constituents: main.swift, other.swift",
+        "Incremental compilation: Queuing Compiling main.swift, other.swift",
+        "Starting Compiling main.swift, other.swift",
+        "Finished Compiling main.swift, other.swift",
+        "Starting Linking theModule",
+        "Finished Linking theModule",
+      ],
+      whenAutolinking: autolinkLifecycleExpectations)
+  }
+
   func touch(_ name: String) {
     print("*** touching \(name) ***", to: &stderrStream); stderrStream.flush()
     let (path, contents) = try! XCTUnwrap(inputPathsAndContents.filter {$0.0.pathString.contains(name)}.first)
@@ -517,17 +555,20 @@ final class IncrementalCompilationTests: XCTestCase {
   }
   func doABuild(_ message: String,
                 checkDiagnostics: Bool,
+                extraArguments: [String] = [],
                 expectingRemarks texts: [String],
                 whenAutolinking: [String]) throws {
     try doABuild(
       message,
       checkDiagnostics: checkDiagnostics,
+      extraArguments: extraArguments,
       expecting: texts.map {.remark($0)},
       expectingWhenAutolinking: whenAutolinking.map {.remark($0)})
   }
 
   func doABuild(_ message: String,
                 checkDiagnostics: Bool,
+                extraArguments: [String] = [],
                 expecting expectations: [Diagnostic.Message],
                 expectingWhenAutolinking autolinkExpectations: [Diagnostic.Message]) throws {
     print("*** starting build \(message) ***", to: &stderrStream); stderrStream.flush()
@@ -537,8 +578,9 @@ final class IncrementalCompilationTests: XCTestCase {
       try? driver.run(jobs: jobs)
     }
 
+    let allArgs = args + extraArguments
     if checkDiagnostics {
-      try assertDriverDiagnostics(args: args) {driver, verifier in
+      try assertDriverDiagnostics(args: allArgs) {driver, verifier in
         verifier.forbidUnexpected(.error, .warning, .note, .remark, .ignored)
         expectations.forEach {verifier.expect($0)}
         if driver.isAutolinkExtractJobNeeded {
@@ -551,7 +593,7 @@ final class IncrementalCompilationTests: XCTestCase {
       let diagnosticEngine = DiagnosticsEngine(handlers: [
         {print($0, to: &stderrStream); stderrStream.flush()}
       ])
-      var driver = try Driver(args: args, env: ProcessEnv.vars,
+      var driver = try Driver(args: allArgs, env: ProcessEnv.vars,
                               diagnosticsEngine: diagnosticEngine,
                               fileSystem: localFileSystem)
       doIt(&driver)