-
Notifications
You must be signed in to change notification settings - Fork 207
Support reading binary swiftdeps files #190
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own old code for your benefit (but also for you to fix, heh):
let topByteIndex = upperBound >> 3 | ||
var result: UInt64 = 0 | ||
if upperBound & 7 != 0 { | ||
let mask: UInt8 = (1 << UInt8(upperBound & 7)) &- 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cast to UInt8 shouldn't be necessary in modern Swift, which allows mixed-type casts. I don't think that has a performance impact either…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this turned out to be needed so that 7
gets inferred as Int
instead of UInt8
There might still be a simpler way to write this, but I couldn't find it
} | ||
|
||
extension Bitcode { | ||
init(data: Data) throws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One downside of this approach is that it's going to read the entire file structure into memory at once, rather than processing entry by entry. That's probably fine for now, but it'll be something worth cleaning up later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I added a fix to come back and improve this in a follow-up
Sources/SwiftDriver/Incremental Compilation/SourceFileDependencyGraph.swift
Outdated
Show resolved
Hide resolved
Originally authored by Jordan Rose Modified to drop some printing and logging code that's not needed for the driver's use cases Co-authored-by: Jordan Rose <[email protected]>
@swift-ci test |
@swift-ci test |
Input info maps, not output file maps. (I didn't recognize the name because my code called them "build records" or "master dependencies files".) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent! Unless @belkadan has any reservations about merging this, I say go for it!
I only reviewed the parts I wrote originally but they look good to me. :-) |
Merge pull request apple#190 from owenv/binary-swiftdeps
A couple of notes:
Also cc @belkadan who's the original author of the bitstream reader code added in the first commit. I've added you as a commit author, but let me know if you'd like a different form of attribution!