-
Notifications
You must be signed in to change notification settings - Fork 349
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
Enable compilation with Swift 6 for most targets #7311
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 294 files at r1, all commit messages.
Reviewable status: 5 of 294 files reviewed, 1 unresolved discussion (waiting on @buggmagnet)
ios/PacketTunnel/DeviceCheck/DeviceCheckOperation.swift
line 137 at r1 (raw file):
dispatchGroup.enter() let accountTask = remoteService.getAccountData(accountNumber: accountNumber) { result in accountResult = result
I got this waring which is referring to the concurrency checking:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 115 of 294 files at r1, all commit messages.
Reviewable status: 115 of 294 files reviewed, 2 unresolved discussions (waiting on @buggmagnet)
ios/MullvadREST/ApiHandlers/RESTTaskIdentifier.swift
line 13 at r1 (raw file):
extension REST { private static let nslock = NSLock() nonisolated(unsafe) private static var taskCount: UInt32 = 0
Would it make sense to encapsulate this state into an Actor?
db519d8
to
a24fed8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 115 of 294 files reviewed, 2 unresolved discussions (waiting on @acb-mv and @mojganii)
ios/MullvadREST/ApiHandlers/RESTTaskIdentifier.swift
line 13 at r1 (raw file):
Previously, acb-mv wrote…
Would it make sense to encapsulate this state into an Actor?
I believe it would have to be a globally shared actor to achieve the same result, which would then force all network operations to communicate with that actor, which Operation
cannot inherently do since they are not actors.
ios/PacketTunnel/DeviceCheck/DeviceCheckOperation.swift
line 137 at r1 (raw file):
Yes, this file is used by the PacketTunnelProvider
which is still using Swift 5.0 compilation mode, hence the warnings.
However we can silence those warnings.
163772e
to
39d30b7
Compare
This PR enables us to compile most targets with Swift 6.0 except for all the targets that consume NetworkExtensions (duplicated symbols galore and other compilation issues that I want to solve later)
It mostly declares a lot of types as
Sendable
or disables strict concurrency checks in order to compile.We will revisit the app piece by piece over time to remove all those
@unchecked Sendable
since most of the code is already working (as proven by shipping it, and not seeing crashes)This change is