-
Notifications
You must be signed in to change notification settings - Fork 199
[Explicit Module Builds][Target Variant] Refactor build planning to allow for a second, separate emit-module task for the target variant #1856
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
base: main
Are you sure you want to change the base?
Conversation
@swift-ci test |
6d351de
to
95c6c6d
Compare
@swift-ci test |
95c6c6d
to
e1c9db6
Compare
@swift-ci test |
e1c9db6
to
16688ae
Compare
@swift-ci test |
@swift-ci test Windows platform |
1 similar comment
@swift-ci test Windows platform |
80d34fb
to
5d7f59a
Compare
@swift-ci test |
@swift-ci test Windows platform |
1 similar comment
@swift-ci test Windows platform |
5d7f59a
to
94c621f
Compare
@swift-ci test |
@swift-ci test Windows platform |
94c621f
to
e47991f
Compare
@swift-ci test |
@swift-ci test Windows platform |
e47991f
to
6010b76
Compare
@swift-ci test |
@swift-ci test Windows platform |
6010b76
to
bdf0ec6
Compare
@swift-ci test Windows platform |
@swift-ci test |
bdf0ec6
to
ae75bc0
Compare
@swift-ci test |
@swift-ci test Windows platform |
ae75bc0
to
8ea30fb
Compare
@swift-ci test |
@swift-ci test Windows platform |
8ea30fb
to
4049481
Compare
@swift-ci test |
@swift-ci test Windows platform |
4049481
to
f8f26ff
Compare
@swift-ci test Linux platform |
8978aee
to
d1b112b
Compare
@swift-ci test |
@swift-ci test Windows platform |
1 similar comment
@swift-ci test Windows platform |
Ping for code review. |
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.
Other than the inline comment, if you are not squashing, MergeModule is not deprecated. It is obsoleted and deleted.
if driver.parsedOptions.contains(.targetVariant), | ||
driver.isFrontendArgSupported(.clangTargetVariant), | ||
let targetVariantTripleStr = frontendTargetInfo.targetVariant?.triple { | ||
var clangTargetVariantTriple: String? = nil |
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.
indentation is really weird 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.
Fixed up best I can. 👍🏼
let sdkPath = frontendTargetInfo.sdkPath?.path | ||
let sdkInfo: DarwinSDKInfo? = sdkPath != nil ? getTargetSDKInfo(sdkPath: sdkPath!) : nil | ||
|
||
// Pass down -clang-target. |
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.
Can we make sure we have test for all -clang-target
cases? I think still missing:
- Only pass
-clang-target
- Not passing
-clang-target-variant
but specify-target-variant
.
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 testClangTargetOptionsExplicit
.
@@ -47,6 +47,9 @@ public enum VirtualPath: Hashable { | |||
/// Standard output | |||
case standardOutput | |||
|
|||
/// ACTODO: Comment |
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.
Fix TODO
if !fileSystem.exists(path.parentDirectory) { | ||
try fileSystem.createDirectory(path.parentDirectory, recursive: true) | ||
} | ||
return try VirtualPath.createBuildProductFileWithKnownContents(path, chainedHeader.content.data(using: .utf8)!).intern() |
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 don't like the force unwrap here.
@@ -15,7 +15,8 @@ extension Driver { | |||
var commandLine: [Job.ArgTemplate] = swiftCompilerPrefixArgs.map { Job.ArgTemplate.flag($0) } | |||
var inputs: [TypedVirtualPath] = [] | |||
|
|||
try addCommonFrontendOptions(commandLine: &commandLine, inputs: &inputs, kind: .repl) | |||
try addCommonFrontendOptions(commandLine: &commandLine, inputs: &inputs, kind: .repl, | |||
explicitModulePlanner: nil) |
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 is just the default parameter.
…llow for a second, separte emit-module task for the target variant This change allows builds that specify a variant triple and which must emit a target variant module to do so from within a single `swiftc` invocation when using Explicit Module Builds. This is an extensive refactor of the build planning procedure with respect to Explicit Module Builds: - Remove 'ExplicitDependencyBuildPlanner' global state from the 'Driver' instance - Instantiate a 'ExplicitDependencyBuildPlanner' instance at the beginning if build planning and propagate/thread this instance to all the required compilation task creation routines, all the way to 'addCommonFrontendOptions' - For '-target-variant' builds which emit a variant triple module, instantiate a whole separate explicit build planner, including performing a secondary dependency scan using the variant triple. Using this information: - Schedule variant triple module pre-compile dependency tasks - Schedule a variant triple PCH compilation task, if necessary - Schedule the variant emit-module task - Schedule the variant textual module verification task, if necessary
…th known contents Similarly to a 'temporaryWithKnownContents', but at an explicitly-determined-by-the-driver 'AbsolutePath'. This is useful for when the driver is able to determine that build planning will produce an input to compilation which must be placed in a specific location. For example, the chained bridging header content and output path are determined during build planning (specifically, dependency scanning) and must be fed into compilation tasks as inputs. This new file type will allow the driver to compute the new file and its contents and then have them get written to disk when resolving input paths to said compilation tasks.
d1b112b
to
91811bb
Compare
@swift-ci test |
@swift-ci test Windows platform |
This change allows builds that specify a variant triple and which must emit a target variant module to do so from within a single
swiftc
invocation when using Explicit Module Builds.This is an extensive refactor of the build planning procedure with respect to Explicit Module Builds:
ExplicitDependencyBuildPlanner
global state from theDriver
instanceExplicitDependencyBuildPlanner
instance at the beginning if build planning and propagate/thread this instance to all the required compilation task creation routines, all the way toaddCommonFrontendOptions
-target-variant
builds which emit a variant triple module, instantiate a whole separate explicit build planner, including performing a secondary dependency scan using the variant triple. Using this information: