Merge pull request #466 from CodaFi/options-trading

Trade A Pack of Booleans for a Formal OptionSet

Author: CodaFi

Sources/SwiftDriver/IncrementalCompilation/IncrementalCompilationState.swift

@@ -12,9 +12,8 @@
 import TSCBasic
 import Foundation
 import SwiftOptions
+
 public class IncrementalCompilationState {
-  /// Whether cross-module incrementality is enabled
-  private let isCrossModuleIncrementalBuildEnabled: Bool
 
   /// The oracle for deciding what depends on what. Applies to this whole module.
   private let moduleDependencyGraph: ModuleDependencyGraph
@@ -45,24 +44,23 @@ public class IncrementalCompilationState {
   /// Return nil if not compiling incrementally
   init?(
     driver: inout Driver,
+    options: Options,
     jobsInPhases: JobsInPhases
   ) throws {
     guard driver.shouldAttemptIncrementalCompilation()
     else {
       return nil
     }
 
-    if driver.parsedOptions.hasArgument(.driverShowIncremental) || driver.showJobLifecycle {
+    if options.contains(.showIncremental) {
       self.reporter = Reporter(diagnosticEngine: driver.diagnosticEngine,
                                outputFileMap: driver.outputFileMap)
     } else {
       self.reporter = nil
     }
 
-    self.isCrossModuleIncrementalBuildEnabled =
-      driver.parsedOptions.contains(.enableExperimentalCrossModuleIncrementalBuild)
     reporter?.report(
-      "\(self.isCrossModuleIncrementalBuildEnabled ? "Enabling" : "Disabling") incremental cross-module building")
+      "\(options.contains(.enableCrossModuleIncrementalBuild) ? "Enabling" : "Disabling") incremental cross-module building")
 
 
     guard let (outputFileMap, buildRecordInfo, outOfDateBuildRecord)
@@ -75,18 +73,19 @@ public class IncrementalCompilationState {
       moduleDependencyGraph,
       inputsHavingMalformedDependencySources: inputsHavingMalformedDependencySources
     ) = Self.computeModuleDependencyGraph(
+      options,
       buildRecordInfo,
       outOfDateBuildRecord,
       outputFileMap,
-      &driver,
-      self.reporter,
-      isCrossModuleIncrementalBuildEnabled: isCrossModuleIncrementalBuildEnabled)
+      driver,
+      self.reporter)
     else {
       return nil
     }
 
     (skippedCompileGroups: self.skippedCompileGroups,
      mandatoryJobsInOrder: self.mandatoryJobsInOrder) = try Self.computeInputsAndGroups(
+      options,
       jobsInPhases,
       &driver,
       buildRecordInfo,
@@ -101,14 +100,18 @@ public class IncrementalCompilationState {
     self.driver = driver
   }
 
+  @_spi(Testing) public var options: Options {
+    self.moduleDependencyGraph.options
+  }
+
 
   private static func computeModuleDependencyGraph(
+    _ options: Options,
     _ buildRecordInfo: BuildRecordInfo,
     _ outOfDateBuildRecord: BuildRecord,
     _ outputFileMap: OutputFileMap,
-    _ driver: inout Driver,
-    _ reporter: Reporter?,
-    isCrossModuleIncrementalBuildEnabled: Bool
+    _ driver: Driver,
+    _ reporter: Reporter?
   )
   -> (ModuleDependencyGraph,
       inputsHavingMalformedDependencySources: [TypedVirtualPath])?
@@ -123,12 +126,10 @@ public class IncrementalCompilationState {
       inputs: buildRecordInfo.compilationInputModificationDates.keys,
       previousInputs: outOfDateBuildRecord.allInputs,
       outputFileMap: outputFileMap,
-      parsedOptions: &driver.parsedOptions,
+      options: options,
       remarkDisabled: Diagnostic.Message.remark_incremental_compilation_has_been_disabled,
       reporter: reporter,
-      fileSystem: driver.fileSystem,
-      isCrossModuleIncrementalBuildEnabled: isCrossModuleIncrementalBuildEnabled
-      )
+      fileSystem: driver.fileSystem)
     else {
       return nil
     }
@@ -147,6 +148,7 @@ public class IncrementalCompilationState {
   }
 
   private static func computeInputsAndGroups(
+    _ options: Options,
     _ jobsInPhases: JobsInPhases,
     _ driver: inout Driver,
     _ buildRecordInfo: BuildRecordInfo,
@@ -168,7 +170,7 @@ public class IncrementalCompilationState {
       inputsHavingMalformedDependencySources: inputsHavingMalformedDependencySources,
       moduleDependencyGraph: moduleDependencyGraph,
       outOfDateBuildRecord: outOfDateBuildRecord,
-      alwaysRebuildDependents: driver.parsedOptions.contains(.driverAlwaysRebuildDependents),
+      alwaysRebuildDependents: options.contains(.alwaysRebuildDependents),
       reporter: reporter)
 
     let skippedCompileGroups = compileGroups.filter {skippedInputs.contains($0.key)}
@@ -699,3 +701,37 @@ extension IncrementalCompilationState {
     }
   }
 }
+
+extension IncrementalCompilationState {
+  /// Options that control the behavior of various aspects of the
+  /// incremental build.
+  public struct Options: OptionSet {
+    public var rawValue: UInt8
+
+    public init(rawValue: UInt8) {
+      self.rawValue = rawValue
+    }
+
+    /// Be maximally conservative about rebuilding dependents of dirtied files
+    /// during the incremental build. Dependent files are always scheduled to
+    /// rebuild.
+    public static let alwaysRebuildDependents                = Options(rawValue: 1 << 0)
+    /// Print incremental build decisions as remarks.
+    public static let showIncremental                        = Options(rawValue: 1 << 1)
+    /// After integrating each source file dependency graph into the driver's
+    /// module dependency graph, dump a dot file to the current working
+    /// directory showing the state of the driver's dependency graph.
+    ///
+    /// FIXME: This option is not yet implemented.
+    public static let emitDependencyDotFileAfterEveryImport  = Options(rawValue: 1 << 2)
+    /// After integrating each source file dependency graph, verifies the
+    /// integrity of the driver's dependency graph and aborts if any errors
+    /// are detected.
+    public static let verifyDependencyGraphAfterEveryImport  = Options(rawValue: 1 << 3)
+    /// Enables the cross-module incremental build infrastructure.
+    ///
+    /// FIXME: This option is transitory. We intend to make this the
+    /// default behavior. This option should flip to a "disable" bit after that.
+    public static let enableCrossModuleIncrementalBuild      = Options(rawValue: 1 << 4)
+  }
+}

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraph.swift

@@ -34,27 +34,26 @@ import SwiftOptions
   // The set of paths to incremental external dependencies known to be in the graph
   public internal(set) var incrementalExternalDependencies = Set<ExternalDependency>()
 
-  let isCrossModuleIncrementalBuildEnabled: Bool
-  let verifyDependencyGraphAfterEveryImport: Bool
-  let emitDependencyDotFileAfterEveryImport: Bool
+  let options: IncrementalCompilationState.Options
   let reporter: IncrementalCompilationState.Reporter?
 
   private let diagnosticEngine: DiagnosticsEngine
 
   public init(
     diagnosticEngine: DiagnosticsEngine,
     reporter: IncrementalCompilationState.Reporter?,
-    isCrossModuleIncrementalBuildEnabled: Bool,
-    emitDependencyDotFileAfterEveryImport: Bool,
-    verifyDependencyGraphAfterEveryImport: Bool
+    options: IncrementalCompilationState.Options
   ) {
-    self.isCrossModuleIncrementalBuildEnabled = isCrossModuleIncrementalBuildEnabled
-    self.verifyDependencyGraphAfterEveryImport = verifyDependencyGraphAfterEveryImport
-    self.emitDependencyDotFileAfterEveryImport = emitDependencyDotFileAfterEveryImport
     self.reporter = reporter
     self.diagnosticEngine = diagnosticEngine
+    self.options = options
+  }
+
+  var isCrossModuleIncrementalBuildEnabled: Bool {
+    self.options.contains(.enableCrossModuleIncrementalBuild)
   }
 }
+
 // MARK: - initial build only
 extension ModuleDependencyGraph {
   /// Builds a graph
@@ -65,25 +64,20 @@ extension ModuleDependencyGraph {
     inputs: Inputs,
     previousInputs: Set<VirtualPath>,
     outputFileMap: OutputFileMap?,
-    parsedOptions: inout ParsedOptions,
+    options: IncrementalCompilationState.Options,
     remarkDisabled: (String) -> Diagnostic.Message,
     reporter: IncrementalCompilationState.Reporter?,
-    fileSystem: FileSystem,
-    isCrossModuleIncrementalBuildEnabled: Bool
+    fileSystem: FileSystem
   ) -> (
     ModuleDependencyGraph,
     inputsAndMalformedSwiftDeps: [(TypedVirtualPath, VirtualPath)]
   )?
     where Inputs.Element == TypedVirtualPath
   {
-    let emitOpt = Option.driverEmitFineGrainedDependencyDotFileAfterEveryImport
-    let veriOpt = Option.driverVerifyFineGrainedDependencyGraphAfterEveryImport
     let graph = Self(
       diagnosticEngine: diagnosticEngine,
       reporter: reporter,
-      isCrossModuleIncrementalBuildEnabled: isCrossModuleIncrementalBuildEnabled,
-      emitDependencyDotFileAfterEveryImport: parsedOptions.contains(emitOpt),
-      verifyDependencyGraphAfterEveryImport: parsedOptions.contains(veriOpt))
+      options: options)
 
     let inputsAndSwiftdeps = inputs.map { input in
       (input, outputFileMap?.existingOutput(inputFile: input.file,
@@ -118,7 +112,7 @@ extension ModuleDependencyGraph {
               reporter: reporter,
               diagnosticEngine: diagnosticEngine,
               fileSystem: fileSystem,
-              isCrossModuleIncrementalBuildEnabled: isCrossModuleIncrementalBuildEnabled)
+              options: options)
       else {
         return (input, swiftDepsFile)
       }
@@ -188,7 +182,7 @@ extension ModuleDependencyGraph {
       reporter: reporter,
       diagnosticEngine: diagnosticEngine,
       fileSystem: fileSystem,
-      isCrossModuleIncrementalBuildEnabled: graph.isCrossModuleIncrementalBuildEnabled)
+      options: graph.options)
     return results
   }
 
@@ -258,7 +252,7 @@ extension ModuleDependencyGraph {
       reporter: self.reporter,
       diagnosticEngine: diagnosticEngine,
       fileSystem: fileSystem,
-      isCrossModuleIncrementalBuildEnabled: isCrossModuleIncrementalBuildEnabled)
+      options: self.options)
     guard let results = resultsIfOK else {
       return nil
     }
@@ -437,7 +431,7 @@ extension ModuleDependencyGraph {
     on fileSystem: FileSystem,
     diagnosticEngine: DiagnosticsEngine,
     reporter: IncrementalCompilationState.Reporter?,
-    isCrossModuleIncrementalBuildEnabled: Bool
+    options: IncrementalCompilationState.Options
   ) throws -> ModuleDependencyGraph {
     let data = try fileSystem.readFileContents(path)
 
@@ -456,14 +450,12 @@ extension ModuleDependencyGraph {
       init(
         diagnosticEngine: DiagnosticsEngine,
         reporter: IncrementalCompilationState.Reporter?,
-        isCrossModuleIncrementalBuildEnabled: Bool
+        options: IncrementalCompilationState.Options
       ) {
         self.graph = ModuleDependencyGraph(
           diagnosticEngine: diagnosticEngine,
           reporter: reporter,
-          isCrossModuleIncrementalBuildEnabled: isCrossModuleIncrementalBuildEnabled,
-          emitDependencyDotFileAfterEveryImport: false,
-          verifyDependencyGraphAfterEveryImport: false)
+          options: options)
       }
 
       func finalizeGraph() -> ModuleDependencyGraph {
@@ -591,7 +583,7 @@ extension ModuleDependencyGraph {
       let data = Data(bytesNoCopy: baseAddr, count: buf.count, deallocator: .none)
       var visitor = Visitor(diagnosticEngine: diagnosticEngine,
                             reporter: reporter,
-                            isCrossModuleIncrementalBuildEnabled: isCrossModuleIncrementalBuildEnabled)
+                            options: options)
       try Bitcode.read(stream: data, using: &visitor)
       guard let major = visitor.majorVersion,
             let minor = visitor.minorVersion,

Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraphParts/Integrator.swift

@@ -38,24 +38,24 @@ extension ModuleDependencyGraph {
     /// the graph to be integrated into
     let destination: ModuleDependencyGraph
 
-    let isCrossModuleIncrementalBuildEnabled: Bool
-
     /// Starts with all nodes in dependencySource. Nodes that persist will be removed.
     /// After integration is complete, contains the nodes that have disappeared.
     var disappearedNodes = [DependencyKey: Graph.Node]()
 
     init(source: SourceFileDependencyGraph,
          dependencySource: DependencySource,
-         destination: ModuleDependencyGraph,
-         isCrossModuleIncrementalBuildEnabled: Bool)
+         destination: ModuleDependencyGraph)
     {
       self.source = source
       self.dependencySource = dependencySource
       self.destination = destination
-      self.isCrossModuleIncrementalBuildEnabled = isCrossModuleIncrementalBuildEnabled
       self.disappearedNodes = destination.nodeFinder.findNodes(for: dependencySource)
         ?? [:]
     }
+
+    var isCrossModuleIncrementalBuildEnabled: Bool {
+      destination.options.contains(.enableCrossModuleIncrementalBuild)
+    }
   }
 }
 
