For better readability and consistency with other annotations in this file, there should be a space between the closing parenthesis of the API_DEPRECATED macro and the NS_SWIFT_UI_ACTOR attribute.

        ios(8.0, 10.0)) NS_SWIFT_UI_ACTOR;

For better readability and consistency with other annotations in this file, there should be a space between the closing parenthesis of the API_DEPRECATED macro and the NS_SWIFT_UI_ACTOR attribute.

        ios(4.0, 10.0)) NS_SWIFT_UI_ACTOR;

For better readability and consistency with other annotations in this file, there should be a space between the closing parenthesis of the API_AVAILABLE macro and the NS_SWIFT_UI_ACTOR attribute.

    API_AVAILABLE(ios(9.0)) NS_SWIFT_UI_ACTOR;

(et format did this I think).

Can you explain why this one is non-sendable?

I believe this PR is still pending design doc review.

Looks like this is only static helpers, so it is confusing to explicitly marking it as either sendable or non-sendable, since Sendability is irrelevant here (we are not creating any instance of it).

A potential solution can be to create a custom no-op annotation (something like FLUTTER_SWIFT_CONCURRENCY_AUDITED_STATIC_UTIL), this would help us distinguish between unaudited annotations vs audited but no-op annotations.

This seems to be a missing case not discussed in the design doc.

A custom macro works but I'm a bit concerned about exposing it, since if you want to use it in public APIs you will have to expose the macro as part of the public API, but the annotation is internal and for dev-purpose-only so it shouldn't pollute the public name space.

Technically this type is still inhabited (it still has init) so I'll change it to Sendable.

Also added a "uninhabited types such as helper classes that only have static methods." to classes that should be made NS_SWIFT_SENDABLE, but the decision is quite arbitrary, I just went with the one that makes the flow easier to write.

As discussed offline, this should be annotated with non-isolated. The concept of "sendability" is only relevant for an instance, and marking static utils as sendable or non-sendable is confusing.

I still prefer not introducing a special case just for static helpers (also since NS_SWIFT_NONISOLATED is usually only used on types when one needs to opt out of @MainActor-isolation, for instance the superclass is @MainActor-isolated).

The class does not have un-protected mutable states so it still makes sense it is marked @Sendable (it's just that nobody will try to "send" it).

Why is this one non-sendable? It doesn't have any properties and only has a static method?

Same comment as above.

Changed to Sendable, same as FlutterHourFormat.

Why not NS_SWIFT_UI_ACTOR?

Things like FlutterImplicitEngineDelegate must be NS_SWIFT_UI_ACTOR. Looking at the code, the delegate method is called by UI code (e.g. FlutterViewController's awakeFromNib, which is main actor isolated.

It's a protocol so I only put NS_SWIFT_UI_ACTOR to the (only) method requirement. It's probably fine to mark the entire protocol NS_SWIFT_UI_ACTOR but only marking the method is the safer bet (so it does not force conforming classes to be @MainActor, less chance of breaking existing code).

Offline sync: Why we have to use UIActor for things like engine implicit protocol? This is because implementor can access main isolated api:

func didInitializeImplicitEngine() {
// this is main isoalted, so you can access those APIs. 
let vc = engine.viewController
vc.view.color = ...
}

There was existing code broken by putting @MainActor on FlutterPlugin. There was this class that conforms to FlutterPlugin and another protocol with a nonisolated method (let's say func method()). Making FlutterPlugin @MainActor broke the code because func method() was required by the other protocol to be nonisolated but conforming to the @MainActor-isolated FlutterPlugin made the default isolation @MainActor. I'm leaning towards changing the design doc to say that always use NS_SWIFT_NONSENDABLE when migrating existing API.

I don't see implicit engine stuff in the link. am i missing anything.

Oh you are referring to FlutterPlugin - why is that plugin Sendable? I think we should remove that if it's incorrect annotation.

Looking closer, it looks like the complaint isn't around FlutterPlugin, but pigeon?

The problem with the internal code is exactly https://github.com/swiftlang/swift-evolution/blob/main/proposals/0316-global-actors.md#global-actor-inference, the concrete type is inferred to have @MainActor isolation because we made FlutterPlugin @MainActor-isolated, but to conform the other protocol some of the methods must be nonisolated (right now since they do not have any isolation specifier, they are @MainActor-isolated by default since the concrete type now has @MainActor-isolation). The breakage can also be fixed by explicitly marking those methods nonisolated but that require changing source code.

Is this the swift bug that you mentioned earlier? I remember you mentioned something related to whether they are inside same file?

Are there any drawback of marking the protocol as non-isolated but all its functions to be isolated (which is a weird practice not done by Apple)? If no drawback, I think we can go ahead with this, but we have to document this behavior nicely.

Why not NS_SWIFT_UI_ACTOR?

Here and below, for protocols, in case the conforming type is actually not @MainActor isolated we should avoid NS_SWIFT_UI_ACTOR, to avoid breaking existing code. There's a

For migrating existing APIs, avoid NS_SWIFT_UI_ACTOR if the protocol is not strictly tied to the @MainActor.

section in the design doc.

Why not NS_SWIFT_UI_ACTOR?

Hmmm, this isn't static, should it be NS_SWIFT_NONISOLATED?

The implementation doesn't use any instance variables so can be made static:

The implementation must be thread-safe as the codec can be used on any thread.

Used on any thread doesn't mean it must be thread safe. There's a difference between "used on any thread", vs "used across multiple thread". The former is actor isolation (or non-isolated in this case), and the latter is Sendability (or sendable in this case)

Ah good catch. Rephrased to "initialized and used on different threads".

Can you add comment of why these attributes are used (like what you did in FlutterMessageCodec).

added to writer/reader/ReaderWriter.

Why is FlutterStandardReader and FlutterStandardWriter non-sendable, but FlutterStandardReaderWriter sendable?

FlutterStandardReaderWriter is basically just a namespace for two factory methods that create readers / writers. It doesn't have mutable states like the real reader / write classes do.

It can be subclassed. And the engine implementation assumes it's thread-safe.

Looks like this is only static helpers, so it is confusing to explicitly marking it as either sendable or non-sendable, since Sendability is irrelevant here (we are not creating any instance of it).

A potential solution can be to create a custom no-op annotation (something like FLUTTER_SWIFT_CONCURRENCY_AUDITED_STATIC_UTIL), this would help us distinguish between unaudited annotations vs audited but no-op annotations.

This seems to be a missing case not discussed in the design doc.

Same comment as above.

Things like FlutterImplicitEngineDelegate must be NS_SWIFT_UI_ACTOR. Looking at the code, the delegate method is called by UI code (e.g. FlutterViewController's awakeFromNib, which is main actor isolated.