Skip to content

[6.0][Concurrency] Implement a narrow carve out in the isolation override checking for NSObject.init(). #75749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5285,6 +5285,16 @@ ConcreteDeclRef swift::getDeclRefInContext(ValueDecl *value) {
return ConcreteDeclRef(value);
}

static bool isNSObjectInit(ValueDecl *overridden) {
auto *classDecl = dyn_cast_or_null<ClassDecl>(
overridden->getDeclContext()->getSelfNominalTypeDecl());
if (!classDecl || !classDecl->isNSObject()) {
return false;
}

return isa<ConstructorDecl>(overridden);
}

/// Generally speaking, the isolation of the decl that overrides
/// must match the overridden decl. But there are a number of exceptions,
/// e.g., the decl that overrides can be nonisolated.
Expand Down Expand Up @@ -5317,9 +5327,21 @@ static OverrideIsolationResult validOverrideIsolation(
}

// If the overridden declaration is from Objective-C with no actor
// annotation, allow it.
if (ctx.LangOpts.StrictConcurrencyLevel != StrictConcurrency::Complete &&
overridden->hasClangNode() && !overriddenIsolation) {
// annotation, don't allow overriding isolation in complete concurrency
// checking. Calls from outside the actor, via the nonisolated superclass
// method that is dynamically dispatched, will crash at runtime due to
// the dynamic isolation check in the @objc thunk.
//
// There's a narrow carve out for `NSObject.init()` because overriding
// this init of `@MainActor`-isolated type is difficult-to-impossible,
// especially if you need to call an initializer from an intermediate
// superclass that is also `@MainActor`-isolated. This won't admit a
// runtime data-race safety hole, because dynamic isolation checks will
// be inserted in the @objc thunks under `DynamicActorIsolation`, and
// direct calls will enforce `@MainActor` as usual.
if (isNSObjectInit(overridden) ||
(ctx.LangOpts.StrictConcurrencyLevel != StrictConcurrency::Complete &&
overridden->hasClangNode() && !overriddenIsolation)) {
return OverrideIsolationResult::Allowed;
}

Expand Down
16 changes: 14 additions & 2 deletions test/ClangImporter/objc_isolation_complete.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,27 @@ class IsolatedSub: NXSender {
}
}

class NotSendable {}

@MainActor
class NSObjectInitOverride: NSObject {
var ns: NotSendable

override init() {
self.ns = NotSendable()
super.init()
}
}


@objc
@MainActor
class Test : NSObject {
static var shared: Test? // expected-note {{mutation of this static property is only permitted within the actor}}
static var shared: Test?

override init() {
super.init()

Self.shared = self
// expected-warning@-1 {{main actor-isolated static property 'shared' can not be mutated from a nonisolated context; this is an error in the Swift 6 language mode}}
}
}