-
Notifications
You must be signed in to change notification settings - Fork 205
Add -coverage-prefix-map arg #128
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
@@ -296,6 +296,7 @@ public struct Driver { | |||
self.numParallelJobs = Self.determineNumParallelJobs(&parsedOptions, diagnosticsEngine: diagnosticEngine, env: env) | |||
|
|||
try Self.validateWarningControlArgs(&parsedOptions) | |||
Self.validateCoverageArgs(&parsedOptions, diagnosticsEngine: diagnosticEngine) |
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.
I didn't love creating a new function for this but there wasn't another obvious place to put it
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.
Your comment made me think about an alternative approach. validateCoverageArgs
could be parseCoverageArgs
that parses the args, diagnoses failures, and then returns an array of (original, remapped)
pairs. Then, we render that array to arguments in addCommonFrontendOptions
. What you do you think about that approach?
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.
(I'm open to make any changes like that) Gut feeling is it might not be worth adding that much processing to these options considering we don't even handle them, it feels like it could increase the surface area for bugs around this
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.
Sure. I'm fine with treating this as a "validate" call like you have it, but wanted to float the idea in case you preferred this alternative design. I don't feel strongly about it.
@@ -31,6 +31,7 @@ extension Option { | |||
public static let colorDiagnostics: Option = Option("-color-diagnostics", .flag, attributes: [.frontend, .doesNotAffectIncrementalBuild], helpText: "Print diagnostics in color") | |||
public static let compileModuleFromInterface: Option = Option("-compile-module-from-interface", .flag, attributes: [.helpHidden, .frontend, .noDriver], helpText: "Treat the (single) input as a swiftinterface and produce a module", group: .modes) | |||
public static let continueBuildingAfterErrors: Option = Option("-continue-building-after-errors", .flag, attributes: [.frontend, .doesNotAffectIncrementalBuild], helpText: "Continue building, even after errors are encountered") | |||
public static let coveragePrefixMap: Option = Option("-coverage-prefix-map", .separate, attributes: [.frontend], helpText: "Remap source paths in coverage info") |
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.
I did this manually because I couldn't get the option generation working, but I think I got the right stuff here
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.
Looks fine. We can do a separate regeneration pass.
This is analogous to this change in Driver.cpp swiftlang/swift#32175
53e4604
to
58efe70
Compare
@swift-ci please test |
This is analogous to this change in Driver.cpp swiftlang/swift#32175