Skip to content

Add support for -disable-upcoming-feature and -disable-experimental-feature #1730

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

Merged
merged 1 commit into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 16 additions & 8 deletions Sources/SwiftDriver/Jobs/FrontendJobHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ extension Driver {
case computed
}

/// If the given option is specified but the frontend doesn't support it, throw an error.
func verifyFrontendSupportsOptionIfNecessary(_ option: Option) throws {
if parsedOptions.hasArgument(option) && !isFrontendArgSupported(option) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This is a nice utility, but the name made me do a double-take once I realized it can fail the build.
Not sure I have a better suggestion other than to reference that in Driver.swift we have a couple of instances of verify*Options methods, but those do slightly different things than this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like verify as prefix here better, I'll switch to that.

diagnosticEngine.emit(.error_unsupported_opt_for_frontend(option: option))
throw ErrorDiagnostics.emitted
}
}

/// Add frontend options that are common to different frontend invocations.
mutating func addCommonFrontendOptions(
commandLine: inout [Job.ArgTemplate],
Expand Down Expand Up @@ -154,6 +162,9 @@ extension Driver {
throw ErrorDiagnostics.emitted
}

try verifyFrontendSupportsOptionIfNecessary(.disableUpcomingFeature)
try verifyFrontendSupportsOptionIfNecessary(.disableExperimentalFeature)

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

Expand Down Expand Up @@ -275,14 +286,11 @@ extension Driver {
if isFrontendArgSupported(.compilerAssertions) {
try commandLine.appendLast(.compilerAssertions, from: &parsedOptions)
}
if isFrontendArgSupported(.enableExperimentalFeature) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We're alright dropping isFrontendArgSupported(.enableExperimentalFeature) && isFrontendArgSupported(.enableUpcomingFeature) at this point and rely on them always being supported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my thinking since they've been around for multiple releases at this point. My impression was that we only check for a flag being supported for the interim period where it is conceivable that the a frontend and driver are slightly out of sync on support for a particular flag, and that supporting mixing and matching a new driver with a much older frontend is a non-goal. Optionally dropping feature flags doesn't really seem safe to me, as they have important semantic effects.

try commandLine.appendAll(
.enableExperimentalFeature, from: &parsedOptions)
}
if isFrontendArgSupported(.enableUpcomingFeature) {
try commandLine.appendAll(
.enableUpcomingFeature, from: &parsedOptions)
}
try commandLine.appendAll(.enableExperimentalFeature,
.disableExperimentalFeature,
.enableUpcomingFeature,
.disableUpcomingFeature,
from: &parsedOptions)
try commandLine.appendAll(.moduleAlias, from: &parsedOptions)
if isFrontendArgSupported(.enableBareSlashRegex) {
try commandLine.appendLast(.enableBareSlashRegex, from: &parsedOptions)
Expand Down
4 changes: 4 additions & 0 deletions Sources/SwiftDriver/Utilities/Diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ extension Diagnostic.Message {
.error("unsupported argument '\(argument)' to option '\(option.spelling)'")
}

static func error_unsupported_opt_for_frontend(option: Option) -> Diagnostic.Message {
.error("frontend does not support option '\(option.spelling)'")
}

static func error_option_requires_sanitizer(option: Option) -> Diagnostic.Message {
.error("option '\(option.spelling)' requires a sanitizer to be enabled. Use -sanitize= to enable a sanitizer")
}
Expand Down
4 changes: 4 additions & 0 deletions Sources/SwiftOptions/Options.swift
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ extension Option {
public static let disableDynamicActorIsolation: Option = Option("-disable-dynamic-actor-isolation", .flag, attributes: [.frontend, .doesNotAffectIncrementalBuild], helpText: "Disable dynamic actor isolation checks")
public static let disableEmitGenericClassRoTList: Option = Option("-disable-emit-generic-class-ro_t-list", .flag, attributes: [.helpHidden, .frontend, .noDriver], helpText: "Disable emission of a section with references to class_ro_t of generic class patterns")
public static let disableExperimentalClangImporterDiagnostics: Option = Option("-disable-experimental-clang-importer-diagnostics", .flag, attributes: [.helpHidden, .frontend, .noDriver, .moduleInterface], helpText: "Disable experimental diagnostics when importing C, C++, and Objective-C libraries")
public static let disableExperimentalFeature: Option = Option("-disable-experimental-feature", .separate, attributes: [.frontend, .moduleInterface], helpText: "Disable an experimental feature")
public static let disableExperimentalLifetimeDependenceInference: Option = Option("-disable-experimental-lifetime-dependence-inference", .flag, attributes: [.helpHidden, .frontend, .noDriver], helpText: "Disable experimental lifetime dependence inference")
public static let disableExperimentalOpenedExistentialTypes: Option = Option("-disable-experimental-opened-existential-types", .flag, attributes: [.helpHidden, .frontend, .noDriver], helpText: "Disable experimental support for implicitly opened existentials")
public static let disableExperimentalParserRoundTrip: Option = Option("-disable-experimental-parser-round-trip", .flag, attributes: [.helpHidden, .frontend, .noDriver], helpText: "Disable round trip through the new swift parser")
Expand Down Expand Up @@ -263,6 +264,7 @@ extension Option {
public static let disableThrowsPrediction: Option = Option("-disable-throws-prediction", .flag, attributes: [.helpHidden, .frontend, .noDriver], helpText: "Disables optimization assumption that functions rarely throw errors.")
public static let disableTypeLayouts: Option = Option("-disable-type-layout", .flag, attributes: [.helpHidden, .frontend, .noDriver], helpText: "Disable type layout based lowering")
public static let disableTypoCorrection: Option = Option("-disable-typo-correction", .flag, attributes: [.frontend, .noDriver], helpText: "Disable typo correction")
public static let disableUpcomingFeature: Option = Option("-disable-upcoming-feature", .separate, attributes: [.frontend, .moduleInterface], helpText: "Disable a feature that will be introduced in an upcoming language version")
public static let disableVerifyExclusivity: Option = Option("-disable-verify-exclusivity", .flag, attributes: [.helpHidden, .frontend, .noDriver], helpText: "Disable verification of access markers used to enforce exclusivity.")
public static let disallowForwardingDriver: Option = Option("-disallow-use-new-driver", .flag, helpText: "Disable using new swift-driver")
public static let downgradeTypecheckInterfaceError: Option = Option("-downgrade-typecheck-interface-error", .flag, attributes: [.helpHidden, .frontend, .noDriver], helpText: "Downgrade error to warning when typechecking emitted module interfaces")
Expand Down Expand Up @@ -1087,6 +1089,7 @@ extension Option {
Option.disableDynamicActorIsolation,
Option.disableEmitGenericClassRoTList,
Option.disableExperimentalClangImporterDiagnostics,
Option.disableExperimentalFeature,
Option.disableExperimentalLifetimeDependenceInference,
Option.disableExperimentalOpenedExistentialTypes,
Option.disableExperimentalParserRoundTrip,
Expand Down Expand Up @@ -1166,6 +1169,7 @@ extension Option {
Option.disableThrowsPrediction,
Option.disableTypeLayouts,
Option.disableTypoCorrection,
Option.disableUpcomingFeature,
Option.disableVerifyExclusivity,
Option.disallowForwardingDriver,
Option.downgradeTypecheckInterfaceError,
Expand Down
39 changes: 39 additions & 0 deletions Tests/SwiftDriverTests/SwiftDriverTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8295,6 +8295,45 @@ final class SwiftDriverTests: XCTestCase {
XCTAssertTrue(!jobs[0].commandLine.contains("-emit-irgen"))
}
}

func testEnableFeatures() throws {
do {
let featureArgs = [
"-enable-upcoming-feature", "MemberImportVisibility",
"-enable-experimental-feature", "ParserValidation",
"-enable-upcoming-feature", "ConcisePoundFile",
]
var driver = try Driver(args: ["swiftc", "file.swift"] + featureArgs)
let jobs = try driver.planBuild().removingAutolinkExtractJobs()
XCTAssertEqual(jobs.count, 2)

// Verify that the order of both upcoming and experimental features is preserved.
XCTAssertTrue(jobs[0].commandLine.contains(subsequence: featureArgs.map { Job.ArgTemplate.flag($0) }))
}
}

func testDisableFeatures() throws {
let driver = try Driver(args: ["swiftc", "foo.swift"])
guard driver.isFrontendArgSupported(.disableUpcomingFeature) else {
throw XCTSkip("Skipping: compiler does not support '-disable-upcoming-feature'")
}

do {
let featureArgs = [
"-enable-upcoming-feature", "MemberImportVisibility",
"-disable-upcoming-feature", "MemberImportVisibility",
"-disable-experimental-feature", "ParserValidation",
"-enable-experimental-feature", "ParserValidation",
]

var driver = try Driver(args: ["swiftc", "file.swift"] + featureArgs)
let jobs = try driver.planBuild().removingAutolinkExtractJobs()
XCTAssertEqual(jobs.count, 2)

// Verify that the order of both upcoming and experimental features is preserved.
XCTAssertTrue(jobs[0].commandLine.contains(subsequence: featureArgs.map { Job.ArgTemplate.flag($0) }))
}
}
}

func assertString(
Expand Down