Skip to content

Commit 4eec522

Browse files
committed
Sema: Fix inheritance of @requires_stored_property_inits
Previously we would copy this attribute from a superclass to the subclass when validating a subclass. However, this is only correct if the superclass is always guaranteed to have been validated before the subclass. Indeed, it appears this assumption is no longer true, so we would sometimes lose track of the attribute, which would result in SILGen failing to emit the ivar initializer entry point. Instead, check for the attribute as part of the superclass walk in checkAncestry(), ensuring the result is always up to date, correct, and cached. As a follow-up, we should also convert checkAncestry() into a request, but I'm not doing that here to keep the fix short. Fixes <rdar://problem/50845438>.
1 parent 755c02c commit 4eec522

File tree

9 files changed

+44
-31
lines changed

9 files changed

+44
-31
lines changed

include/swift/AST/Decl.h

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -543,11 +543,7 @@ class alignas(1 << DeclAlignInBits) Decl {
543543
NumRequirementsInSignature : 16
544544
);
545545

546-
SWIFT_INLINE_BITFIELD(ClassDecl, NominalTypeDecl, 1+2+1+2+1+6+1+1+1+1,
547-
/// Whether this class requires all of its instance variables to
548-
/// have in-class initializers.
549-
RequiresStoredPropertyInits : 1,
550-
546+
SWIFT_INLINE_BITFIELD(ClassDecl, NominalTypeDecl, 2+1+2+1+7+1+1+1+1,
551547
/// The stage of the inheritance circularity check for this class.
552548
Circularity : 2,
553549

@@ -565,7 +561,7 @@ class alignas(1 << DeclAlignInBits) Decl {
565561
HasDestructorDecl : 1,
566562

567563
/// Information about the class's ancestry.
568-
Ancestry : 6,
564+
Ancestry : 7,
569565

570566
/// Whether we have computed the above field or not.
571567
AncestryComputed : 1,
@@ -3713,6 +3709,9 @@ enum class AncestryFlags : uint8_t {
37133709

37143710
/// The class or one of its superclasses is imported from Clang.
37153711
ClangImported = (1<<5),
3712+
3713+
/// The class or one of its superclasses requires stored property initializers.
3714+
RequiresStoredPropertyInits = (1<<6),
37163715
};
37173716

37183717
/// Return type of ClassDecl::checkAncestry(). Describes a set of interesting
@@ -3810,14 +3809,8 @@ class ClassDecl final : public NominalTypeDecl {
38103809

38113810
//// Whether this class requires all of its stored properties to
38123811
//// have initializers in the class definition.
3813-
bool requiresStoredPropertyInits() const {
3814-
return Bits.ClassDecl.RequiresStoredPropertyInits;
3815-
}
3816-
3817-
/// Set whether this class requires all of its stored properties to
3818-
/// have initializers in the class definition.
3819-
void setRequiresStoredPropertyInits(bool requiresInits) {
3820-
Bits.ClassDecl.RequiresStoredPropertyInits = requiresInits;
3812+
bool requiresStoredPropertyInits() const {
3813+
return checkAncestry(AncestryFlags::RequiresStoredPropertyInits);
38213814
}
38223815

38233816
/// \see getForeignClassKind

include/swift/Serialization/ModuleFormat.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
5252
/// describe what change you made. The content of this comment isn't important;
5353
/// it just ensures a conflict if two people change the module format.
5454
/// Don't worry about adhering to the 80-column limit for this line.
55-
const uint16_t SWIFTMODULE_VERSION_MINOR = 502; // move specifier down to ParamDecl
55+
const uint16_t SWIFTMODULE_VERSION_MINOR = 503; // remove 'requires stored property inits'
5656

5757
using DeclIDField = BCFixed<31>;
5858

@@ -983,7 +983,6 @@ namespace decls_block {
983983
DeclContextIDField, // context decl
984984
BCFixed<1>, // implicit?
985985
BCFixed<1>, // explicitly objc?
986-
BCFixed<1>, // requires stored property initial values?
987986
BCFixed<1>, // inherits convenience initializers from its superclass?
988987
GenericEnvironmentIDField, // generic environment
989988
TypeIDField, // superclass

lib/AST/Decl.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3745,7 +3745,6 @@ ClassDecl::ClassDecl(SourceLoc ClassLoc, Identifier Name, SourceLoc NameLoc,
37453745
ClassLoc(ClassLoc) {
37463746
Bits.ClassDecl.Circularity
37473747
= static_cast<unsigned>(CircularityCheck::Unchecked);
3748-
Bits.ClassDecl.RequiresStoredPropertyInits = 0;
37493748
Bits.ClassDecl.InheritsSuperclassInits = 0;
37503749
Bits.ClassDecl.RawForeignKind = 0;
37513750
Bits.ClassDecl.HasDestructorDecl = 0;
@@ -3918,6 +3917,9 @@ AncestryOptions ClassDecl::checkAncestry() const {
39183917
if (CD->hasResilientMetadata(M, ResilienceExpansion::Maximal))
39193918
result |= AncestryFlags::ResilientOther;
39203919

3920+
if (CD->getAttrs().hasAttribute<RequiresStoredPropertyInitsAttr>())
3921+
result |= AncestryFlags::RequiresStoredPropertyInits;
3922+
39213923
CD = CD->getSuperclassDecl();
39223924
} while (CD != nullptr);
39233925

lib/ClangImporter/ImportDecl.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4581,7 +4581,6 @@ namespace {
45814581
auto a = new (Impl.SwiftContext)
45824582
RequiresStoredPropertyInitsAttr(/*IsImplicit=*/true);
45834583
decl->getAttrs().add(a);
4584-
cast<ClassDecl>(decl)->setRequiresStoredPropertyInits(true);
45854584
}
45864585
}
45874586

lib/Sema/TypeCheckDecl.cpp

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3963,14 +3963,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
39633963

39643964
validateAttributes(*this, D);
39653965

3966-
if (auto CD = dyn_cast<ClassDecl>(nominal)) {
3967-
// Determine whether we require in-class initializers.
3968-
if (CD->getAttrs().hasAttribute<RequiresStoredPropertyInitsAttr>() ||
3969-
(CD->hasSuperclass() &&
3970-
CD->getSuperclassDecl()->requiresStoredPropertyInits()))
3971-
CD->setRequiresStoredPropertyInits(true);
3972-
}
3973-
39743966
if (auto *ED = dyn_cast<EnumDecl>(nominal)) {
39753967
// @objc enums use their raw values as the value representation, so we
39763968
// need to force the values to be checked.

lib/Serialization/Deserialization.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3516,7 +3516,7 @@ class swift::DeclDeserializer {
35163516
StringRef blobData) {
35173517
IdentifierID nameID;
35183518
DeclContextID contextID;
3519-
bool isImplicit, isObjC, requiresStoredPropertyInits;
3519+
bool isImplicit, isObjC;
35203520
bool inheritsSuperclassInitializers;
35213521
GenericEnvironmentID genericEnvID;
35223522
TypeID superclassID;
@@ -3525,7 +3525,6 @@ class swift::DeclDeserializer {
35253525
ArrayRef<uint64_t> rawInheritedAndDependencyIDs;
35263526
decls_block::ClassLayout::readRecord(scratch, nameID, contextID,
35273527
isImplicit, isObjC,
3528-
requiresStoredPropertyInits,
35293528
inheritsSuperclassInitializers,
35303529
genericEnvID, superclassID,
35313530
rawAccessLevel, numConformances,
@@ -3569,8 +3568,6 @@ class swift::DeclDeserializer {
35693568
theClass->setImplicit();
35703569
theClass->setIsObjC(isObjC);
35713570
theClass->setSuperclass(MF.getType(superclassID));
3572-
if (requiresStoredPropertyInits)
3573-
theClass->setRequiresStoredPropertyInits(true);
35743571
if (inheritsSuperclassInitializers)
35753572
theClass->setInheritsSuperclassInitializers();
35763573

lib/Serialization/Serialization.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3266,7 +3266,6 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
32663266
contextID,
32673267
theClass->isImplicit(),
32683268
theClass->isObjC(),
3269-
theClass->requiresStoredPropertyInits(),
32703269
inheritsSuperclassInitializers,
32713270
S.addGenericEnvironmentRef(
32723271
theClass->getGenericEnvironment()),
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
import Foundation
3+
4+
@requires_stored_property_inits public class OtherBase : NSObject {}
5+
6+
public class OtherMiddle : OtherBase {}

test/SILGen/ivar_initializer.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %build-clang-importer-objc-overlays
3+
4+
// RUN: %target-swift-emit-silgen(mock-sdk: %clang-importer-sdk-nosource -I %t) -primary-file %s %S/Inputs/ivar_initializer_other.swift | %FileCheck %s
5+
6+
// REQUIRES: objc_interop
7+
8+
import Foundation
9+
10+
@requires_stored_property_inits public class MyBase : NSObject {}
11+
12+
public class MyMiddle : MyBase {}
13+
14+
public class C {}
15+
16+
public class MyDerived : MyMiddle {
17+
var c = C()
18+
}
19+
20+
public class OtherDerived : OtherMiddle {
21+
var c = C()
22+
}
23+
24+
// CHECK-LABEL: sil hidden [ossa] @$s16ivar_initializer9MyDerivedCfeTo : $@convention(objc_method) (@owned MyDerived) -> @owned MyDerived {
25+
26+
// CHECK-LABEL: sil hidden [ossa] @$s16ivar_initializer12OtherDerivedCfeTo : $@convention(objc_method) (@owned OtherDerived) -> @owned OtherDerived {

0 commit comments

Comments
 (0)