@@ -69,7 +69,7 @@ extension ModuleDependencyGraph.Integrator {
     reporter: IncrementalCompilationState.Reporter?,
     diagnosticEngine: DiagnosticsEngine,
     fileSystem: FileSystem,
-    isCrossModuleIncrementalBuildEnabled: Bool
+    options: IncrementalCompilationState.Options
   ) -> Results? {
     guard let sfdg = try? SourceFileDependencyGraph.read(
             from: dependencySource, on: fileSystem)
@@ -79,8 +79,7 @@ extension ModuleDependencyGraph.Integrator {
     }
     return integrate(from: sfdg,
                      dependencySource: dependencySource,
-                     into: destination,
-                     isCrossModuleIncrementalBuildEnabled: isCrossModuleIncrementalBuildEnabled)
+                     into: destination)
   }
 }
 // MARK: - integrate a graph
@@ -92,19 +91,17 @@ extension ModuleDependencyGraph.Integrator {
   /*@_spi(Testing)*/ public static func integrate(
     from g: SourceFileDependencyGraph,
     dependencySource: Graph.DependencySource,
-    into destination: Graph,
-    isCrossModuleIncrementalBuildEnabled: Bool
-  ) ->  Results {
+    into destination: Graph
+  ) -> Results {
     var integrator = Self(source: g,
                           dependencySource: dependencySource,
-                          destination: destination,
-                          isCrossModuleIncrementalBuildEnabled: isCrossModuleIncrementalBuildEnabled)
+                          destination: destination)
     integrator.integrate()
 
-    if destination.verifyDependencyGraphAfterEveryImport {
+    if destination.options.contains(.verifyDependencyGraphAfterEveryImport) {
       integrator.verifyAfterImporting()
     }
-    if destination.emitDependencyDotFileAfterEveryImport {
+    if destination.options.contains(.emitDependencyDotFileAfterEveryImport) {
       destination.emitDotFile(g, dependencySource)
     }
     return integrator.results

Sources/SwiftDriver/Jobs/Planning.swift

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 import TSCBasic
+import SwiftOptions
 
 public enum PlanningError: Error, DiagnosticData {
   case replReceivedInput
@@ -117,6 +118,7 @@ extension Driver {
     // Determine the state for incremental compilation
     let incrementalCompilationState = try IncrementalCompilationState(
       driver: &self,
+      options: self.computeIncrementalOptions(),
       jobsInPhases: jobsInPhases)
 
     return try (
@@ -130,6 +132,27 @@ extension Driver {
     )
   }
 
+  mutating func computeIncrementalOptions() -> IncrementalCompilationState.Options {
+    var options: IncrementalCompilationState.Options = []
+    if self.parsedOptions.contains(.driverAlwaysRebuildDependents) {
+      options.formUnion(.alwaysRebuildDependents)
+    }
+    if self.parsedOptions.contains(.driverShowIncremental) || self.showJobLifecycle {
+      options.formUnion(.showIncremental)
+    }
+    let emitOpt = Option.driverEmitFineGrainedDependencyDotFileAfterEveryImport
+    if self.parsedOptions.contains(emitOpt) {
+      options.formUnion(.emitDependencyDotFileAfterEveryImport)
+    }
+    let veriOpt = Option.driverVerifyFineGrainedDependencyGraphAfterEveryImport
+    if self.parsedOptions.contains(veriOpt) {
+      options.formUnion(.verifyDependencyGraphAfterEveryImport)
+    }
+    if self.parsedOptions.contains(.enableExperimentalCrossModuleIncrementalBuild) {
+      options.formUnion(.enableCrossModuleIncrementalBuild)
+    }
+    return options
+  }
 
   private mutating func addPrecompileModuleDependenciesJobs(addJob: (Job) -> Void) throws {
     // If asked, add jobs to precompile module dependencies