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

[Swift 6] Compilation issues when migrating to Swift 6 #2279

Open
trevoranderson opened this issue Oct 17, 2024 · 6 comments
Open

[Swift 6] Compilation issues when migrating to Swift 6 #2279

trevoranderson opened this issue Oct 17, 2024 · 6 comments

Comments

@trevoranderson
Copy link

Similar to #2274 but adding a few more from my project. This is all tested by setting SWIFT_STRICT_CONCURRENCY = complete in the project's build settings in XCode. Most of the issues are on the class that Swift passes into Rust to get notified about certain events. I believe it's called Foreign traits in the docs.

Foreign Trait Swift 6 concurrency issues

private enum UniffiCallbackInterfaceRustEventHandler {
    // Create the VTable using a series of closures.
    // Swift automatically converts these into C callback functions.
    static var vtable: UniffiVTableCallbackInterfaceRustEventHandler = .init(

gives the error

Static property 'vtable' is not concurrency-safe because it is nonisolated global shared mutable state; this is an error in the Swift 6 language mode

another location

public struct FfiConverterTypeRustEventHandler: FfiConverter {
    fileprivate static var handleMap = UniffiHandleMap<RustEventHandler>()

gives the error

Static property 'handleMap' is not concurrency-safe because it is nonisolated global shared mutable state; this is an error in the Swift 6 language mode

Initialization Result Swift 6 concurrency issues

This last one is perhaps more important because it seems like it will affect all Rust+Swift users. The comment seems to disagree with the compiler. (This one might be fixable by switching var to let)

// Use a global variable to perform the versioning checks. Swift ensures that
// the code inside is only computed once.
private var initializationResult: InitializationResult = {

produces error

Var 'initializationResult' is not concurrency-safe because it is nonisolated global shared mutable state; this is an error in the Swift 6 language mode

Fixing

In all cases, the trivial fix is to mark the relevant locations as nonisolated(unsafe) var, which should immediately solve the warnings and allow users to migrate to Swift6 while a more "Swifty" solution is designed. The InitializationResult one looks like just a one-liner bug

@CyonAlexRDX
Copy link

Relevant: #2046

@trevoranderson I agree that it would be good with the nonisolated(unsafe) as a temporary hack.

By longer term plan is probably to put the vtable in an actor, and let it be GlobalActor, should probably "inherit" MainActor for now, so that ALL UniFFI code need not be marked await... but probably further down the line it would be good to be able to control this from the Rust code, choosing to opt in to non mainactor for big uniffi::Record, where it actually can take > 10ms, maybe even a couple of hundered MS to cross the UniFFI boundary, which is ~ slow a network call.

@bendk
Copy link
Contributor

bendk commented Oct 29, 2024

I believe the way to reproduce this is to add -strict-concurrency=complete as an argument here.

It looks like I get the same errors on swift 5.10.1 with that argument, although the messages are not as nice.

@heckj
Copy link
Contributor

heckj commented Oct 30, 2024

Found this issue after recently hitting it myself. While there's more I'm sure I'm not seeing, would a PR changing that hosted initialization from a var to a let be acceptable, as that would - specifically - solve the one place where I'm hitting this.

@mhammond
Copy link
Member

PR's certainly welcome, and a trivial change like you mention would be a no-brainer IMO. Other changes might stretch the limits of our knowledge so might take more time to evaluate, but we certainly welcome them!

@heckj
Copy link
Contributor

heckj commented Oct 30, 2024

I just opened #2290 to provide the "quick and easy" fix here - switching the var to let

bendk added a commit to bendk/uniffi-rs that referenced this issue Oct 31, 2024
Converted some more vars to lets.  Swift still complains about async
calls.  I believe the next step would be to make all UniFFI objects
conform to `Sendable`.

See mozilla#2279.
bendk added a commit to bendk/uniffi-rs that referenced this issue Nov 1, 2024
Converted some more vars to lets.  Swift still complains about async
calls.  I believe the next step would be to make all UniFFI objects
conform to `Sendable`.

See mozilla#2279.
bendk added a commit to bendk/uniffi-rs that referenced this issue Nov 1, 2024
Converted some more vars to lets.  Swift still complains about async
calls.  I believe the next step would be to make all UniFFI objects
conform to `Sendable`.

Split the `FfiType::Reference` variant into `Reference` and
`MutReference`.  This allows us to define some more variables using
`let` in swift.

See mozilla#2279.
@bendk
Copy link
Contributor

bendk commented Nov 1, 2024

I converted some more variables to let in #2294, I'd love to hear feedback from the Swift devs on the changes. In particular, am I right about the need to create an array in order to get a const pointer? I don't think I can use withUnsafePointer, because that requires creating an in-out pointer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants