Skip to content
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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

buggmagnet
Copy link
Contributor

@buggmagnet buggmagnet commented Dec 10, 2024

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)

⚠️ We should not merge this PR before we move the CI to Xcode 16 because Xcode 15.0 does not support building with Swift 6.0 ⚠️


This change is Reviewable

@buggmagnet buggmagnet added the iOS Issues related to iOS label Dec 10, 2024
@buggmagnet buggmagnet self-assigned this Dec 10, 2024
Copy link

linear bot commented Dec 10, 2024

Copy link
Collaborator

@mojganii mojganii left a 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:
Screenshot 2024-12-11 at 09.32.52.png

Copy link
Contributor

@acb-mv acb-mv left a 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?

@buggmagnet buggmagnet force-pushed the fix-warnings-introduced-by-xcode-16-ios-741 branch from db519d8 to a24fed8 Compare December 11, 2024 11:52
Copy link
Contributor Author

@buggmagnet buggmagnet left a 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):

Previously, mojganii wrote…

I got this waring which is referring to the concurrency checking:
Screenshot 2024-12-11 at 09.32.52.png

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.

@buggmagnet buggmagnet force-pushed the fix-warnings-introduced-by-xcode-16-ios-741 branch from 163772e to 39d30b7 Compare December 13, 2024 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iOS Issues related to iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants