Skip to content

[Incremental] Improve the interface to DependencySource #490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
CodaFi merged 3 commits into from
Feb 12, 2021
Merged

[Incremental] Improve the interface to DependencySource #490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
86f66a3
Improve the interface to DependencySource
Feb 12, 2021
6b0e5f0
Fixup tests
Feb 12, 2021
451641d
rm IfPresents
Feb 12, 2021
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
13 changes: 11 additions & 2 deletions Sources/SwiftDriver/IncrementalCompilation/DependencyKey.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ import TSCBasic
try? fileSystem.getFileInfo(file).modTime
}

var swiftModuleFile: TypedVirtualPath? {
isSwiftModule ? TypedVirtualPath(file: file, type: .swiftModule) : nil
}

public var description: String {
file.name
}
Expand All @@ -39,8 +43,13 @@ public struct FingerprintedExternalDependency: Hashable, Equatable, ExternalDepe
assert(verifyExternalDependencyAndFingerprint())
}
var externalDependencyToCheck: ExternalDependency? { externalDependency }
var isIncremental: Bool {
fingerprint != nil && externalDependency.isSwiftModule
var incrementalDependencySource: DependencySource? {
guard let _ = fingerprint,
Copy link
Contributor

@CodaFi CodaFi Feb 12, 2021

Choose a reason for hiding this comment

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

Suggested change
guard let _ = fingerprint,
guard fingerprint != nil,

Copy link
Contributor Author

@davidungar davidungar Feb 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Happy to remove the ifPresent's. I was on the fence anyway. Wrt let _ = ..., I'm inclined to leave it in. That idiom is used a tad more frequently in this code than the != nil.

let swiftModuleFile = externalDependency.swiftModuleFile
else {
return nil
}
return DependencySource(swiftModuleFile)
}
}

Expand Down
15 changes: 8 additions & 7 deletions Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraph.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,9 @@ extension ModuleDependencyGraph {
externalDependency: FingerprintedExternalDependency,
fileSystem: FileSystem
) -> Integrator.Results? {
// Save time; don't even try
guard externalDependency.isIncremental else {
guard let dependencySource = externalDependency.incrementalDependencySource else {
return Integrator.Results()
}
let file = externalDependency.externalDependency.file
let dependencySource = DependencySource(file)
reporter?.report("integrating incremental external dependency",
dependencySource.typedFile)
guard let sourceGraph = dependencySource.read(
Expand Down Expand Up @@ -445,6 +442,7 @@ extension ModuleDependencyGraph {
case unexpectedSubblock
case bogusNameOrContext
case unknownKind
case unknownDependencySourceExtension
}

/// Attempts to read a serialized dependency graph from the given path.
Expand Down Expand Up @@ -559,9 +557,12 @@ extension ModuleDependencyGraph {
let swiftDepsStr = hasSwiftDeps ? identifiers[Int(record.fields[5])] : nil
let hasFingerprint = Int(record.fields[6]) != 0
let fingerprint = hasFingerprint ? fingerprintStr : nil
let dependencySource = try swiftDepsStr
.map({ try VirtualPath(path: $0) })
.map(DependencySource.init)
guard let dependencySource = try swiftDepsStr
.map({ try VirtualPath(path: $0) })
.map(DependencySource.init)
else {
throw ReadError.unknownDependencySourceExtension
}
self.finalize(node: Node(key: key,
fingerprint: fingerprint,
dependencySource: dependencySource))
Expand Down
11 changes: 6 additions & 5 deletions Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraphParts/DependencySource.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ public struct DependencySource: Hashable, CustomStringConvertible {
}

/*@_spi(Testing)*/
public init(_ file: VirtualPath) {
/// Returns nil if cannot be a source
public init?(_ file: VirtualPath) {
let ext = file.extension
let typeIfExpected =
guard let type =
ext == FileType.swiftDeps .rawValue ? FileType.swiftDeps :
ext == FileType.swiftModule.rawValue ? FileType.swiftModule
: nil
guard let type = typeIfExpected else {
fatalError("unexpected dependencySource extension: \(String(describing: ext))")
else {
return nil
}
self.init(TypedVirtualPath(file: file, type: type))
}
Expand All @@ -48,7 +49,7 @@ public struct DependencySource: Hashable, CustomStringConvertible {
// MARK: - mocking
extension DependencySource {
/*@_spi(Testing)*/ public init(mock i: Int) {
self.init(try! VirtualPath(path: String(i) + "." + FileType.swiftDeps.rawValue))
self.init(try! VirtualPath(path: String(i) + "." + FileType.swiftDeps.rawValue))!
}

/*@_spi(Testing)*/ public var mockID: Int {
Expand Down
9 changes: 5 additions & 4 deletions Tests/SwiftDriverTests/IncrementalCompilationTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ final class NonincrementalCompilationTests: XCTestCase {
func testReadBinarySourceFileDependencyGraph() throws {
let absolutePath = try XCTUnwrap(Fixture.fixturePath(at: RelativePath("Incremental"),
for: "main.swiftdeps"))
let dependencySource = DependencySource(VirtualPath.absolute(absolutePath))
let dependencySource = DependencySource(VirtualPath.absolute(absolutePath))!
let graph = try XCTUnwrap(
try SourceFileDependencyGraph(
contentsOf: dependencySource,
Expand Down Expand Up @@ -117,7 +117,7 @@ final class NonincrementalCompilationTests: XCTestCase {
for: "hello.swiftdeps"))
let graph = try XCTUnwrap(
try SourceFileDependencyGraph(
contentsOf: DependencySource(VirtualPath.absolute(absolutePath)),
contentsOf: DependencySource(VirtualPath.absolute(absolutePath))!,
on: localFileSystem))
XCTAssertEqual(graph.majorVersion, 1)
XCTAssertEqual(graph.minorVersion, 0)
Expand Down Expand Up @@ -171,7 +171,7 @@ final class NonincrementalCompilationTests: XCTestCase {
let data = try localFileSystem.readFileContents(absolutePath)
let graph = try XCTUnwrap(
try SourceFileDependencyGraph(data: data,
from: DependencySource(.absolute(absolutePath)),
from: DependencySource(.absolute(absolutePath))!,
fromSwiftModule: true))
XCTAssertEqual(graph.majorVersion, 1)
XCTAssertEqual(graph.minorVersion, 0)
Expand Down Expand Up @@ -822,7 +822,8 @@ class CrossModuleIncrementalBuildTests: XCTestCase {

let sourcePath = path.appending(component: "main.swiftdeps")
let data = try localFileSystem.readFileContents(sourcePath)
let graph = try XCTUnwrap(SourceFileDependencyGraph(data: data, from: DependencySource(.absolute(sourcePath)), fromSwiftModule: false))
let graph = try XCTUnwrap(SourceFileDependencyGraph(data: data, from: DependencySource(.absolute(sourcePath))!,
fromSwiftModule: false))
XCTAssertEqual(graph.majorVersion, 1)
XCTAssertEqual(graph.minorVersion, 0)
graph.verify()
Expand Down