Skip to content

Commit 454dfb9

Browse files
authored
Merge pull request #75748 from hborla/override-nsobject-init
[Concurrency] Implement a narrow carve out in the isolation override checking for `NSObject.init()`.
2 parents 3e21fc6 + 0d5e598 commit 454dfb9

File tree

2 files changed

+39
-5
lines changed

2 files changed

+39
-5
lines changed

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5309,6 +5309,16 @@ ConcreteDeclRef swift::getDeclRefInContext(ValueDecl *value) {
53095309
return ConcreteDeclRef(value);
53105310
}
53115311

5312+
static bool isNSObjectInit(ValueDecl *overridden) {
5313+
auto *classDecl = dyn_cast_or_null<ClassDecl>(
5314+
overridden->getDeclContext()->getSelfNominalTypeDecl());
5315+
if (!classDecl || !classDecl->isNSObject()) {
5316+
return false;
5317+
}
5318+
5319+
return isa<ConstructorDecl>(overridden);
5320+
}
5321+
53125322
/// Generally speaking, the isolation of the decl that overrides
53135323
/// must match the overridden decl. But there are a number of exceptions,
53145324
/// e.g., the decl that overrides can be nonisolated.
@@ -5341,9 +5351,21 @@ static OverrideIsolationResult validOverrideIsolation(
53415351
}
53425352

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

test/ClangImporter/objc_isolation_complete.swift

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,27 @@ class IsolatedSub: NXSender {
3232
}
3333
}
3434

35+
class NotSendable {}
36+
37+
@MainActor
38+
class NSObjectInitOverride: NSObject {
39+
var ns: NotSendable
40+
41+
override init() {
42+
self.ns = NotSendable()
43+
super.init()
44+
}
45+
}
46+
47+
3548
@objc
3649
@MainActor
3750
class Test : NSObject {
38-
static var shared: Test? // expected-note {{mutation of this static property is only permitted within the actor}}
51+
static var shared: Test?
3952

4053
override init() {
4154
super.init()
4255

4356
Self.shared = self
44-
// 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}}
4557
}
4658
}

0 commit comments

Comments
 (0)