[Pitch] Amend SE-0466 & SE-0470 to improve isolation inference

Evolution Pitches
1

Hi all,

I've been testing isolated conformance inference in combination with other features, especially default main-actor isolation (which implies InferIsolatedConformances) , and I've found a few places where we probably want to tweak the inference rules.

Conformance to Sendable-inheriting protocols

If a protocol inherits Sendable, the conformance to it cannot have actor isolation because that would admit data races when it is used with generic code:

protocol P: Sendable { }

@MainActor
class C: @MainActor P { } // error: main-actor isolated conformance to 'Sendable' protocol P

We should adjust the conformance isolation inference rules to not introduce an actor-isolated conformance to a Sendable protocol like P:

@MainActor
class C2: P { // currently infers @MainActor conformance to P
              // change to infer nonisolated conformances to P
}

Default actor isolation andSendable-inheriting protocols

In practice, actor-isolated types like C and c@ above are relatively hard to conform to protocols that inherit from Sendable, because most of the members of the actor-isolated type will also be actor-isolated. Consider a class similar to C and C2 that doesn't specify its isolation, e.g.,

protocol Q: Sendable {
  func f()
}

class D: Q {
  func f() { }
}

With the default main actor isolation introduced in SE-0466, this code will produce an error due to the way in which @MainActor is inferred:

  • D is inferred to @MainActor because it isn't specified as nonisolated
  • D.f is inferred to @MainActor because it is part of D
  • The conformance D: Q remains nonisolated because Q is Sendable.
  • There is an error in the non-isolated conformance D: Q because the main-actor-isolated D.f cannot satisfy the requirement for a non-isolated Q.f().

Conformance to a Sendable-inheriting protocol is a strong indication that a type cannot be main-actor-isolated. Therefore, SE-0466 should suppress inference of @MainActor on types like D that declare a conformance to a Sendable-inheriting protocol in their primary definition.

nonisolated members satisfying requirements

Inference of isolated conformances is specified to only look at the type and the protocol. However, that means that it will change the meaning of code that intentionally provides nonisolated members to satisfy a protocol requirement, like this:

protocol Q {
  func f()
}

@MainActor
class D: Q {
  nonisolated func f() { ... }
}

func acceptQ<T: Q>(_: T.Type) { }
nonisolated func passD() {
  acceptQ(D.self) // accepted today
                  // error under InferIsolatedConformances because D: Q is @MainActor
}

Without InferIsolatedConformances, the conformance of D to Q is non-isolated. With InferIsolatedConformances, the conformance D: Q is isolated to the main actor, even though it doesn't have to be. The fix is to mark the conformance itself as nonisolated when you use InferIsolatedConformances.

Instead, we could look at the declarations that are used to satisfy the conformance requirements. If they are all nonisolated, then the conformance is nonisolated. Then, this code would continue to work the same way it always has. This idea came up in the review discussion, but we rejected it due to concerns over its impact on compile times and the expectation that the migration to nonisolated would be straightforward.

I've found two issues with this. The first is macros: macros like Observable introduce nonisolated members to satisfy protocol requirements. The macros were written with the expectation that the conformance had to be non-isolated, and don't know whether the type they're extending is actor-isolated or not. With SE-0470 as it is, the macros themselves will need to be updated to emit nonisolated on the conformance. This isn't automatically migratable because the code is generated by a macro, so users of the macro will be affected by this source compatibility issue but can't directly fix the problem.

The second issue is that the impact of having the wrong isolation inferred can be far from where the problem was introduced. The inferred @MainActor isolation on the conformance of D to Q, above, won't be apparent until someone tries to use that conformance from non-isolated code. That could easily be in a different module or package.

Synthesized conformances (e.g., Equatable, Hashable, Codable)

For several common protocols, like Equatable, Hashable, and the two protocols that make up Codable (Encodable and Decodable), the Swift compiler introduces members to satisfy the conformance requirements (e.g., ==, hash(into:), and encode(to:)). Similar to the issue with macros above, the compiler has always introduced these as nonisolated members. Instead, the compiler should only make these nonisolated when the type itself is not global-actor isolated. Otherwise, they should get their isolation inferred in the same way as other members do. In practice, this means that main-actor types can make use of synthesized, isolated conformances:

@MainActor
class C: Codable {
  // ... okay, conformance is infferred @MainActor, as are init(from:) and encode(to:).
}

Cheers,
Doug

12 Likes
2

I'm torn on this one because —individually— the proposed changes for each of the scenarios are convincing and solve real problems. However, when combining all of them I find the end result quite convoluted, as even for a simple protocol conformance:

final class Foo: P { ... }

It's not easy to know whether P will be an isolated conformance or not:

  • Is P a protocol refining Sendable?
  • Is the implementation of P using nonisolated methods only?
  • Is the file using default main actor isolation?

I really wish I could just look at the type's signature and know whether the conformances are isolated or not. I could keep a one or two considerations in mind (ie whether default main actor isolation is used) but any more than that and knowing whether the conformances are isolated or not becomes an educated guess at best.

I wonder if emitting errors + fixits for this scenario has been considered. Maybe that'd too annoying (I seem to recall from the review thread that emitting diagnostics for inferred behaviors was considered undesirable). But at least the end result would be:

@MainActor class C2: nonisolated P { ... }

Which is crystal clear to any reader. Perhaps the commonly used "code is written once, but read multiple times" could be extended to fixing errors. It's a minor inconvenience to fix it at first, but the code will be clearer to any future readers.

One concern I have with this is that on a protocol with a long list of requirements, adding a new method and forgetting to add nonisolated to it would trigger a cascade of errors as the inference would unexpectedly switch from nonisolated to @MainActor (or whatever global actor the type was using) in a way that wouldn't be easily traced back to the new method.

As with the other behaviors, I wonder if requiring explicitness wouldn't be a better tradeoff. In this case, by doing the same analysis of the implementation (to see whether it's already nonisolated) and emitting a warning + two fix-its:

@MainActor
class D: Q { // ⚠ Conformance to Q is inferred to be @MainActor, but implementation is nonisolated.
             // - [Fix-it] Did you to intend to write `nonisolated Q`?
             // - [Fix-it] Make isolated conformance explicit with `@MainActor Q`.
  nonisolated func f() { ... }
}

So devs would need to make the decision explicit by annotating the conformance with either nonisolated or @MainActor. That way, if the protocol is extended with new methods, and those methods are written with the wrong isolation, the issue would be diagnosed in a sensible place (instead of triggering a change in the isolation inference, which would likely cause errors to appear in other parts of the codebase):

@MainActor
class D: nonisolated Q {
  nonisolated func f() { ... }
  func f2() { ... } // ❌ Main actor-isolated instance method 'f()' cannot be used to satisfy nonisolated protocol requirement
}

This could also result in better code on the long-term. Seems to me like the main goal of this addition is to avoid breaking (or adding diagnostics) to existing code. But if simply writing nonisolated implementations is sufficient for the conformance to be inferred nonisolated, this is a behavior developers could pick up for new code, which strikes me as highly undesirable. It's just much more brittle than writing nonisolated in the conformance. You wouldn't ever want to add nonisolated to a bunch of methods but elide it in the conformance when writing new code.

I understand the hesitance towards adding even more diagnostics to Swift Concurrency, but the scenario where a conformance would be inferred to be isolated, yet the entire implementation is nonisolated, sounds to me like the kind of thing where the developer needs to clarify intent instead of the compiler trying to magically infer the behavior.

Ah, this is great! I hadn't realized this wasn't already a thing.

3 Likes
3

It's a meta-point, but this feels like an argument against inference in general. I could look at type inference for

let b = f(g(a))

and ask a similar set of questions:

  1. What does b refer to?
  2. Which overload of g is selected?
  3. Which generic arguments are inferred if f is generic?

Inference is generally meant to do the right thing, and tools help show what actually happened in practice. I view the inference rules described here in the same way.

We did talk about this somewhat during the review, and the proposal skewed toward making these explicit as you suggest. I've been experimenting with the implementation on real code, and the lack of these inference rules gets in the way a lot more than I'd expected.

