Skip to content

Commit 5787b95

Browse files
committed
AST: Rework @objcMembers inheritance to not depend on validation order
Sema would directly check for the presence of the @objcMembers attribute, and inherit it from the immediate superclass in validateDecl(). We don't want validateDecl() to have side effects like this and this was already a problem, because it would not inherit the attribute transitively in some cases. Instead, add a ClassDecl::hasObjCMembers() method that walks over all ancestors and caches the result. <rdar://problem/46420252>
1 parent d0d64a1 commit 5787b95

File tree

7 files changed

+48
-21
lines changed

7 files changed

+48
-21
lines changed

include/swift/AST/Decl.h

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ class alignas(1 << DeclAlignInBits) Decl {
524524
NumRequirementsInSignature : 16
525525
);
526526

527-
SWIFT_INLINE_BITFIELD(ClassDecl, NominalTypeDecl, 1+2+1+2+1+3+1+1,
527+
SWIFT_INLINE_BITFIELD(ClassDecl, NominalTypeDecl, 1+2+1+2+1+3+1+1+1+1,
528528
/// Whether this class requires all of its instance variables to
529529
/// have in-class initializers.
530530
RequiresStoredPropertyInits : 1,
@@ -548,6 +548,10 @@ class alignas(1 << DeclAlignInBits) Decl {
548548
/// Whether the class has @objc ancestry.
549549
ObjCKind : 3,
550550

551+
/// Whether this class has @objc members.
552+
HasObjCMembersComputed : 1,
553+
HasObjCMembers : 1,
554+
551555
HasMissingDesignatedInitializers : 1,
552556
HasMissingVTableEntries : 1
553557
);
@@ -3574,6 +3578,8 @@ class ClassDecl final : public NominalTypeDecl {
35743578
friend class SuperclassTypeRequest;
35753579
friend class TypeChecker;
35763580

3581+
bool hasObjCMembersSlow();
3582+
35773583
public:
35783584
ClassDecl(SourceLoc ClassLoc, Identifier Name, SourceLoc NameLoc,
35793585
MutableArrayRef<TypeLoc> Inherited,
@@ -3732,11 +3738,20 @@ class ClassDecl final : public NominalTypeDecl {
37323738
Bits.ClassDecl.InheritsSuperclassInits = true;
37333739
}
37343740

3735-
/// Figure out if this class has any @objc ancestors, in which case it should
3736-
/// have implicitly @objc members. Note that a class with generic ancestry
3737-
/// might have implicitly @objc members, but will never itself be @objc.
3741+
/// Returns if this class has any @objc ancestors, or if it is directly
3742+
/// visible to Objective-C. The latter is a stronger condition which is only
3743+
/// true if the class does not have any generic ancestry.
37383744
ObjCClassKind checkObjCAncestry() const;
37393745

3746+
/// Returns if the class has implicitly @objc members. This is true if any
3747+
/// ancestor class has the @objcMembers attribute.
3748+
bool hasObjCMembers() const {
3749+
if (Bits.ClassDecl.HasObjCMembersComputed)
3750+
return Bits.ClassDecl.HasObjCMembers;
3751+
3752+
return const_cast<ClassDecl *>(this)->hasObjCMembersSlow();
3753+
}
3754+
37403755
/// The type of metaclass to use for a class.
37413756
enum class MetaclassKind : uint8_t {
37423757
ObjC,

lib/AST/Decl.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3315,6 +3315,8 @@ ClassDecl::ClassDecl(SourceLoc ClassLoc, Identifier Name, SourceLoc NameLoc,
33153315
Bits.ClassDecl.RawForeignKind = 0;
33163316
Bits.ClassDecl.HasDestructorDecl = 0;
33173317
Bits.ClassDecl.ObjCKind = 0;
3318+
Bits.ClassDecl.HasObjCMembersComputed = 0;
3319+
Bits.ClassDecl.HasObjCMembers = 0;
33183320
Bits.ClassDecl.HasMissingDesignatedInitializers = 0;
33193321
Bits.ClassDecl.HasMissingVTableEntries = 0;
33203322
}
@@ -3477,6 +3479,20 @@ ObjCClassKind ClassDecl::checkObjCAncestry() const {
34773479
return kind;
34783480
}
34793481

3482+
bool ClassDecl::hasObjCMembersSlow() {
3483+
// Don't attempt to calculate this again.
3484+
Bits.ClassDecl.HasObjCMembersComputed = true;
3485+
3486+
bool result = false;
3487+
if (getAttrs().hasAttribute<ObjCMembersAttr>())
3488+
result = true;
3489+
else if (auto *superclassDecl = getSuperclassDecl())
3490+
result = superclassDecl->hasObjCMembers();
3491+
3492+
Bits.ClassDecl.HasObjCMembers = result;
3493+
return result;
3494+
}
3495+
34803496
ClassDecl::MetaclassKind ClassDecl::getMetaclassKind() const {
34813497
assert(getASTContext().LangOpts.EnableObjCInterop &&
34823498
"querying metaclass kind without objc interop");

lib/ClangImporter/ImportDecl.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7416,13 +7416,9 @@ void ClangImporter::Implementation::importAttributes(
74167416
return;
74177417
}
74187418

7419-
// Map Clang's swift_objc_members attribute to @objcMembers. Also handle
7420-
// inheritance of @objcMembers by looking at the superclass.
7421-
if (ID->hasAttr<clang::SwiftObjCMembersAttr>() ||
7422-
(isa<ClassDecl>(MappedDecl) &&
7423-
cast<ClassDecl>(MappedDecl)->hasSuperclass() &&
7424-
cast<ClassDecl>(MappedDecl)->getSuperclassDecl()
7425-
->getAttrs().hasAttribute<ObjCMembersAttr>())) {
7419+
// Map Clang's swift_objc_members attribute to @objcMembers.
7420+
if (ID->hasAttr<clang::SwiftObjCMembersAttr>() &&
7421+
isa<ClassDecl>(MappedDecl)) {
74267422
if (!MappedDecl->getAttrs().hasAttribute<ObjCMembersAttr>()) {
74277423
auto attr = new (C) ObjCMembersAttr(/*IsImplicit=*/true);
74287424
MappedDecl->getAttrs().add(attr);

lib/Sema/TypeCheckDecl.cpp

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3957,14 +3957,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
39573957
(CD->hasSuperclass() &&
39583958
CD->getSuperclassDecl()->requiresStoredPropertyInits()))
39593959
CD->setRequiresStoredPropertyInits(true);
3960-
3961-
// Inherit @objcMembers.
3962-
if (auto superclass = CD->getSuperclassDecl()) {
3963-
if (superclass->getAttrs().hasAttribute<ObjCMembersAttr>() &&
3964-
!CD->getAttrs().hasAttribute<ObjCMembersAttr>()) {
3965-
CD->getAttrs().add(new (Context) ObjCMembersAttr(/*IsImplicit=*/true));
3966-
}
3967-
}
39683960
}
39693961

39703962
if (auto *ED = dyn_cast<EnumDecl>(nominal)) {

lib/Sema/TypeCheckDeclObjC.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -988,7 +988,7 @@ static bool isMemberOfObjCMembersClass(const ValueDecl *VD) {
988988
auto classDecl = VD->getDeclContext()->getSelfClassDecl();
989989
if (!classDecl) return false;
990990

991-
return classDecl->getAttrs().hasAttribute<ObjCMembersAttr>();
991+
return classDecl->hasObjCMembers();
992992
}
993993

994994
// A class is @objc if it does not have generic ancestry, and it either has
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
@objcMembers class BaseClassWithObjCMembers {}
2+
3+
class OtherClassWithObjCMembers : BaseClassWithObjCMembers {}

test/attr/attr_objcMembers.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -disable-objc-attr-requires-foundation-module -typecheck -verify %s
1+
// RUN: %target-swift-frontend -disable-objc-attr-requires-foundation-module -typecheck -verify %s %S/Inputs/attr_objcMembers_other.swift
22
// REQUIRES: objc_interop
33

44
import Foundation
@@ -24,12 +24,17 @@ extension SubClassOfSomeClassWithObjCMembers {
2424
func wibble() { }
2525
}
2626

27+
class SubClassOfOtherClassWithObjCMembers : OtherClassWithObjCMembers {
28+
func quux() { }
29+
}
30+
2731
// @objc should be inferred for everything referenced here.
2832
func selectorTest() {
2933
_ = #selector(SomeClassWithObjCMembers.foo)
3034
_ = #selector(getter: SomeClassWithObjCMembers.bar)
3135
_ = #selector(SubClassOfSomeClassWithObjCMembers.baz)
3236
_ = #selector(SubClassOfSomeClassWithObjCMembers.wibble)
37+
_ = #selector(SubClassOfOtherClassWithObjCMembers.quux)
3338
}
3439

3540
@nonobjc extension SomeClassWithObjCMembers {

0 commit comments

Comments
 (0)