Skip to content

Conversation

pull[bot]
Copy link

@pull pull bot commented Mar 8, 2022

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

kavon and others added 6 commits March 7, 2022 16:41
During actor isolation inference, we would unconditionally choose the
isolation of the overridden decl (when, say, there is no attribute on the decl).
The overridden decl is identified with `getOverriddenDecl`.

This works mostly fine, but initializers have some unusual overridden decls
returned by that method. For example, in the following code:

```swift
@objc class PictureFrame: NSObject {
  init(size: Int) { }
}

@mainactor
class FooFrame: PictureFrame {
  init() {
    super.init(size: 0)
  }
}
```

that method claims that `FooFrame.init()` overrides `PictureFrame.init()`, when
it really does not! So, if we were to unconditionally take the isolation from
`PictureFrame.init()` (and thus `NSObject.init()`), then we'd infer that
`FooFrame.init()` has unspecified isolation, despite the nominal it resides in
being marked as `MainActor`. This is in essence the problem in SR-15694, where
placing the isolation directly on the initializer fixes this issue.

If `FooFrame.init()` really does override, then why can it be `MainActor`? Well,
we have a rule in one part of the type-checker saying that if an ObjC-imported
decl has unspecified isolation, then overriding it with isolation is permitted.
But the other part of the type-checker dealing with the isolation inference was
not permitting that.

So, this patch unifies how actor-isolation inference is conducted to reuse that
same logic. In addition, the inference now effectively says that, if the decl
has no inferred isolation, or the inferred isolation is invalid according to the
overriding rules, then we use the isolation of the overridden decl. This
preserves the old behavior of the inference, while also fixing this issue with
ObjC declarations by expanding where isolation is allowed.

For example, as a consequence of this change, in the following code:

```swift
@mainactor
class FooFrame: NotIsolatedPictureFrame {
  override func rotate() {
    mainActorFn()
  }
}
```

if `NotIsolatedPictureFrame` is a plain-old not-isolated class imported from
Objective-C, then `rotate` will now have `MainActor` isolation, instead of
having unspecified isolation like it was inferred to have previously. This
helps make things consistent, because `rotate` is allowed to be `@MainActor` if
it were explicitly marked as such.

resolves rdar://87217618 / SR-15694
[SR-15694] make isolation inference + override more consistent
Revert "visualc: clean up the module definition"
Fix error in MutableCollection.shuffle() docs
@MaxDesiatov MaxDesiatov enabled auto-merge March 8, 2022 13:32
@MaxDesiatov MaxDesiatov merged commit a78f259 into swiftwasm Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants