-
Notifications
You must be signed in to change notification settings - Fork 204
[Dependency Scanning] Add ability to use the dependency scanner's import-prescan mode #717
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
@swift-ci please test |
1d55311
to
c98af5b
Compare
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.
Thanks! LGTM. There was one place I had to reread, and mentioned a nit there.
// We generate full swiftc -frontend -scan-dependencies invocations in order to also be | ||
// able to launch them as standalone jobs. Frontend's argument parser won't recognize | ||
// -frontend when passed directly. |
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.
Nice "why" comment!
} | ||
|
||
private func sanitizeCommandForLibScanInvocation(_ command: inout [String]) { | ||
// Remove the tool executable to only leave the arguments |
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 comment leaves me wondering what the executable will be?
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.
Or why there doesn't need to be one?
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.
Added an explanation. Thanks for catching.
let moduleVersionedGraphMap: [ModuleDependencyId: [InterModuleDependencyGraph]] | ||
if (!fallbackToFrontend) { | ||
|
||
if !(try initSwiftScanLib()) { |
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.
Nit: Would be clearer to break up:
let isSwiftScanLibAvailable = !try initSwiftScanLib()
if isSwiftScanLibAvailable {
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.
Much clearer, indeed. Fixed.
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.
PS: Someday, I'd like to meet the llvm engineer who decided to use "true" for "hadError" and find out the rationale.
… mode An ability to do a simple import pre-scan was added to the dependency scanner in swiftlang/swift#34028, with a corresponding libSwiftScan API added later. This PR adds the ability to invoke this import pre-scan from the driver.
c98af5b
to
e5d6e9f
Compare
@swift-ci please test |
Thanks, @davidungar! |
PPS: Glad the comments were helpful. |
An ability to do a simple import pre-scan was added to the dependency scanner in swiftlang/swift#34028, with a corresponding libSwiftScan API added later. It parses input files looking for
import
statements and outputs an array of module names imported in the provided input source files.This PR adds the ability to invoke this import pre-scan from the driver.