Skip to content

[SE-0306] Disallow actor inheritance from NSObject, but allow @objc actor #38246

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
Jul 3, 2021
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
2 changes: 2 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -4351,6 +4351,8 @@ ERROR(async_objc_dynamic_self,none,
ERROR(actor_inheritance,none,
"%select{actor|distributed actor}0 types do not support inheritance",
(bool))
NOTE(actor_inheritance_nsobject,none,
"use '@objc' to expose actor %0 to Objective-C", (DeclName))

ERROR(actor_protocol_illegal_inheritance,none,
"non-actor type %0 cannot conform to the 'Actor' protocol",
Expand Down
15 changes: 7 additions & 8 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8289,15 +8289,14 @@ bool ClassDecl::isRootDefaultActor(ModuleDecl *M,
}

bool ClassDecl::isNativeNSObjectSubclass() const {
// Only if we inherit from NSObject.
auto superclass = getSuperclassDecl();
if (!superclass || !superclass->isNSObject())
return false;
// @objc actors implicitly inherit from NSObject.
if (isActor() && getAttrs().hasAttribute<ObjCAttr>())
return true;

// For now, only actors (regardless of whether they're default actors).
// Eventually we should roll this out to more classes, but we have to
// do it with ABI compatibility.
return isActor();
// For now, non-actor classes cannot use the native NSObject subclass.
// Eventually we should roll this out to more classes that directly
// inherit NSObject, but we have to do it with ABI compatibility.
return false;
}

bool ClassDecl::isNSObject() const {
Expand Down
5 changes: 3 additions & 2 deletions lib/Sema/TypeCheckDeclObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1271,10 +1271,11 @@ static Optional<ObjCReason> shouldMarkClassAsObjC(const ClassDecl *CD) {
reason.describe(CD);
}

// Only allow ObjC-rooted classes to be @objc.
// Only allow actors and ObjC-rooted classes to be @objc.
// (Leave a hole for test cases.)
if (ancestry.contains(AncestryFlags::ObjC) &&
!ancestry.contains(AncestryFlags::ClangImported)) {
!ancestry.contains(AncestryFlags::ClangImported) &&
!CD->isActor()) {
if (ctx.LangOpts.EnableObjCAttrRequiresFoundation) {
swift::diagnoseAndRemoveAttr(CD, attr,
diag::invalid_objc_swift_rooted_class)
Expand Down
10 changes: 8 additions & 2 deletions lib/Sema/TypeCheckDeclPrimary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2398,12 +2398,18 @@ class DeclChecker : public DeclVisitor<DeclChecker> {

if (auto superclass = CD->getSuperclassDecl()) {
// Actors cannot have superclasses, nor can they be superclasses.
if (CD->isActor() && !superclass->isNSObject())
if (CD->isActor()) {
CD->diagnose(diag::actor_inheritance,
/*distributed=*/CD->isDistributedActor());
else if (superclass->isActor())
if (superclass->isNSObject() && !CD->isDistributedActor()) {
CD->diagnose(diag::actor_inheritance_nsobject, CD->getName())
.fixItInsert(CD->getAttributeInsertionLoc(/*forModifier=*/false),
"@objc ");
}
} else if (superclass->isActor()) {
CD->diagnose(diag::actor_inheritance,
/*distributed=*/CD->isDistributedActor());
}
}

// Force lowering of stored properties.
Expand Down
6 changes: 2 additions & 4 deletions test/IRGen/actor_class_objc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import Foundation

// CHECK-LABEL: @"OBJC_METACLASS_$__TtC16actor_class_objc7MyClass" = global
// Metaclass is an instance of the root class.
// CHECK-SAME: %objc_class* {{.*}}@"OBJC_METACLASS_$_NSObject{{(.ptrauth)?}}"
// Metaclass superclass is the metaclass of the superclass.
// CHECK-SAME: %objc_class* {{.*}}@"OBJC_METACLASS_$_SwiftNativeNSObject{{(.ptrauth)?}}"

// CHECK: @"$s16actor_class_objc7MyClassCMf" = internal global
Expand All @@ -29,9 +27,9 @@ import Foundation
// CHECK-64-SAME: i64 112,
// CHECK-32-SAME: i32 56,

public actor MyClass: NSObject {
@objc public actor MyClass {
public var x: Int
public override init() { self.x = 0 }
public init() { self.x = 0 }
}

// CHECK-LABEL: define {{.*}} @"$s16actor_class_objc7MyClassC1xSivg"
Expand Down
17 changes: 2 additions & 15 deletions test/Interpreter/actor_class_forbid_objc_assoc_objects.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
// UNSUPPORTED: back_deployment_runtime

import ObjectiveC
import Foundation
import _Concurrency
import StdlibUnittest

Expand Down Expand Up @@ -70,7 +71,7 @@ if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
}

@available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *)
actor ActorNSObjectSubKlass : NSObject {}
@objc actor ActorNSObjectSubKlass {}

if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
Tests.test("no crash when inherit from nsobject")
Expand All @@ -79,17 +80,3 @@ if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
}
}

@available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *)
actor ActorNSObjectSubKlassGeneric<T> : NSObject {
var state: T
init(state: T) { self.state = state }
}

if #available(macOS 10.4.4, iOS 12.2, watchOS 5.2, tvOS 12.2, *) {
Tests.test("no crash when generic inherit from nsobject")
.code {
let x = ActorNSObjectSubKlassGeneric(state: 5)
objc_setAssociatedObject(x, "myKey", "myValue", .OBJC_ASSOCIATION_RETAIN)
}
}
6 changes: 3 additions & 3 deletions test/ModuleInterface/actor_objc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import Foundation

// CHECK-LABEL: @objc @_inheritsConvenienceInitializers public actor SomeActor : ObjectiveC.NSObject {
// CHECK: @objc override public init()
public actor SomeActor: NSObject {
// CHECK-LABEL: @objc public actor SomeActor {
// CHECK-NOT: @objc override public init()
@objc public actor SomeActor {
}
7 changes: 5 additions & 2 deletions test/attr/attr_objc_async.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@ actor class MyActor2 { }
// expected-error@-1 {{keyword 'class' cannot be used as an identifier here}}

// CHECK: @objc actor MyObjCActor
@objc actor MyObjCActor: NSObject { }
@objc actor MyObjCActor { }

@objc actor class MyObjCActor2: NSObject {}
@objc actor class MyObjCActor2 {}
// expected-error@-1 {{keyword 'class' cannot be used as an identifier here}}

actor MyObjCActor3: NSObject { } // expected-error{{actor types do not support inheritance}}
// expected-note@-1{{use '@objc' to expose actor 'MyObjCActor3' to Objective-C}}