Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions Sources/SwiftDriver/Incremental Compilation/DependencyKey.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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? {
Expand All @@ -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
}
}


Expand All @@ -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
}

Expand Down Expand Up @@ -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 {
}

Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public class IncrementalCompilationState {
buildRecordInfo: buildRecordInfo,
moduleDependencyGraph: moduleDependencyGraph,
outOfDateBuildRecord: outOfDateBuildRecord,
alwaysRebuildDependents: parsedOptions.contains(.driverAlwaysRebuildDependents),
reportIncrementalDecision: reportIncrementalDecision)

self.moduleDependencyGraph = moduleDependencyGraph
Expand Down Expand Up @@ -208,6 +209,7 @@ extension IncrementalCompilationState {
buildRecordInfo: BuildRecordInfo,
moduleDependencyGraph: ModuleDependencyGraph,
outOfDateBuildRecord: BuildRecord,
alwaysRebuildDependents: Bool,
reportIncrementalDecision: ((String, TypedVirtualPath?) -> Void)?
) -> Set<TypedVirtualPath> {

Expand Down Expand Up @@ -238,6 +240,7 @@ extension IncrementalCompilationState {
let speculativeInputs = computeSpeculativeInputs(
changedInputs: changedInputs,
moduleDependencyGraph: moduleDependencyGraph,
alwaysRebuildDependents: alwaysRebuildDependents,
reportIncrementalDecision: reportIncrementalDecision)
.subtracting(definitelyRequiredInputs)

Expand Down Expand Up @@ -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):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test that exercises the (.upToDate, true) to ensure this is not regressed in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the legacy lit tests already does so; that's how I found the issue. But, let me see if I can add an XC test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And thanks!

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
46 changes: 44 additions & 2 deletions Tests/SwiftDriverTests/IncrementalCompilationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand All @@ -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 {
Expand All @@ -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)
Expand Down