Remove no-longer-needed @preconcurrencys

[MahdiBM]

See apple/swift-openapi-generator#396.

[czechboy0]

@swift-server-bot test this please

[czechboy0]

@MahdiBM Foundation.URL:1:15: note: struct 'URL' does not conform to the 'Sendable' protocol

[czechboy0]

@MahdiBM let me know when it's ready for a re-review/re-CI 🙂

[MahdiBM]

@czechboy0 it should be ready

[czechboy0]

@swift-server-bot test this please

[czechboy0]

@swift-server-bot test this please

[czechboy0]

Hi @MahdiBM, thanks for doing this!

Unfortunately I realized we have a gap in our CI story, and that's testing with the 5.9.0 toolchain. The 5.9 pipeline resolves to 5.9.1, which already has the Sendability improvements in corelibs-foundation, but 5.9.0 doesn't.

We don't want to require 5.9.1 at the package level, because it wouldn't work with the latest released stable Xcode, so we plan to keep 5.9.0 as the minimum required toolchain to build the project.

With that, I opened PRs in all 4 repos to explicitly request 5.9.0 in CI, to ensure we don't accidentally break those users, see, for examples, #84. Once that lands, please update your PRs and hopefully the CI status should reflect whether we can take that change.

It's possible that we simply live with the remarks (they're not warnings, are they?) about unnecessary preconcurrency attributes until 5.10 is released, and then we could conditionalize the imports using an #if check for the compiler version – what do you think?

cc @simonjbeaumont

[MahdiBM]

Hmm right, i did notice that the CI is resolving to 5.9.1.

This PR doesn't require or break anything.
To be honest with the bugs introduced in 5.9.0 due to those major changes, and the bug fixes in 5.9 patch versions, i think it might be more reasonable to assume more people are on 5.9.1+, than on 5.9.
In Docker, most people i've seen just use 5.9 which resolves to 5.9.1 (you were doing the same too) and in Xcode, this wouldn't matter anyway as that's a Darwin platform.
There are some people using Linux for Swift development which might or might not have updated to 5.9.1, but i think overall the situation is on 5.9.1's favor and it's much more accurate to assume Linux environments are using 5.9.1 than 5.9.0.
As far as i understand, this PR will simply make it so some remarks are emitted on 5.9.0, but not emitted on 5.9.1, on Linux only. No breaking changes.

[czechboy0]

I agree that most people probably use 5.9.1, but we need to follow the documented minimum Swift version we claim to support, which is 5.9.0 on all platforms.

Once #84 lands I'll click the Update Branch button on this PR and we'll see what 5.9.0 CI says, if it's all green then of course we can merge it.

[simonjbeaumont]

Yes, until a released (not pre-release) Xcode that uses Swift 5.9.1 has been out for a reasonable amount of time, we must support 5.9.0. However, I'm sympathetic that we should be expecting folks to use the latest patch release for Swift 5.9 available for their platform, which, on Linux, is 5.9.1.

It's a laudable goal to have compile without any warnings remarks on all the supported environments but, IIUC, the only way we'll achieve that is with a more elaborate #if dance:

#if canImport(Darwin)
import Foundation
#else  // canImport(Darwin)
#if swift(>=5.9.1)
import struct Foundation....  // for all the things that have been fixed up in 5.9.1
...
#else // swift(>=5.9.1)
@preconcurrency import struct Foundation....  // for all the things that have been fixed up in 5.9.1
#endif // swift(>=5.9.1)
import struct Foundation....  // for all the things that we can import without @preconcurrency, even on Swift 5.9. 
#endif  // canImport(Darwin)

[czechboy0]

@swift-server-bot test this please

[czechboy0]

So on 5.9.0 we get Foundation.Date:1:15: note: struct 'Date' does not conform to the 'Sendable' protocol.

[MahdiBM]

How should we proceed? I'm fine with dropping the PRs if the #if dance doesn't seem worth it 🙂

[czechboy0]

@MahdiBM Up to you, we'd accept the PR if you can get all the CI to green, but won't have time ourselves to get to this for 1.0 🙂

[MahdiBM]

@czechboy0 why is 5.9.0 CI is having fatal errors? Is it how the "fail on warning" swift compiler flag works? Because the PR should not have resulted in this 🤔

Edit: nevermind i don't think you have that flag enabled.

Another Edit: I see that's enabled in the docker file so my initial suspicion seems correct?

[czechboy0]

Yes we build with warnings-as-errors, and I believe missing a @preconcurrency on an import but using its types as Sendable results in at least a warning. It's asymmetric, as having @preconcurrency without needing it only results in a remark (not a warning) in 5.9 and 5.10 (changed to warning in 5.11/main).

[simonjbeaumont]

Right. This is exactly the reason we put the CI in place because, before, with just a 5.9.1 pipeline it looked fine but could produce warnings, which can be promoted to errors when compiled with warnings-as-errors.

In this project it won't affect adopters since if warnings-as-errors doesn't apply to dependencies. But, for the generator repo, it can impact adopters because generated code will be subject to the compiler flags the adopter chooses and there's no current way to ignore warnings on a per-file basis like in some other languages.

However, for symmetry, and just because we'd like to be warning clean going into 1.0, we'd like to have no warnings on any supported platforms for the repos we maintain.

[czechboy0]

@swift-server-bot test this please

[MahdiBM]

This time it should work for 5.9.0/5.9.1 as i tested locally before pushing

[czechboy0]

@swift-server-bot test this please

[czechboy0]

Thanks @MahdiBM!