Skip to content

Add the missing StrictConcurrency upcoming feature flag #65993

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

Closed
wants to merge 1 commit into from
Closed

Add the missing StrictConcurrency upcoming feature flag #65993

wants to merge 1 commit into from

Conversation

gwynne
Copy link
Contributor

@gwynne gwynne commented May 18, 2023

SE-0362 § Proposals define their own feature identifier claims that an upcoming feature flag named StrictConcurrency exists and is equivalent to the -strict-concurrency=complete compiler flag. However, such a feature flag does not currently exist.

This is a problem for package maintainers attempting to add Sendable correctness to their own packages. Enabling full checking currently means either always building from the command line with -Xswiftc -strict-concurrency=complete setting manually specified or adding it to the swiftSettings for the appropriate target(s) in the package manifest. The former is easily forgotten, and the latter makes the package unusable as a dependency due to the use of "unsafe" flags. Allowing the use of .enableUpcomingFeature("StrictConcurrency") instead solves both issues.

As such, this PR adds the missing upcoming feature. As "unknown" upcoming feature names are silently ignored by the compiler, there are no backwards compatibility concerns.

@ktoso ktoso requested a review from DougGregor May 18, 2023 09:05
@ktoso ktoso added the concurrency Feature: umbrella label for concurrency language features label May 18, 2023
@ktoso
Copy link
Contributor

ktoso commented May 18, 2023

@swift-ci please smoke test

@FranzBusch
Copy link
Member

+1 from me on this. I would love to enable this experimental feature on all our Swift on Server libraries to avoid regressing here.

@ktoso
Copy link
Contributor

ktoso commented May 18, 2023

Yeah additional context here is that this was discussed in the SSWG and it seems to be a missing feature. I'm trying to get more clarity on this and see if that's the right fix or something else should be done.

Thank you for the PR @gwynne

@hborla
Copy link
Member

hborla commented May 19, 2023

The absence of an upcoming feature flag for StrictConcurrency is deliberate at the moment. There are still several holes in the model for strict checking. The intention is to add a StrictConcurrency flag once those holes are filled.

@DougGregor
Copy link
Member

What I'm trying to avoid here is having StrictConcurrency become an upcoming feature and then change behavior, e.g., because make the checking more restrict (e.g., as a bug for or deliberate change). Upcoming features need to be in the form that they would have in Swift 6, and strict concurrency isn't there yet.

@FranzBusch
Copy link
Member

Can we make it an experimental feature then? Just making opt-in easier would go a long way even if it is only used in CI builds.

@gwynne
Copy link
Contributor Author

gwynne commented May 30, 2023

@FranzBusch I went to try to make it an experimental feature then, but I saw that I have to mark whether or not it's "available in production"; if I don't, it won't work in release builds and thus doesn't solve the problem for package maintainers that this PR was created to address. If I do, we'll run afoul of this notation in the code when we later want to it an upcoming feature: // setting `AvailableInProd` to `true` on a feature means that the flag cannot be dropped in the future.

I'm open to suggestions on this one.

@KeithBauerANZ
Copy link

We need something like this to exist in the short term (ideally 5.9!); we need to be able to enforce the strictest available concurrency checking and still have the package consumable by others.

It could simply be allowed at the SPM level — have a SwiftSetting like .strictConcurrency(.complete) for example that maps onto the -strict-concurrency argument to the compiler, and errors if swift version > 5? Effectively just white-listing that flag as "safe" for consumers?

Or just use a different name and an experimental feature here that won't clash with the future, so we can .enableExperimentalFeature("StrictConcurrencyComplete")?

@xedin xedin removed their request for review June 13, 2023 17:30
@DougGregor
Copy link
Member

@FranzBusch I went to try to make it an experimental feature then, but I saw that I have to mark whether or not it's "available in production"; if I don't, it won't work in release builds and thus doesn't solve the problem for package maintainers that this PR was created to address. If I do, we'll run afoul of this notation in the code when we later want to it an upcoming feature: // setting `AvailableInProd` to `true` on a feature means that the flag cannot be dropped in the future.

I'm open to suggestions on this one.

It's fine to make it an experimental feature that's available in production. And we can probably extend the grammar a bit to deal with StrictConcurrency=targeted.

@DougGregor
Copy link
Member

I'll take a whack at this

@gwynne
Copy link
Contributor Author

gwynne commented Jun 28, 2023

Superseded by #66991, thanks @DougGregor and @FranzBusch!

@gwynne gwynne closed this Jun 28, 2023
@gwynne gwynne deleted the gwynne-add-strictconcurrency-feature branch June 28, 2023 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concurrency Feature: umbrella label for concurrency language features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants