Skip to content

[embedded] A number of changes to support embedded Swift. #1455

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
merged 4 commits into from
Oct 2, 2023
Merged

[embedded] A number of changes to support embedded Swift. #1455

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6e8ef82
[embeded] A number of changes to support embeded Swift.
zoecarver Sep 26, 2023
ba66fdc
[embedded] fix test and address review comments
zoecarver Oct 2, 2023
9279287
[embedded] revert test changes (just compare strings).
zoecarver Oct 2, 2023
c2a8868
[embedded] Check that the messages match options exactly; use `@testa…
zoecarver Oct 2, 2023
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
19 changes: 15 additions & 4 deletions Sources/SwiftDriver/Jobs/CompileJob.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,13 +363,24 @@ extension Driver {
commandLine.appendFlag(map)
}

let expirementalFeatures = parsedOptions.arguments(for: .enableExperimentalFeature)
let embeddedEnabled = expirementalFeatures.map(\.argument).map(\.asSingle).contains("Embedded")

try commandLine.appendLast(.trackSystemDependencies, from: &parsedOptions)
try commandLine.appendLast(.CrossModuleOptimization, from: &parsedOptions)
try commandLine.appendLast(.ExperimentalPerformanceAnnotations, from: &parsedOptions)
try commandLine.appendLast(.disableAutolinkingRuntimeCompatibility, from: &parsedOptions)
try commandLine.appendLast(.runtimeCompatibilityVersion, from: &parsedOptions)
try commandLine.appendLast(.disableAutolinkingRuntimeCompatibilityDynamicReplacements, from: &parsedOptions)
try commandLine.appendLast(.disableAutolinkingRuntimeCompatibilityConcurrency, from: &parsedOptions)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should drop this change as we probably still want to let users configure these options manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CC @kubamracek

Copy link
Contributor

Choose a reason for hiding this comment

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

All we need is to make sure these flags don't passed by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sort of feel like we should pass -runtime-compatibility-version none by default. But this seems to work 🤷


if !embeddedEnabled {
try commandLine.appendLast(.disableAutolinkingRuntimeCompatibility, from: &parsedOptions)
try commandLine.appendLast(.runtimeCompatibilityVersion, from: &parsedOptions)
try commandLine.appendLast(.disableAutolinkingRuntimeCompatibilityDynamicReplacements, from: &parsedOptions)
try commandLine.appendLast(.disableAutolinkingRuntimeCompatibilityConcurrency, from: &parsedOptions)
} else {
try commandLine.appendFlag(.disableAutolinkingRuntimeCompatibility)
try commandLine.appendFlag(.disableAutolinkingRuntimeCompatibilityDynamicReplacements)
try commandLine.appendFlag(.disableAutolinkingRuntimeCompatibilityConcurrency)
}

try commandLine.appendLast(.checkApiAvailabilityOnly, from: &parsedOptions)

try addCommonSymbolGraphOptions(commandLine: &commandLine,
Expand Down
21 changes: 20 additions & 1 deletion Sources/SwiftDriver/Jobs/FrontendJobHelpers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,11 @@ extension Driver {
commandLine.appendFlag("-aarch64-use-tbi")
}

let expirementalFeatures = parsedOptions.arguments(for: .enableExperimentalFeature)
let embeddedEnabled = expirementalFeatures.map(\.argument).map(\.asSingle).contains("Embedded")

// Enable or disable ObjC interop appropriately for the platform
if targetTriple.isDarwin {
if targetTriple.isDarwin && !embeddedEnabled {
commandLine.appendFlag(.enableObjcInterop)
} else {
commandLine.appendFlag(.disableObjcInterop)
Expand All @@ -122,6 +125,22 @@ extension Driver {
commandLine.appendFlag("-stdlib=\(stdlibVariant)")
}

if embeddedEnabled && parsedOptions.hasArgument(.enableLibraryEvolution) {
diagnosticEngine.emit(.error_no_library_evolution_embedded)
throw ErrorDiagnostics.emitted
}

if embeddedEnabled &&
(!parsedOptions.hasArgument(.wmo) || !parsedOptions.hasArgument(.wholeModuleOptimization)) {
diagnosticEngine.emit(.error_need_wmo_embedded)
throw ErrorDiagnostics.emitted
}

if embeddedEnabled && parsedOptions.hasArgument(.enableObjcInterop) {
diagnosticEngine.emit(.error_no_objc_interop_embedded)
throw ErrorDiagnostics.emitted
}

// Handle the CPU and its preferences.
try commandLine.appendLast(.targetCpu, from: &parsedOptions)

Expand Down
33 changes: 19 additions & 14 deletions Sources/SwiftDriver/Jobs/Toolchain+LinkerSupport.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,21 +117,26 @@ extension DarwinToolchain {
}
}

for compatibilityLib in targetInfo.target.compatibilityLibraries {
let shouldLink: Bool
switch compatibilityLib.filter {
case .all:
shouldLink = true
break

case .executable:
shouldLink = linkerOutputType == .executable
}
let expirementalFeatures = parsedOptions.arguments(for: .enableExperimentalFeature)
let embeddedEnabled = expirementalFeatures.map(\.argument).map(\.asSingle).contains("Embedded")

if !embeddedEnabled {
for compatibilityLib in targetInfo.target.compatibilityLibraries {
let shouldLink: Bool
switch compatibilityLib.filter {
case .all:
shouldLink = true
break

case .executable:
shouldLink = linkerOutputType == .executable
}

if shouldLink {
// Old frontends don't set forceLoad at all; assume it's true in that case
try addArgsForBackDeployLib("lib" + compatibilityLib.libraryName + ".a",
forceLoad: compatibilityLib.forceLoad ?? true)
if shouldLink {
// Old frontends don't set forceLoad at all; assume it's true in that case
try addArgsForBackDeployLib("lib" + compatibilityLib.libraryName + ".a",
forceLoad: compatibilityLib.forceLoad ?? true)
}
}
}

Expand Down
12 changes: 12 additions & 0 deletions Sources/SwiftDriver/Utilities/Diagnostics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,16 @@ extension Diagnostic.Message {
static func error_expected_frontend_command() -> Diagnostic.Message {
.error("expected a swift frontend command")
}

static var error_no_library_evolution_embedded: Diagnostic.Message {
.error("Library evolution cannot be enabled with embedded Swift.")
}

static var error_need_wmo_embedded: Diagnostic.Message {
.error("Whole module optimization (wmo) must be enabled with embedded Swift.")
}

static var error_no_objc_interop_embedded: Diagnostic.Message {
.error("Objective-C interop cannot be enabled with embedded Swift.")
}
}
30 changes: 30 additions & 0 deletions Tests/SwiftDriverTests/SwiftDriverTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6288,6 +6288,36 @@ final class SwiftDriverTests: XCTestCase {
}
}

func testEmbeddedSwiftOptions() throws {
do {
var driver = try Driver(args: ["swiftc", "-target", "arm64-apple-macosx10.13", "test.swift", "-enable-experimental-feature", "Embedded", "-parse-as-library", "-wmo", "-o", "a.out", "-module-name", "main"])
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 2)
let compileJob = plannedJobs[0]
let linkJob = plannedJobs[1]
XCTAssertTrue(compileJob.commandLine.contains(.flag("-disable-objc-interop")))
XCTAssertFalse(linkJob.commandLine.contains(.flag("-force_load")))
}
do {
let diags = DiagnosticsEngine()
var driver = try Driver(args: ["swiftc", "-target", "arm64-apple-macosx10.13", "test.swift", "-enable-experimental-feature", "Embedded", "-parse-as-library", "-wmo", "-o", "a.out", "-module-name", "main", "-enable-library-evolution"], diagnosticsEngine: diags)
_ = try driver.planBuild()
XCTAssertTrue(diags.diagnostics.first!.message.text == "Library evolution cannot be enabled with embedded Swift.")
} catch _ { }
do {
let diags = DiagnosticsEngine()
var driver = try Driver(args: ["swiftc", "-target", "arm64-apple-macosx10.13", "test.swift", "-enable-experimental-feature", "Embedded", "-parse-as-library", "-o", "a.out", "-module-name", "main"], diagnosticsEngine: diags)
_ = try driver.planBuild()
XCTAssertTrue(diags.diagnostics.first!.message.text == "Whole module optimization (wmo) must be enabled with embedded Swift.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this assert against the actual variable instead of hardcoded text? That way tests won't break if diagnostic text needs to change.

Suggested change
XCTAssertTrue(diags.diagnostics.first!.message.text == "Whole module optimization (wmo) must be enabled with embedded Swift.")
XCTAssertTrue(diags.diagnostics.first!.message.text == error_need_wmo_embedded.message.text)

} catch _ { }
do {
let diags = DiagnosticsEngine()
var driver = try Driver(args: ["swiftc", "-target", "arm64-apple-macosx10.13", "test.swift", "-enable-experimental-feature", "Embedded", "-parse-as-library", "-wmo", "-o", "a.out", "-module-name", "main", "-enable-objc-interop"], diagnosticsEngine: diags)
_ = try driver.planBuild()
XCTAssertTrue(diags.diagnostics.first!.message.text == "Objective-C interop cannot be enabled with embedded Swift.")
} catch _ { }
}

func testVFSOverlay() throws {
do {
var driver = try Driver(args: ["swiftc", "-c", "-vfsoverlay", "overlay.yaml", "foo.swift"])
Expand Down