SE-0401: Remove Actor Isolation Inference caused by Property Wrappers

What is your evaluation of the proposal?

+1

The proposal is addressing the issue very clearly. This feature can confuse developers in understanding why a Type is suddenly isolated to a specific globalActor. Removing it and making it more explicit is going to clarify this confusion.

Can you clarify this point? Are you saying that there are scenarios in which a warning (which would imply also any subsequent error) cannot be fixed—i.e., the functionality being proposed for removal here cannot be expressed in any other way?

The warning I was considering in the "Alternatives Considered" section wouldn't be in cases where there would be a future error; it would be for warning users that the default behavior will change in Swift 6, but would not necessarily produce an error. For example, I was considering generating a warning like this:

// Warning: PizzaView is currently MainActor-isolated, because
// it uses @StateObject (a MainActor-isolated property wrapper).
// In Swift 6, types will no longer infer actor isolation based
// on the property wrappers they use, so PizzaView will become
// non-isolated.
//
// FIXIT: Add '@MainActor struct' to preserve the current isolation
//
// FIXIT: Add 'nonisolated struct' to discard the inferred isolation
//        (⚠️ This doesn't work)
struct PizzaView: View {
  @StateObject private var pizzaState = PizzaState()
 
  var body: some View {
    Text("🍕")
  }
}

If users do want to still preserve @MainActor isolation under Swift 6, then they can explicitly declare it as such. This is what I expect many users will do when Swift 6 is released if errors arise from this change.

However, if users actually wanted this struct to be non-isolated, then there's no way for them to do that and silence the warning. The FIXIT above suggests declaring the struct as nonisolated struct ..., but that's not actually valid syntax. You can't declare a type to be explicitly nonisolated, so if users do want the type to be nonisolated, there's no way for them to mark it as such in Swift 5. Thus, they'd just be stuck with a non-actionable warning until Swift 6 ships. At that point, the warning would go away, and the type would be non-isolated. If that change in inferred isolation causes compilation errors, then those errors would appear at that time. But those errors are separate from the warning I was considering here.

We could consider pitching nonisolated struct, of course, but that feels like a separate proposal.

3 Likes

I don't have too much exciting to add in the review but wanted to voice a wholehearted "+1, please!". This behavior was well intended but turned out to be very weird in practice -- it's great that we have a chance now to fix it :+1:

1 Like

Would you be open to making the Upcoming Feature Flag value more concise?
It reads more like a title or a sentence than a flag.

Possibly ‘DisablePropertyWrapperActorInference’?

I'm certainly open to considerations on the name of the flag. One concern I have about DisablePropertyWrapperActorInference is that even with this flag, actor inference still happens from property wrappers; it just stays at the property level. For example, in the following example, there are currently two places where @MainActor is inferred:

/* @MainActor */
struct MyType {
  
  /* @MainActor */  
  @StateObject private var model = Model()

}

Both the property and the containing type currently infer actor isolation. With this flag (or in Swift 6), only the first inference is removed. The property-level actor isolation remains in effect.

I worry that the name DisablePropertyWrapperActorInference might mislead users into thinking that all inference is being removed. The proposed DisableActorInferenceFromPropertyWrapperUsage isn't all that much better, but I hoped that the Usage part might help clarify that it's about types that use property wrappers.

If there's something that's clearer, I'm happy to consider it.

2 Likes

Restrict… instead of Disable…?

1 Like

I see your point about not wanting to mislead users. In general, I think it is difficult to convey nuanced details in a flag.

No matter what the flag is, I think a user who wants to fully understand the change will need to read the proposal or other documentation about the change.

With most existing UFFs, the flag gives a sense of what is being enabled, but the details are not explicit. For example, with ConciseMagicFile or ExistentialAny the flag names alone don’t convey exact details.

I think “Reduced” or “Less” would work better than “Disable” since, as you say, not all inference is disabled.

A flag name like ReducedPropertyWrapperActorInference or LessPropertyWrapperActorInference would state what the feature flag enables without risk of misleading users by trying to convey all the details.

1 Like

To be perfectly clear, does that mean changes like this would still be required?

I’d thought we’d discussed this issue in the past. Adding a property with a different actor-isolated property wrapper would accomplish this goal, no? It isn’t elegant, but it’d be expressible (and since it’s not intuitive, a fix-it may in fact be particularly beneficial).

It's not entirely clear to me what the original problem was, so it's hard for me to answer conclusively. If the type was previously inferring @MainActor because it contained an @ObservableObject property, that inference would have trickled down to the update() method. If nothing else is causing the type to become @MainObject, then update would no longer be main-actor-isolated, and I would think this change should no longer be needed.

But again, without understanding the original problem more completely, it's hard for me to say for sure.

That's true; it technically is possible to express this in Swift 5.9 by adding an additional property wrapper. But the fixit would also have to declare a new global actor somewhere (it can't rely on there being an existing one). That felt like such an invasive change, with such unintuitive (and undocumented) behavior that I didn't think it was worth proposing as a "fix-it".

When we've added such warnings in the past, the purpose was to help users proactively migrate their code to the new spelling, before it was required. (For example, with -warn-concurrency, users can receive warnings about code that will require concurrency annotations in Swift 6.) But adding a spurious secondary property wrapper wouldn't be the desired spelling in Swift 6, so I don't think the compiler should suggest it here.

1 Like

The original problem was that _observableObject was inferred as @MainActor, when only observableObject itself needed to be. I could write nonisolated func update(), but then I couldn't access _observableObject from within that function.

Ah, I see. Under the the new rules proposed here, _observableObject no longer inferred as @MainActor. Here's a test case, running in Swift 5.9:

class Test {

    // Having two different isolated wrappers disables the 
    // actor inference on 'Test' the same way SE-0401 would
    @MainWrapper var x = 1
    @DBWrapper var y = 2

    nonisolated func thing() {
        print(_x)
        print(x)  // 🛑 Main actor-isolated property 'x' can not be referenced from a non-isolated context
        print(_y)
        print(y)  // 🛑 Global actor 'DBActor'-isolated property 'y' can not be referenced from a non-isolated context
    }
}

Because Test no longer infers a global actor, neither _x or _y inherits that inferred actor isolation. We can now access them directly. We still can't access x and y, though, because those are isolated, as expected.

If you remove @DBWrapper, though, then Test infers main actor isolation, and _x is no longer accessible from thing().

1 Like

As for naming the flag, I could be okay with something like DisableUpwardActorInference. But as we're at the end of the review period (or maybe past it?) I'll leave that up to the core team. I don't feel strongly about the name of the flag, so long as it's not misleading.

1 Like

Absolutely, it isn't pretty: a user who wants to opt into the Swift 6 behavior would be better served turning on the feature flag. For completeness, though, I think the following incantation should be enough:

// Given:
@propertyWrapper
struct S<T> {
  @MainActor var wrappedValue: T
}

struct Foo { // @MainActor inferred
  @S var intValue: Int = 0
}

struct Bar { // @MainActor inference disabled
  @S var intValue: Int = 0
  // --- 🤢
  @propertyWrapper private struct _NonisolatedContainingType {
    actor _Actor { }
    @globalActor struct _GlobalActor { static let shared = _Actor() }
    @_GlobalActor var wrappedValue: Void = ()
  }
  @_NonisolatedContainingType private var _placeholder: Void = ()
  // ---
}