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

Hello, Swift community!

The review of SE-0401: Remove Actor Isolation Inference caused by Property Wrappers begins now and runs through June 30th, 2023.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager by email or DM. When contacting the review manager directly, please put "SE-0401" in the subject line.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/main/process.md

Thank you,

Holly Borla
Review Manager

33 Likes

Unconditional unequivocal 100% thumbs-up in every way. The problem is well-explained, definitely needs addressing, is an incontrovertible fit for a Swift language which can be reasoned about, and for good measure, the proposal is so clearly and concisely constructed that an in-depth study hardly takes any longer than a quick reading would.

(In case it wasn't clear, I'm in favor of this proposal :grin:)

8 Likes

Im 100% in favor of this.

The current behaviour is very unexpected.

The proposal is very clear and simple. Big +1 from me as this behaviour has been unexpected by me and my whole team.

+1. The current behaviour is completely baffling, and makes it significantly harder to reason about actor isolation in SwiftUI code than it should be, since you cannot simply see whether a view is @MainActor isolated just by reading its code.

+1. The existing behavior leaves an impression that View itself is @MainActor isolated, which is not the case. It will be much clearer to ask the user to add @MainActor explicitly, just as it works for plain properties of MainActor isolated types.

1 Like

+1 great change :)

Another +1 here, good improvement.

+1, although:

It's not clear why this upward inference based on property wrappers was initially proposed. No discussion of that aspect was found during the Global Actors review. We can speculate that it may have been intended to make it easier to interact with SwiftUI's @ObservedObject when that first rolled out. But it's not clear it actually makes anything significantly easier; it only saves us from writing a single annotation on the type, and the loss of that annotation introduces violations of the principle of least surprise.

I find it strange -- this proposal is suggesting that we remove a language feature, yet the reason for the feature's existence remains "not clear" and left to speculation. Even though I find given explanation to be perfectly plausible, we really should have this confirmed else there is a danger that we may be overlooking something.

Perhaps @John_McCall or @Douglas_Gregor, as authors of the global actors proposal, can help shed light on why the rule was introduced, so we won't need to speculate.

5 Likes

Oh, sorry for not addressing this question more specifically early; I hadn't realized it was such an open question. This part of the global actors proposal came from the observation[*] that the presence of @ObservedObject, or other property wrappers that had to be used from the main actor, were a strong indication that the enclosing type itself could only reasonably be used from the main actor, and the general desire to lessen the @MainActor annotation burden.

Doug

[*] Hah. Hah.

11 Likes

Something that wasn’t obvious to me from a quick reading of the text: In the absence of this inference, what does happen now when a property wrapper’s wrappedValue method is actor-isolated? Does the compiler isolate just that property to the actor? Do you have apply an attribute somewhere by hand?

I think I support the idea here—I’d just like to make sure we’re on the same page about how this change will affect the overall behavior of the language.

Ah, good point. Yes, the individual property itself is still isolated, just as if you had annotated that property. No additional attribute needed.

2 Likes

+1. The inference in question is unexpected, I'm happy for this proposal to remove it.

I think it's a good proposal. I implemented the inference rule that's proposed to be removed, and my initial intuition turned out to be quite wrong. Removing this inference rule is the right path forward.

Yes. The inference rule doesn't help much code, but it causes a surprising amount of confusion. We're better off without it.

We did some extra compatibility testing of this change, and our results weren't quite as rosy as what's presented in the proposal: we hit a handful of places where removing this inference rule caused well-formed code to break. That's neither a surprise nor a problem---it's just confirmation that the inference rule is in use in real code, so staging this change in via an upcoming feature flag and Swift 6 mode is the right approach.

Doug

10 Likes

+1

What is your evaluation of the proposal?
It makes sense.

Is it significant enough to warrant a change to Swift?
Yes, the current behavior is non-obvious.

Does the proposal fit well with the feel and direction of Swift?
Yes, the current behavior does not.

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
    I have not.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
    A quick reading.

Could you expand on how the code was broken in the examples you saw? I read the Alternatives Considered section and saw that the idea of adding a warning was discarded. I’m slightly concerned that users might have their code broken and not understand what they have to do to fix it (e.g. add @MainActor to the type).

The decision was to produce errors when this feature is enabled (either in Swift 6 mode, or with the "upcoming feature" flag). I considered also adding warnings before the flag was enabled, to let users know that the code would become invalid in the future. However, that warning isn't actionable in all cases, and would likely be overly noisy in other cases. No behavior is changing for current users, so the lack of a warning won't hide any broken code.

I did some compatibility testing while drafting the proposal and didn't find any cases where enabling the feature actually caused a source breakage. That could have been an argument for enabling the feature in Swift 5 mode, without the "upcoming feature" flag. I believe Doug is saying that they tried out enabling the feature on existing code bases and did indeed find cases where the compiler produced such errors, so it wouldn't be feasible to enable this feature for all users prior to Swift 6. I'm not surprised that further testing did indeed find source incompatibility; I just didn't run into it during my limited sample.

If users are actually affected by this change, an error will be product that indicates what they need to do to fix it.

3 Likes

Oh I see, it makes sense then. Thanks for clearing that up!

  • What is your evaluation of the proposal?

+1

I’ve had to explain the discrepancies between the inference results from @State and @StateObject. Would love to not have to do that again :slight_smile:

+1