[5.5, Incremental] Harness immutability to make inputDependencySourceMap robust

Sources/SwiftDriver/CMakeLists.txt

@@ -33,6 +33,7 @@ add_library(SwiftDriver
   Execution/ParsableOutput.swift
   Execution/ProcessProtocol.swift
 
+  "IncrementalCompilation/ModuleDependencyGraphParts/InputDependencySourceMap.swift"
   "IncrementalCompilation/ModuleDependencyGraphParts/Integrator.swift"
   "IncrementalCompilation/ModuleDependencyGraphParts/Node.swift"
   "IncrementalCompilation/ModuleDependencyGraphParts/NodeFinder.swift"

Sources/SwiftDriver/IncrementalCompilation/BidirectionalMap.swift

@@ -12,10 +12,6 @@
 
 /// Like a two-way dictionary
 ///
-// FIXME: The current use of this abstraction in the driver is
-// fundamentally broken. This data structure should be retired ASAP.
-// See the extended FIXME in its use in
-// `ModuleDependencyGraph.inputDependencySourceMap`
 public struct BidirectionalMap<T1: Hashable, T2: Hashable>: Equatable, Sequence {
   private var map1: [T1: T2] = [:]
   private var map2: [T2: T1] = [:]
@@ -74,6 +70,20 @@ public struct BidirectionalMap<T1: Hashable, T2: Hashable>: Equatable, Sequence
   public func contains(key: T2) -> Bool {
     map2.keys.contains(key)
   }
+
+  public mutating func updateValue(_ newValue: T2, forKey key: T1) -> T2? {
+    let oldValue = map1.updateValue(newValue, forKey: key)
+    _ = oldValue.map {map2.removeValue(forKey: $0)}
+    map2[newValue] = key
+    return oldValue
+  }
+  public mutating func updateValue(_ newValue: T1, forKey key: T2) -> T1? {
+    let oldValue = map2.updateValue(newValue, forKey: key)
+   _ = oldValue.map {map1.removeValue(forKey: $0)}
+    map1[newValue] = key
+    return oldValue
+  }
+
   public mutating func removeValue(forKey t1: T1) {
     if let t2 = map1[t1] {
       map2.removeValue(forKey: t2)

Sources/SwiftDriver/IncrementalCompilation/DirectAndTransitiveCollections.swift

@@ -19,7 +19,7 @@ public struct Directly {}
 public struct InvalidatedSet<ClosureLevel, Element: Hashable>: Sequence {
   var contents: Set<Element>
 
-  init(_ s: Set<Element> = Set()) {
+  @_spi(Testing) public init(_ s: Set<Element> = Set()) {
     self.contents = s
   }
   init<Elements: Sequence>(_ elements: Elements)

Sources/SwiftDriver/IncrementalCompilation/IncrementalDependencyAndInputSetup.swift

@@ -237,9 +237,6 @@ extension IncrementalCompilationState.IncrementalDependencyAndInputSetup {
     else {
       return buildInitialGraphFromSwiftDepsAndCollectInputsInvalidatedByChangedExternals()
     }
-    guard graph.populateInputDependencySourceMap() else {
-      return nil
-    }
     graph.dotFileWriter?.write(graph)
 
     // Any externals not already in graph must be additions which should trigger
@@ -264,12 +261,10 @@ extension IncrementalCompilationState.IncrementalDependencyAndInputSetup {
   private func buildInitialGraphFromSwiftDepsAndCollectInputsInvalidatedByChangedExternals()
   -> (ModuleDependencyGraph, TransitivelyInvalidatedInputSet)?
   {
-    let graph = ModuleDependencyGraph(self, .buildingWithoutAPrior)
-    assert(outputFileMap.onlySourceFilesHaveSwiftDeps())
-    guard graph.populateInputDependencySourceMap() else {
+    guard let graph = ModuleDependencyGraph(self, .buildingWithoutAPrior)
+    else {
       return nil
     }
-
     var inputsInvalidatedByChangedExternals = TransitivelyInvalidatedInputSet()
     for input in sourceFiles.currentInOrder {
        guard let invalidatedInputs =

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraph.swift

@@ -24,19 +24,7 @@ import SwiftOptions
   @_spi(Testing) public var nodeFinder = NodeFinder()
 
   /// Maps input files (e.g. .swift) to and from the DependencySource object.
-  ///
-  // FIXME: The map between swiftdeps and swift files is absolutely *not*
-  // a bijection. In particular, more than one swiftdeps file can be encountered
-  // in the course of deserializing priors *and* reading the output file map
-  // *and* re-reading swiftdeps files after frontends complete
-  // that correspond to the same swift file. These cause two problems:
-  // - overwrites in this data structure that lose data and
-  // - cache misses in `getInput(for:)` that cause the incremental build to
-  // turn over when e.g. entries in the output file map change. This should be
-  // replaced by a multi-map from swift files to dependency sources,
-  // and a regular map from dependency sources to swift files -
-  // since that direction really is one-to-one.
-  @_spi(Testing) public private(set) var inputDependencySourceMap = BidirectionalMap<TypedVirtualPath, DependencySource>()
+  @_spi(Testing) public let inputDependencySourceMap: InputDependencySourceMap
 
   // The set of paths to external dependencies known to be in the graph
   public internal(set) var fingerprintedExternalDependencies = Set<FingerprintedExternalDependency>()
@@ -55,7 +43,7 @@ import SwiftOptions
   /// Minimize the number of file system modification-time queries.
   private var externalDependencyModTimeCache = [ExternalDependency: Bool]()
 
-  public init(_ info: IncrementalCompilationState.IncrementalDependencyAndInputSetup,
+  public init?(_ info: IncrementalCompilationState.IncrementalDependencyAndInputSetup,
               _ phase: Phase
   ) {
     self.info = info
@@ -64,34 +52,22 @@ import SwiftOptions
     : nil
     self.phase = phase
     self.creationPhase = phase
-  }
-
-  private func addMapEntry(_ input: TypedVirtualPath, _ dependencySource: DependencySource) {
-    assert(input.type == .swift && dependencySource.typedFile.type == .swiftDeps)
-    inputDependencySourceMap[input] = dependencySource
-  }
-
-  @_spi(Testing) public func getSource(for input: TypedVirtualPath,
-                                       function: String = #function,
-                                       file: String = #file,
-                                       line: Int = #line) -> DependencySource {
-    guard let source = inputDependencySourceMap[input] else {
-      fatalError("\(input.file) not found in map: \(inputDependencySourceMap), \(file):\(line) in \(function)")
+    guard let inputDependencySourceMap = InputDependencySourceMap(info)
+    else {
+      return nil
     }
-    return source
+    self.inputDependencySourceMap = inputDependencySourceMap
   }
 
-  @_spi(Testing) public func getInput(for source: DependencySource) -> TypedVirtualPath? {
-    guard let input =
-            info.simulateGetInputFailure ? nil
-            : inputDependencySourceMap[source]
+  @_spi(Testing) public func sourceRequired(for input: TypedVirtualPath,
+                                            function: String = #function,
+                                            file: String = #file,
+                                            line: Int = #line) -> DependencySource {
+    guard let source = inputDependencySourceMap.sourceIfKnown(for: input)
     else {
-      info.diagnosticEngine.emit(warning: "Failed to find source file for '\(source.file.basename)', recovering with a full rebuild. Next build will be incremental.")
-      info.reporter?.report(
-        "\(info.simulateGetInputFailure ? "Simulating i" : "I")nput not found in inputDependencySourceMap; created for: \(creationPhase), now: \(phase)")
-      return nil
+      fatalError("\(input.file.basename) not found in inputDependencySourceMap, \(file):\(line) in \(function)")
     }
-    return input
+    return source
   }
 }
 
@@ -161,7 +137,7 @@ extension ModuleDependencyGraph {
       return TransitivelyInvalidatedInputSet()
     }
     return collectInputsRequiringCompilationAfterProcessing(
-      dependencySource: getSource(for: input))
+      dependencySource: sourceRequired(for: input))
   }
 }
 
@@ -183,17 +159,16 @@ extension ModuleDependencyGraph {
   /// speculatively scheduled in the first wave.
   func collectInputsInvalidatedBy(input: TypedVirtualPath
   ) -> TransitivelyInvalidatedInputArray {
-    let changedSource = getSource(for: input)
+    let changedSource = sourceRequired(for: input)
     let allDependencySourcesToRecompile =
       collectSwiftDepsUsing(dependencySource: changedSource)
 
     return allDependencySourcesToRecompile.compactMap {
       dependencySource in
       guard dependencySource != changedSource else {return nil}
-      let dependentSource = inputDependencySourceMap[dependencySource]
-      info.reporter?.report(
-        "Found dependent of \(input.file.basename):", dependentSource)
-      return dependentSource
+      let inputToRecompile = inputDependencySourceMap.inputIfKnown(for: dependencySource)
+      info.reporter?.report("Found dependent of \(input.file.basename):", inputToRecompile)
+      return inputToRecompile
     }
   }
 
@@ -210,7 +185,7 @@ extension ModuleDependencyGraph {
   /// Does the graph contain any dependency nodes for a given source-code file?
   func containsNodes(forSourceFile file: TypedVirtualPath) -> Bool {
     precondition(file.type == .swift)
-    guard let source = inputDependencySourceMap[file] else {
+    guard let source = inputDependencySourceMap.sourceIfKnown(for: file) else {
       return false
     }
     return containsNodes(forDependencySource: source)
@@ -220,17 +195,9 @@ extension ModuleDependencyGraph {
     return nodeFinder.findNodes(for: source).map {!$0.isEmpty}
       ?? false
   }
-
-  /// Return true on success
-  func populateInputDependencySourceMap() -> Bool {
-    let ofm = info.outputFileMap
-    let de = info.diagnosticEngine
-    return info.inputFiles.reduce(true) { okSoFar, input in
-      ofm.getDependencySource(for: input, diagnosticEngine: de)
-        .map {source in addMapEntry(input, source); return okSoFar } ?? false
-    }
-  }
 }
+
+
 // MARK: - Scheduling the 2nd wave
 extension ModuleDependencyGraph {
   /// After `source` has been compiled, figure out what other source files need compiling.
@@ -240,7 +207,7 @@ extension ModuleDependencyGraph {
   func collectInputsRequiringCompilation(byCompiling input: TypedVirtualPath
   ) -> TransitivelyInvalidatedInputSet? {
     precondition(input.type == .swift)
-    let dependencySource = getSource(for: input)
+    let dependencySource = sourceRequired(for: input)
     return collectInputsRequiringCompilationAfterProcessing(
       dependencySource: dependencySource)
   }
@@ -327,8 +294,10 @@ extension ModuleDependencyGraph {
   ) -> TransitivelyInvalidatedInputSet? {
     var invalidatedInputs = TransitivelyInvalidatedInputSet()
     for invalidatedSwiftDeps in collectSwiftDepsUsingInvalidated(nodes: directlyInvalidatedNodes) {
-      guard let invalidatedInput = getInput(for: invalidatedSwiftDeps)
+      guard let invalidatedInput = inputDependencySourceMap.inputIfKnown(for: invalidatedSwiftDeps)
       else {
+        info.diagnosticEngine.emit(
+          warning: "Failed to find source file for '\(invalidatedSwiftDeps.file.basename)', recovering with a full rebuild. Next build will be incremental.")
         return nil
       }
       invalidatedInputs.insert(invalidatedInput)
@@ -405,27 +374,6 @@ extension ModuleDependencyGraph {
   }
 }
 
-extension OutputFileMap {
-  fileprivate func getDependencySource(
-    for sourceFile: TypedVirtualPath,
-    diagnosticEngine: DiagnosticsEngine
-  ) -> DependencySource? {
-    assert(sourceFile.type == FileType.swift)
-    guard let swiftDepsPath = existingOutput(inputFile: sourceFile.fileHandle,
-                                             outputType: .swiftDeps)
-    else {
-      // The legacy driver fails silently here.
-      diagnosticEngine.emit(
-        .remarkDisabled("\(sourceFile.file.basename) has no swiftDeps file")
-      )
-      return nil
-    }
-    assert(VirtualPath.lookup(swiftDepsPath).extension == FileType.swiftDeps.rawValue)
-    let typedSwiftDepsFile = TypedVirtualPath(file: swiftDepsPath, type: .swiftDeps)
-    return DependencySource(typedSwiftDepsFile)
-  }
-}
-
 // MARK: - tracking traced nodes
 extension ModuleDependencyGraph {
 
@@ -536,12 +484,15 @@ extension ModuleDependencyGraph {
       private var identifiers: [String] = [""]
       private var currentDefKey: DependencyKey? = nil
       private var nodeUses: [(DependencyKey, Int)] = []
-      private var inputDependencySourceMap: [(TypedVirtualPath, DependencySource)] = []
       public private(set) var allNodes: [Node] = []
 
-      init(_ info: IncrementalCompilationState.IncrementalDependencyAndInputSetup) {
+      init?(_ info: IncrementalCompilationState.IncrementalDependencyAndInputSetup) {
         self.fileSystem = info.fileSystem
-        self.graph = ModuleDependencyGraph(info, .updatingFromAPrior)
+        guard let graph = ModuleDependencyGraph(info, .updatingFromAPrior)
+        else {
+          return nil
+        }
+        self.graph = graph
       }
 
       func finalizeGraph() -> ModuleDependencyGraph {
@@ -550,10 +501,6 @@ extension ModuleDependencyGraph {
             .record(def: dependencyKey, use: self.allNodes[useID])
           assert(isNewUse, "Duplicate use def-use arc in graph?")
         }
-        for (input, source) in inputDependencySourceMap {
-          graph.addMapEntry(input, source)
-        }
-
         return self.graph
       }
 
@@ -650,20 +597,6 @@ extension ModuleDependencyGraph {
           else {
             throw ReadError.malformedModuleDepGraphNodeRecord
           }
-          let inputPathString = identifiers[Int(record.fields[0])]
-          let dependencySourcePathString = identifiers[Int(record.fields[1])]
-          let inputHandle = try VirtualPath.intern(path: inputPathString)
-          let inputPath = VirtualPath.lookup(inputHandle)
-          let dependencySourceHandle = try VirtualPath.intern(path: dependencySourcePathString)
-          let dependencySourcePath = VirtualPath.lookup(dependencySourceHandle)
-          guard inputPath.extension == FileType.swift.rawValue,
-                dependencySourcePath.extension == FileType.swiftDeps.rawValue,
-                let dependencySource = DependencySource(dependencySourceHandle)
-          else {
-            throw ReadError.malformedMapRecord
-          }
-          let input = TypedVirtualPath(file: inputHandle, type: .swift)
-          inputDependencySourceMap.append((input, dependencySource))
         case .externalDepNode:
           guard record.fields.count == 2,
                 record.fields[0] < identifiers.count,
@@ -689,7 +622,9 @@ extension ModuleDependencyGraph {
       }
     }
 
-    var visitor = Visitor(info)
+    guard var visitor = Visitor(info) else {
+      return nil
+    }
     try Bitcode.read(bytes: data, using: &visitor)
     guard let major = visitor.majorVersion,
           let minor = visitor.minorVersion,
@@ -849,7 +784,7 @@ extension ModuleDependencyGraph {
         }
       }
 
-      for (input, dependencySource) in graph.inputDependencySourceMap {
+      graph.inputDependencySourceMap.enumerateToSerializePriors { input, dependencySource in
         self.addIdentifier(input.file.name)
         self.addIdentifier(dependencySource.file.name)
       }
@@ -994,7 +929,8 @@ extension ModuleDependencyGraph {
             }
           }
         }
-        for (input, dependencySource) in graph.inputDependencySourceMap {
+        graph.inputDependencySourceMap.enumerateToSerializePriors {
+          input, dependencySource in
           serializer.stream.writeRecord(serializer.abbreviations[.mapNode]!) {
             $0.append(RecordID.mapNode)
             $0.append(serializer.lookupIdentifierCode(for: input.file.name))
@@ -1097,7 +1033,6 @@ fileprivate extension DependencyKey.Designator {
 extension ModuleDependencyGraph {
   func matches(_ other: ModuleDependencyGraph) -> Bool {
     guard nodeFinder.matches(other.nodeFinder),
-          inputDependencySourceMap.matches(other.inputDependencySourceMap),
           fingerprintedExternalDependencies.matches(other.fingerprintedExternalDependencies)
     else {
       return false
@@ -1112,24 +1047,8 @@ extension Set where Element == ModuleDependencyGraph.Node {
   }
 }
 
-extension BidirectionalMap where T1 == TypedVirtualPath, T2 == DependencySource {
-  fileprivate func matches(_ other: Self) -> Bool {
-    self == other
-  }
-}
-
 extension Set where Element == FingerprintedExternalDependency {
   fileprivate func matches(_ other: Self) -> Bool {
     self == other
   }
 }
-
-/// This should be in a test file, but addMapEntry should be private.
-extension ModuleDependencyGraph {
-  @_spi(Testing) public func mockMapEntry(
-    _ mockInput: TypedVirtualPath,
-    _ mockDependencySource: DependencySource
-  ) {
-    addMapEntry(mockInput, mockDependencySource)
-  }
-}