Skip to content

[SE-0362] Add enableUpcomingFeature and enableExperimentalFeature Swift settings#5632

Merged
DougGregor merged 3 commits intomainfrom
future-and-experimental-build-settings
Dec 2, 2022
Merged

[SE-0362] Add enableUpcomingFeature and enableExperimentalFeature Swift settings#5632
DougGregor merged 3 commits intomainfrom
future-and-experimental-build-settings

Conversation

@DougGregor
Copy link
Copy Markdown
Member

@DougGregor DougGregor commented Jun 28, 2022

@DougGregor
Copy link
Copy Markdown
Member Author

@swift-ci please test

@tomerd
Copy link
Copy Markdown
Contributor

tomerd commented Jun 28, 2022

@swift-ci please smoke test

@DougGregor
Copy link
Copy Markdown
Member Author

I believe from the code that these flags are being treat as "unsafe", which we didn't want for enableFutureFeature. But it seems rather invasive to change.

@DougGregor
Copy link
Copy Markdown
Member Author

@swift-ci please test

@DougGregor
Copy link
Copy Markdown
Member Author

@swift-ci please smoke test

@abertelrud
Copy link
Copy Markdown
Contributor

Will adding the assignments to PackageBuilder automatically get them added to the PIF generated when using --build-system xcbuild, or would that need to be added separately? From my reading that should work but it's hard to tell.

/// - name: The name of the future feature, e.g., ConciseMagicFile.
/// - condition: A condition that restricts the application of the build
/// setting.
@available(_PackageDescription, introduced: 5.7)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These should be 999.0, since this won't make 5.7

Comment on lines +952 to +1003
values = _values.precedeElements(
with: "-enable-experimental-feature")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think precedeElements(with:) is needed, if the new APIs are implemented like the existing .define API, which only stores a single value.

case .enableExperimentalFeature(let value):
    values = ["-enable-experimental-feature", value]
    switch setting.tool {

@hassila
Copy link
Copy Markdown
Contributor

hassila commented Nov 30, 2022

Just a friendly ping: Out of curiosity, is this going to be part of 5.8? We plan to prepare all our repos for Swift 6 features now and try to decide whether to wait for a 5.x with this nice feature enabled or whether to use the various other mechanisms to enable them.

@DougGregor
Copy link
Copy Markdown
Member Author

Thanks for the ping! Yes, I plan to get this into 5.8.

@DougGregor
Copy link
Copy Markdown
Member Author

(I still need to rename "future" to "upcoming", per the proposal's review discussion)

@hassila
Copy link
Copy Markdown
Contributor

hassila commented Nov 30, 2022

Super, thanks! Then we'll look forward to testing it out and plan accordingly!

@DougGregor DougGregor force-pushed the future-and-experimental-build-settings branch from b1d126e to 0840562 Compare December 1, 2022 19:37
@DougGregor DougGregor changed the title Add enableFutureFeature and enableExperimentalFeature Swift settings Add enableUpcomingFeature and enableExperimentalFeature Swift settings Dec 1, 2022
@DougGregor DougGregor changed the title Add enableUpcomingFeature and enableExperimentalFeature Swift settings [SE-0362] Add enableUpcomingFeature and enableExperimentalFeature Swift settings Dec 1, 2022
@DougGregor
Copy link
Copy Markdown
Member Author

@swift-ci please test

@neonichu
Copy link
Copy Markdown
Contributor

neonichu commented Dec 1, 2022

@swift-ci please smoke test

@DougGregor
Copy link
Copy Markdown
Member Author

@neonichu how's my implementation look?

@DougGregor DougGregor merged commit ef2934d into main Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants