Enable more flags to check for concurrency compatibility

[finestructure]

Discussed in #3442

^{Originally posted by mattmassicotte October 17, 2024}
Hello friends! There are two additional flags that, while technically can be source-incompatible, can also have a meaningfully-positive impact on warnings produced by strict concurrency.

InferSendableFromCaptures: https://github.com/swiftlang/swift-evolution/blob/main/proposals/0418-inferring-sendable-for-methods.md
GlobalActorIsolatedTypesUsability: https://github.com/swiftlang/swift-evolution/blob/main/proposals/0434-global-actor-isolated-types-usability.md

I've had to use both of these to address warnings that I would otherwise not be able to fix. I think it may make sense to consider enabling these as well. Thoughts?

[daveverwer]

I'm not sure how much of an impact these will have on the totals but we should make sure to mark the date when these flags went into effect on the Ready for Swift 6 charts in case their impact is significant.

[finestructure]

I've tried applying these flags with Xcode 16.1 but I'm struggling to find a variant that works with SwiftPM builds:

🖥️ env DEVELOPER_DIR=/Applications/Xcode-16.1.0.app xcrun swift build --arch arm64 -Xswiftc -Xfrontend -Xswiftc -stats-output-dir -Xswiftc -Xfrontend -Xswiftc .stats -Xswiftc -strict-concurrency=complete -Xswiftc -enable-upcoming-feature=InferSendableFromCaptures -Xswiftc -enable-upcoming-feature=GlobalActorIsolatedTypesUsability
Building for debugging...
[0/3] Write sources
[1/3] Copying Documentation.docc
[2/3] Write swift-version--7754E27361AE5C74.txt
error: unknown argument: '-enable-upcoming-feature=InferSendableFromCaptures'

I've tried all manner of variations:

• with/without -Xswiftc
• with both -enable-... and --enable-...
• with both ...-feature=XXX and ...-feature XXX

It does appear to be working when setting OTHER_SWIFT_FLAGS for the xcodebuild builds.

[finestructure]

cc @mattmassicotte

[mattmassicotte]

hmmm. How about this:

-Xswiftc -enable-upcoming-feature -Xswiftc GlobalActorIsolatedTypesUsability`

[finestructure]

Ah, of course, it's a -Xswiftc per flag! That works, thanks a lot for the pointer.

We're currently reprocessing with 16.1 so these new flags won't land straight away (so we don't mix results). It'll go live later this week.

[finestructure]

So I have this in place and passing our builder tests but I'm having second thoughts whether we should run with these flags.

We're already seeing discrepancies between what we report and what users see and these custom flags will make it even harder for authors to compare their local results with what we show. Enabling complete concurrency checking or running in language mode 6 are well enough documented but setting upcoming feature flags is trickier.

While this would presumably help with the error and compatible package counts, shouldn't we be testing as plain a build config as possible?

To you point, @mattmassicotte

I've had to use both of these to address warnings that I would otherwise not be able to fix. I think it may make sense to consider enabling these as well. Thoughts?

On reflection, I don't think we want to achieve the minimum number of warnings, we want to show the number of warnings you're getting when you run the stock compiler with concurrency warnings enabled.

This may guide the decision on defaults but it can't do so if we part with the defaults.

[daveverwer]

While this would presumably help with the error and compatible package counts, shouldn't we be testing as plain a build config as possible?

I agree wholeheartedly with this 👍

[mattmassicotte]

Makes total sense! It's true that the only way to get a valid picture of Swift 6 mode compatibiilty is by running with Swift 6 mode. That's not the same thing as Swift 5 + StrictConcurrency, but that may not be what you want. Totally trust your judgement here!

[finestructure]

the only way to get a valid picture of Swift 6 mode compatibiilty is by running with Swift 6 mode

We run in Swift 5 mode with -strict-concurrency=complete because it's the only way to get stats for concurrency warnings. It's an attempt at running as close as possible to Swift 6 mode with something like "continue build on error".

If we ran in Swift 6 mode we could only really report a boolean "worked" / "didn't work" and no error or warning count, unfortunately.

[mattmassicotte]

This is not as close as possible to Swift 6 mode though! Swift 6 mode is, roughly, all flags on + concurrency warnings become errors.

But, I totally get that these things are in tension. If you turn on the ~ 20 upcoming feature flags, you are almost at Swift 6 mode while preserving warnings. But you also have the most annoying-to-reproduce build set up. I think keeping things simple is very desirable though.

[finestructure]

I actually almost raised that question yesterday, if including these flags is closer to mode 6 than just mode 5 + -strict-concurrency=complete. So you're saying we'd have to pass in 20 additional flags in this awkward -Xswiftc -enable-upcoming-feature -Xswiftc X style...? 😵‍💫

One of the questions is what users are more likely interested in:

• an issue count if the package was compiled with language mode 6
• an issue count if the package was compiled with language mode 5 + complete concurrency checking

When planning the feature we assumed these two to be the same but if that's not the case, we should revisit that assumption.

A few more observations/thoughts:

• We'd be changing a setting while we're gathering data (earlier data points were gathered with different settings). We can highlight this in the plot.
• Unless we add the flags we're unlikely to actually see any changes to the issue count from the compiler side. Language mode 5 + complete concurrency is not going to materially change going forward, so we'd only be measuring package authors' efforts.
• I wish there was an easier way to opt into mode 6 that captures all issues other then running mode 5 + all the flags. The reason we don't want to run with mode 6 is that it will stop at the first error and not give us a complete picture. We lack a mode 6 + errors as warnings.
• If we adopted the flags, how would we ensure to keep up to date if there are more flags to be added?

I think we may want mode 5 + all the flags, awkward as it is. It's about language mode 6 and mode + complete concurrency doesn't seem to cover it well enough.

[daveverwer]

One of the questions is what users are more likely interested in:

• an issue count if the package was compiled with language mode 6
• an issue count if the package was compiled with language mode 5 + complete concurrency checking

The answer to this is clear to me. Swift 5 mode + extra concurrency warnings is no longer a real world situation now that Swift 6 is released.

Developers want the results as if Swift 6 mode were switched on.

It's truly a shame that we can't get the error numbers in that situation, but in my opinion that's clearly what we're aiming for.

[mattmassicotte]

You are correct. The Swift 6 language mode is (pretty much) defined as Swift 5 + ALL upcoming feature flags. So, turning on any more flags gets you closer to 6 mode. I just specifically suggested these particular flags because they can have a meaningful impact on concurrency-specific warnings.

There's no way I know of to go in the opposite direction and only downgrade errors to warnings in 6 mode.

[finestructure]

Just to record a link here as well, there might be a resource to track what flags we should be using in the making here: https://toot.iamkonstantin.eu/@konstantin/113464415244283221

[finestructure]

Also cc-ing @hborla for awareness/review of the idea to include additional build flags.