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>.

Author: slavapestov

include/swift/AST/Decl.h

@@ -566,11 +566,7 @@ class alignas(1 << DeclAlignInBits) Decl {
     NumRequirementsInSignature : 16
   );
 
-  SWIFT_INLINE_BITFIELD(ClassDecl, NominalTypeDecl, 1+2+1+2+1+6+1+1+1,
-    /// Whether this class requires all of its instance variables to
-    /// have in-class initializers.
-    RequiresStoredPropertyInits : 1,
-
+  SWIFT_INLINE_BITFIELD(ClassDecl, NominalTypeDecl, 2+1+2+1+7+1+1+1,
     /// The stage of the inheritance circularity check for this class.
     Circularity : 2,
 
@@ -588,7 +584,7 @@ class alignas(1 << DeclAlignInBits) Decl {
     HasDestructorDecl : 1,
 
     /// Information about the class's ancestry.
-    Ancestry : 6,
+    Ancestry : 7,
 
     /// Whether we have computed the above field or not.
     AncestryComputed : 1,
@@ -3740,6 +3736,9 @@ enum class AncestryFlags : uint8_t {
 
   /// The class or one of its superclasses is imported from Clang.
   ClangImported = (1<<5),
+
+  /// The class or one of its superclasses requires stored property initializers.
+  RequiresStoredPropertyInits = (1<<6),
 };
 
 /// Return type of ClassDecl::checkAncestry(). Describes a set of interesting
@@ -3837,14 +3836,8 @@ class ClassDecl final : public NominalTypeDecl {
 
   //// Whether this class requires all of its stored properties to
   //// have initializers in the class definition.
-  bool requiresStoredPropertyInits() const { 
-    return Bits.ClassDecl.RequiresStoredPropertyInits;
-  }
-
-  /// Set whether this class requires all of its stored properties to
-  /// have initializers in the class definition.
-  void setRequiresStoredPropertyInits(bool requiresInits) {
-    Bits.ClassDecl.RequiresStoredPropertyInits = requiresInits;
+  bool requiresStoredPropertyInits() const {
+    return checkAncestry(AncestryFlags::RequiresStoredPropertyInits);
   }
 
   /// \see getForeignClassKind

include/swift/Serialization/ModuleFormat.h

@@ -52,7 +52,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
 /// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t SWIFTMODULE_VERSION_MINOR = 498; // dependency types for protocols
+const uint16_t SWIFTMODULE_VERSION_MINOR = 499; // remove 'requires stored property inits'
 
 using DeclIDField = BCFixed<31>;
 
@@ -976,7 +976,6 @@ namespace decls_block {
     DeclContextIDField,     // context decl
     BCFixed<1>,             // implicit?
     BCFixed<1>,             // explicitly objc?
-    BCFixed<1>,             // requires stored property initial values?
     BCFixed<1>,             // inherits convenience initializers from its superclass?
     GenericEnvironmentIDField, // generic environment
     TypeIDField,            // superclass

lib/AST/Decl.cpp

@@ -3679,7 +3679,6 @@ ClassDecl::ClassDecl(SourceLoc ClassLoc, Identifier Name, SourceLoc NameLoc,
     ClassLoc(ClassLoc) {
   Bits.ClassDecl.Circularity
     = static_cast<unsigned>(CircularityCheck::Unchecked);
-  Bits.ClassDecl.RequiresStoredPropertyInits = 0;
   Bits.ClassDecl.InheritsSuperclassInits = 0;
   Bits.ClassDecl.RawForeignKind = 0;
   Bits.ClassDecl.HasDestructorDecl = 0;
@@ -3835,6 +3834,9 @@ AncestryOptions ClassDecl::checkAncestry() const {
     if (CD->hasResilientMetadata(M, ResilienceExpansion::Maximal))
       result |= AncestryFlags::ResilientOther;
 
+    if (CD->getAttrs().hasAttribute<RequiresStoredPropertyInitsAttr>())
+      result |= AncestryFlags::RequiresStoredPropertyInits;
+
     CD = CD->getSuperclassDecl();
   } while (CD != nullptr);
 

lib/ClangImporter/ImportDecl.cpp

@@ -4436,7 +4436,6 @@ namespace {
         auto a = new (Impl.SwiftContext)
           RequiresStoredPropertyInitsAttr(/*IsImplicit=*/true);
         decl->getAttrs().add(a);
-        cast<ClassDecl>(decl)->setRequiresStoredPropertyInits(true);
       }
     }
 

lib/Sema/TypeCheckDecl.cpp

@@ -3708,14 +3708,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
 
     validateAttributes(*this, D);
 
-    if (auto CD = dyn_cast<ClassDecl>(nominal)) {
-      // Determine whether we require in-class initializers.
-      if (CD->getAttrs().hasAttribute<RequiresStoredPropertyInitsAttr>() ||
-          (CD->hasSuperclass() &&
-           CD->getSuperclassDecl()->requiresStoredPropertyInits()))
-        CD->setRequiresStoredPropertyInits(true);
-    }
-
     if (auto *ED = dyn_cast<EnumDecl>(nominal)) {
       // @objc enums use their raw values as the value representation, so we
       // need to force the values to be checked.

lib/Serialization/Deserialization.cpp

@@ -3522,7 +3522,7 @@ class swift::DeclDeserializer {
                                     StringRef blobData) {
     IdentifierID nameID;
     DeclContextID contextID;
-    bool isImplicit, isObjC, requiresStoredPropertyInits;
+    bool isImplicit, isObjC;
     bool inheritsSuperclassInitializers;
     GenericEnvironmentID genericEnvID;
     TypeID superclassID;
@@ -3531,7 +3531,6 @@ class swift::DeclDeserializer {
     ArrayRef<uint64_t> rawInheritedAndDependencyIDs;
     decls_block::ClassLayout::readRecord(scratch, nameID, contextID,
                                          isImplicit, isObjC,
-                                         requiresStoredPropertyInits,
                                          inheritsSuperclassInitializers,
                                          genericEnvID, superclassID,
                                          rawAccessLevel, numConformances,
@@ -3575,8 +3574,6 @@ class swift::DeclDeserializer {
       theClass->setImplicit();
     theClass->setIsObjC(isObjC);
     theClass->setSuperclass(MF.getType(superclassID));
-    if (requiresStoredPropertyInits)
-      theClass->setRequiresStoredPropertyInits(true);
     if (inheritsSuperclassInitializers)
       theClass->setInheritsSuperclassInitializers();
 

lib/Serialization/Serialization.cpp

@@ -3301,7 +3301,6 @@ void Serializer::writeDecl(const Decl *D) {
                             contextID,
                             theClass->isImplicit(),
                             theClass->isObjC(),
-                            theClass->requiresStoredPropertyInits(),
                             inheritsSuperclassInitializers,
                             addGenericEnvironmentRef(
                                              theClass->getGenericEnvironment()),

test/SILGen/Inputs/ivar_initializer_other.swift

@@ -0,0 +1,6 @@
+
+import Foundation
+
+@requires_stored_property_inits public class OtherBase : NSObject {}
+
+public class OtherMiddle : OtherBase {}

test/SILGen/ivar_initializer.swift

@@ -0,0 +1,26 @@
+// RUN: %empty-directory(%t)
+// RUN: %build-clang-importer-objc-overlays
+
+// RUN: %target-swift-emit-silgen(mock-sdk: %clang-importer-sdk-nosource -I %t) -primary-file %s %S/Inputs/ivar_initializer_other.swift | %FileCheck %s
+
+// REQUIRES: objc_interop
+
+import Foundation
+
+@requires_stored_property_inits public class MyBase : NSObject {}
+
+public class MyMiddle : MyBase {}
+
+public class C {}
+
+public class MyDerived : MyMiddle {
+    var c = C()
+}
+
+public class OtherDerived : OtherMiddle {
+    var c = C()
+}
+
+// CHECK-LABEL: sil hidden [ossa] @$s16ivar_initializer9MyDerivedCfeTo : $@convention(objc_method) (@owned MyDerived) -> @owned MyDerived {
+
+// CHECK-LABEL: sil hidden [ossa] @$s16ivar_initializer12OtherDerivedCfeTo : $@convention(objc_method) (@owned OtherDerived) -> @owned OtherDerived {