To any reader who understands the concurrency model, sure. Both of the features that are interacting here---isolated conformances and default isolation to the main actor---are part of the approachable concurrency vision, which explicitly tries not to bring concurrency-related topics like @MainActor and nonisolated into Swift code until it introduces concurrency.

What I found is that it's more than a minor inconvenience. When macros are (say) introducing nonisolated members, you have to go and modify the implementation of the macro, which might not even be possible for you.

You'll get an error in places where you depended on the conforming being non-isolated, sure.

That's a goal, but another is to support a more approachable model to concurrency. We lose that when we're asking folks to add a bunch of annotations that they shouldn't need to deal with.

It's arguably a bug fix, but I wanted to call it out in part because it supports the general inference rules described above.

Doug

3 Likes
4

It's a fair point, but often I've found myself surprised by actor inference in types in a way that doesn't happen for type inference. For example, the other day I mindlessly conformed a type to ASWebAuthenticationPresentationContextProviding to fulfill a requirement and —unbeknownst to me— that made my type @MainActor.

I didn't found out immediately though, as I happened to be using the type from the Main Actor already. So everything compiled right away. It was only a day later, when I attempted to initialize this type from a testing function and the compiler complained that I needed to use await for the init() call.

I was able to solve this quickly by looking at the type signature and concluding that one of the protocols had to be @MainActor, and ultimately located which one.

That's okay, but it's a question people will ask themselves fairly often: why is this conformance isolated/nonisolated? As long as the set of rules is relatively simple, it's fine for inference to help. For the problem I had, the only explanation was that one of the protocols was @MainActor. That's easy to check for. But for conformance isolation, I feel like the set of rules is a bit more complex than that (admittedly, it could just be familiarity bias, but I think it's worth pointing it out).

I would have thought that conforming to a protocol that refines Sendable from a globally actor isolated type is introducing concurrency already. Perhaps I'm missing some big use case here, but are there any commonly used protocols that refine Sendable that a newcomer would encounter?

Only Error comes to mind, though that's already a big one. You'd need to write nonisolated enum MyError: Error to be explicit, and that's clearly bad for progressive disclosure (and even opens the door to writing enum MyError: nonisolated Error, which is probably not what you want).

Okay, I think I've talked myself out of this one :sweat_smile: It does feel broken if inference attempts to make a Sendable-conforming type @MainActor. That poses a question, though. The pitch mentions:

Therefore, SE-0466 should suppress inference of @MainActor on types like D that declare a conformance to a Sendable -inheriting protocol in their primary definition.

Would that also apply to types that directly conform to Sendable (and not just Sendable-inheriting protocols)? I'm guessing the answer is yes, just checking. Also, what does primary definition mean above? Would the same behavior not apply if the conformance is declared in an extension?

Exactly. But the source of the problem may not be immediately obvious. Thinking of git merges, for example, where one branch makes methods nonisolated and the other adds a new method (without isolation annotations) to the same protocol.

An explicit annotation would surface the problem in the right place (the new method, without nonisolated). With inference, it would surface the problem where the nonisolated conformance was being used, and the merger would need to trace the problem back to the newly added method.

Well, if using isolated conformances they don't need to add any annotation. Only if they want to make a globally actor isolated type have a nonisolated conformance would they need to reach for those extra annotations. Which sounds about right to me in terms of progressive disclosure.

5

+1. But I was under impression that this is already part of the SE-0470 logic. I could not find this in the proposal text, but isolated conformances make no sense for Sendable-inheriting protocols. IMO, this could be fixed as a bug, without going through review process.

-1. IMO, it would make code less readable. If there is a nonisolated class in @MainActor-by-default code, I want to see nonisolated keyword in code. I would keep existing logic as is, and maybe add a fix-it to insert nonisolated to entire class or individual methods.

Similarly, I would prefer a warning suggesting to disambiguate isolation of the conformance. Warning could be emitted even if at least one member is nonisolated.

+1, but compiler should be checking isolation of the corresponding conformance, not isolation of the type.