Proper handling of @isolated(any)?

Jon_Shier · 2024-06-11T01:14:04.091Z

In Xcode 16b1, Binding's closure init has been updated to

init(
    get: @escaping @isolated(any) () -> Value,
    set: @escaping @isolated(any) (Value, Transaction) -> Void
)

This produces the warning Reference to captured var 'isFirstEdit' in concurrently-executing code in the following extension:

extension Binding where Value == String {
    func nonEmpty() -> Self {
        var isFirstEdit = true

        return Self {
            (wrappedValue.isEmpty && isFirstEdit) ? " " : wrappedValue
        } set: { newValue, transaction in
            var newValue = newValue

            if isFirstEdit && newValue.first == " " {
                newValue.removeFirst()
                isFirstEdit = false
            }

            self.transaction(transaction).wrappedValue = newValue
        }
    }
}

So two questions.

First, why do these closures need @isolated(any) at all? They're not async, and this usage seemed perfectly safe before, so why the change? This is a rather common patter that will break here.

Second, what's the proper resolution here? According to SE-431, the closures could capture the current isolation context if the pitched closure isolation control language feature existed, but that was never reviewed awaiting an implementation. I can, of course, add explicit isolation to the method with a global actor, but that prevents its composition with other Binding transforms that aren't also so marked. Is there a general solution here?

vns · 2024-06-11T08:02:26.895Z

Jon_Shier:

First, why do these closures need @isolated(any) at all?

My assumption is that it is due to possibility to create Binding from any isolation (since it conforms to Sendable) and then use it in SwiftUI views, so it has to carry initial isolation within it. And here your are potentially have such issue due to nonEmpty can cross isolation.

As you mentioned, the easiest way to mark to be isolated on a global actor. You can also pass isolation dynamically (haven’t checked as no Xcode 16 installed yet, but it should work):

func nonEmpty(isolation: isolated (any Actor)? = #isolation) -> Self

(Edit: made actor parameter an optional, as #isolation can return nil when called from nonisolated contexts, should still be fine)

Jon_Shier · 2024-06-11T13:53:21.841Z

Unfortunately that has no effect. Perhaps the closures can't capture that isolation without the pitched closure isolation control syntax?

vns · 2024-06-11T13:57:51.074Z

Hmm… I thought @isolation(any) was about to address this as well. That’s odd.

hborla · 2024-06-11T15:21:33.334Z

Jon_Shier:

Perhaps the closures can't capture that isolation without the pitched closure isolation control syntax?

@isolated(any) function types capture the value that represents whatever the isolation of the closure is. @isolated(any) has no effect on the inferred isolation of a given closure argument; the default inference rules still apply. However, I believe this Binding initializer uses @_inheritActorContext, just like Task.init and the SwiftUI task modifier, so it does capture the static isolation of the context the binding is formed in.

I believe the reason why adding the isolated parameter like @vns suggested does not resolve the warning is because the function type is also @Sendable. In Xcode 16 Beta 1, @Sendable is implied by @isolated(any) (which was removed per the acceptance notes of SE-0431), but I believe @Sendable is correct for this API, because Binding itself conforms Sendable. Now, if the dynamic isolation is an actor like @MainActor, this code is safe because the closure cannot be invoked multiple times concurrently. The code is only unsafe if the function is dynamically nonisolated. If you never need this functionality for non-isolated bindings, then you can resolve the issue by marking the function with @MainActor*. SE-0434: Usability of global-actor-isolated types (which is to be accepted as soon as I write up a section in the alternatives considered to address some LSG feedback), allows you to capture non-Sendable / mutable values in global-actor-isolated closures because if the closure is isolated, the actor will always serialize calls to the closure, so there's no access to the mutable capture.

*I don't think the implementation of SE-0434 is in Xcode 16 Beta 1. However, it has landed on GitHub on the release/6.0 branch, and this code compiles with no errors in the latest development 6.0 snapshot when compiled with -swift-version 6 against the Xcode 16 Beta 1 SDK:

import SwiftUI

extension Binding where Value == String {
  @MainActor func nonEmpty() -> Self {
    var isFirstEdit = true

    return Self {
      (wrappedValue.isEmpty && isFirstEdit) ? " " : wrappedValue
    } set: { newValue, transaction in
      var newValue = newValue

      if isFirstEdit && newValue.first == " " {
        newValue.removeFirst()
        isFirstEdit = false
      }

      self.transaction(transaction).wrappedValue = newValue
    }
  }
}

Jon_Shier · 2024-06-11T15:28:47.437Z

Unfortunately marking it as @MainActor makes it incompatible with all other Binding extensions which weren't forced to do so, so the chained composition breaks outside of inline usage in a View. Which may be okay, just awkward in other contexts.

tgoyne · 2024-06-11T22:53:01.718Z

I think my main confusion here and uncertainty about how to best fix the warnings is that I'm unclear on to what extent this was intended to be a breaking change.

Is this intended to be a change that only produces warnings for things which were already invalid and just weren't diagnosed before? If that's the case then I'm very deeply confused about something, and the Binding documentation is missing a lot of very important details because it sure appeared to be entirely safe to capture a mutable variable shared between get and set and I've seen a lot of code relying on that. Is this just a straightforward breaking change where all existing apps using Binding is going to break when upgrading to iOS 18 because it's going to start calling the closures concurrently and off the main actor? That seems like an implausibly bold choice. Is this something which would be a breaking change, but SwiftUI in practice isn't actually sending the closures so it works out (or is only sending them if the isolation property is set)? If that's the case, does that mean it's safe to ignore the warning for now?

The third seems like the only plausible one, but I want to make sure I'm not just missing something before I go down an incorrect road trying to fix this.