Restore initializing entry points for @objc convenience initializers

docs/SIL.rst

@@ -2345,6 +2345,7 @@ mark_uninitialized
   mu_kind ::= 'derivedself'
   mu_kind ::= 'derivedselfonly'
   mu_kind ::= 'delegatingself'
+  mu_kind ::= 'delegatingselfallocated'
 
   %2 = mark_uninitialized [var] %1 : $*T
   // $T must be an address
@@ -2365,6 +2366,7 @@ the mark_uninitialized instruction refers to:
 - ``derivedself``: designates ``self`` in a derived (non-root) class
 - ``derivedselfonly``: designates ``self`` in a derived (non-root) class whose stored properties have already been initialized
 - ``delegatingself``: designates ``self`` on a struct, enum, or class in a delegating constructor (one that calls self.init)
+- ``delegatingselfallocated``: designates ``self`` on a class convenience initializer's initializing entry point
 
 The purpose of the ``mark_uninitialized`` instruction is to enable
 definitive initialization analysis for global variables (when marked as

include/swift/SIL/SILInstruction.h

@@ -3654,6 +3654,10 @@ class MarkUninitializedInst
     /// DelegatingSelf designates "self" on a struct, enum, or class
     /// in a delegating constructor (one that calls self.init).
     DelegatingSelf,
+
+    /// DelegatingSelfAllocated designates "self" in a delegating class
+    /// initializer where memory has already been allocated.
+    DelegatingSelfAllocated,
   };
 private:
   Kind ThisKind;
@@ -3683,6 +3687,9 @@ class MarkUninitializedInst
   bool isDelegatingSelf() const {
     return ThisKind == DelegatingSelf;
   }
+  bool isDelegatingSelfAllocated() const {
+    return ThisKind == DelegatingSelfAllocated;
+  }
 };
 
 /// MarkFunctionEscape - Represents the escape point of set of variables due to

lib/ParseSIL/ParseSIL.cpp

@@ -3397,10 +3397,12 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
       Kind = MarkUninitializedInst::DerivedSelfOnly;
     else if (KindId.str() == "delegatingself")
       Kind = MarkUninitializedInst::DelegatingSelf;
+    else if (KindId.str() == "delegatingselfallocated")
+      Kind = MarkUninitializedInst::DelegatingSelfAllocated;
     else {
       P.diagnose(KindLoc, diag::expected_tok_in_sil_instr,
                  "var, rootself, crossmodulerootself, derivedself, "
-                 "derivedselfonly, or delegatingself");
+                 "derivedselfonly, delegatingself, or delegatingselfallocated");
       return true;
     }
 

lib/SIL/SILPrinter.cpp

@@ -1297,6 +1297,9 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
       *this << "[derivedselfonly] ";
       break;
     case MarkUninitializedInst::DelegatingSelf: *this << "[delegatingself] ";break;
+    case MarkUninitializedInst::DelegatingSelfAllocated:
+      *this << "[delegatingselfallocated] ";
+      break;
     }
 
     *this << getIDAndType(MU->getOperand());

lib/SIL/SILVerifier.cpp

@@ -2287,9 +2287,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
         ->getInstanceType()->getClassOrBoundGenericClass();
     require(class2,
             "Second operand of dealloc_partial_ref must be a class metatype");
-    require(!class2->isResilient(F.getModule().getSwiftModule(),
-                                 F.getResilienceExpansion()),
-            "cannot directly deallocate resilient class");
     while (class1 != class2) {
       class1 = class1->getSuperclassDecl();
       require(class1, "First operand not superclass of second instance type");

lib/SILGen/SILGen.cpp

@@ -854,33 +854,18 @@ void SILGenModule::emitConstructor(ConstructorDecl *decl) {
 
   bool ForCoverageMapping = doesASTRequireProfiling(M, decl);
 
-  auto emitClassAllocatorThunk = [&]{
-    emitOrDelayFunction(
-        *this, constant, [this, constant, decl](SILFunction *f) {
-          preEmitFunction(constant, decl, f, decl);
-          PrettyStackTraceSILFunction X("silgen emitConstructor", f);
-          SILGenFunction(*this, *f, decl).emitClassConstructorAllocator(decl);
-          postEmitFunction(constant, f);
-        });
-  };
-  auto emitValueConstructorIfHasBody = [&]{
-    if (decl->hasBody()) {
+  if (declCtx->getSelfClassDecl()) {
+    // Designated initializers for classes, as well as @objc convenience
+    // initializers, have have separate entry points for allocation and
+    // initialization.
+    if (decl->isDesignatedInit() || decl->isObjC()) {
       emitOrDelayFunction(
-          *this, constant, [this, constant, decl, declCtx](SILFunction *f) {
+          *this, constant, [this, constant, decl](SILFunction *f) {
             preEmitFunction(constant, decl, f, decl);
             PrettyStackTraceSILFunction X("silgen emitConstructor", f);
-            f->setProfiler(getOrCreateProfilerForConstructors(declCtx, decl));
-            SILGenFunction(*this, *f, decl).emitValueConstructor(decl);
+            SILGenFunction(*this, *f, decl).emitClassConstructorAllocator(decl);
             postEmitFunction(constant, f);
           });
-    }
-  };
-
-  if (declCtx->getSelfClassDecl()) {
-    // Designated initializers for classes have have separate entry points for
-    // allocation and initialization.
-    if (decl->isDesignatedInit()) {
-      emitClassAllocatorThunk();
 
       // Constructors may not have bodies if they've been imported, or if they've
       // been parsed from a parseable interface.
@@ -900,21 +885,21 @@ void SILGenModule::emitConstructor(ConstructorDecl *decl) {
             },
             /*forceEmission=*/ForCoverageMapping);
       }
-    // Convenience initializers for classes behave more like value constructors
-    // in that there's only an allocating entry point that effectively
-    // "constructs" the self reference by invoking another initializer.
-    } else {
-      emitValueConstructorIfHasBody();
-
-      // If the convenience initializer was imported from ObjC, we still have to
-      // emit the allocator thunk.
-      if (decl->hasClangNode()) {
-        emitClassAllocatorThunk();
-      }
+      return;
     }
-  } else {
-    // Struct and enum constructors do everything in a single function.
-    emitValueConstructorIfHasBody();
+  }
+
+  // Struct and enum constructors do everything in a single function, as do
+  // non-@objc convenience initializers for classes.
+  if (decl->hasBody()) {
+    emitOrDelayFunction(
+        *this, constant, [this, constant, decl, declCtx](SILFunction *f) {
+          preEmitFunction(constant, decl, f, decl);
+          PrettyStackTraceSILFunction X("silgen emitConstructor", f);
+          f->setProfiler(getOrCreateProfilerForConstructors(declCtx, decl));
+          SILGenFunction(*this, *f, decl).emitValueConstructor(decl);
+          postEmitFunction(constant, f);
+        });
   }
 }
 

lib/SILGen/SILGenApply.cpp

@@ -1397,14 +1397,21 @@ class SILGenApply : public Lowering::ExprVisitor<SILGenApply> {
     // protocols, which only witness initializing initializers.
     else if (auto proto = dyn_cast<ProtocolDecl>(nominal)) {
       useAllocatingCtor = !proto->isObjC();
-    // Similarly, class initializers self.init-delegate to each other via
-    // their allocating entry points, unless delegating to an ObjC-only,
-    // non-factory initializer.
+    // Factory initializers are effectively "allocating" initializers with no
+    // corresponding initializing entry point.
+    } else if (ctorRef->getDecl()->isFactoryInit()) {
+       useAllocatingCtor = true;
+    // If we're emitting a class initializer's non-allocating entry point and
+    // delegating to an initializer exposed to Objective-C, use the initializing
+    // entry point to avoid replacing an existing allocated object.
+    } else if (!SGF.AllocatorMetatype && ctorRef->getDecl()->isObjC()) {
+      useAllocatingCtor = false;
+    // In general, though, class initializers self.init-delegate to each other
+    // via their allocating entry points.
     } else {
       assert(isa<ClassDecl>(nominal)
              && "some new kind of init context we haven't implemented");
-      useAllocatingCtor = ctorRef->getDecl()->isFactoryInit()
-        || !requiresForeignEntryPoint(ctorRef->getDecl());
+      useAllocatingCtor = !requiresForeignEntryPoint(ctorRef->getDecl());
     }
 
     // Load the 'self' argument.
@@ -1472,9 +1479,16 @@ class SILGenApply : public Lowering::ExprVisitor<SILGenApply> {
         arg->isSelfExprOf(
             cast<AbstractFunctionDecl>(SGF.FunctionDC->getAsDecl()), false);
 
-    constant =
-        constant.asForeign(!isObjCReplacementSelfCall &&
-                           requiresForeignEntryPoint(ctorRef->getDecl()));
+    if (!isObjCReplacementSelfCall) {
+      if (useAllocatingCtor) {
+        constant =
+            constant.asForeign(requiresForeignEntryPoint(ctorRef->getDecl()));
+      } else {
+        // Note: if we ever implement delegating from one designated initializer
+        // to another, this won't be correct; that should do a direct dispatch.
+        constant = constant.asForeign(ctorRef->getDecl()->isObjC());
+      }
+    }
 
     // Determine the callee. This is normally the allocating
     // entry point, unless we're delegating to an ObjC initializer.
@@ -1485,7 +1499,8 @@ class SILGenApply : public Lowering::ExprVisitor<SILGenApply> {
           constant, subs, expr));
     } else if ((useAllocatingCtor || constant.isForeign) &&
                !isSelfCallToReplacedInDynamicReplacement &&
-               getMethodDispatch(ctorRef->getDecl()) == MethodDispatch::Class) {
+               ((constant.isForeign && !useAllocatingCtor) ||
+                getMethodDispatch(ctorRef->getDecl()) == MethodDispatch::Class)) {
       // Dynamic dispatch to the initializer.
       Scope S(SGF, expr);
       setCallee(Callee::forClassMethod(

lib/SILGen/SILGenBridging.cpp

@@ -1388,7 +1388,7 @@ void SILGenFunction::emitNativeToForeignThunk(SILDeclRef thunk) {
   bool isInitializingToAllocatingInitThunk = false;
   if (native.kind == SILDeclRef::Kind::Initializer) {
     if (auto ctor = dyn_cast<ConstructorDecl>(native.getDecl())) {
-      if (!ctor->isDesignatedInit()) {
+      if (!ctor->isDesignatedInit() && !ctor->isObjC()) {
         isInitializingToAllocatingInitThunk = true;
         native = SILDeclRef(ctor, SILDeclRef::Kind::Allocator);
       }

lib/SILGen/SILGenConstructor.cpp

@@ -481,9 +481,10 @@ void SILGenFunction::emitClassConstructorAllocator(ConstructorDecl *ctor) {
   // Allocate the 'self' value.
   bool useObjCAllocation = usesObjCAllocator(selfClassDecl);
 
-  if (ctor->hasClangNode()) {
-    // For an allocator thunk synthesized for an Objective-C init method,
-    // allocate using the metatype.
+  if (ctor->hasClangNode() || ctor->isConvenienceInit()) {
+    assert(ctor->hasClangNode() || ctor->isObjC());
+    // For an allocator thunk synthesized for an @objc convenience initializer
+    // or imported Objective-C init method, allocate using the metatype.
     SILValue allocArg = selfMetaValue;
 
     // When using Objective-C allocation, convert the metatype
@@ -572,10 +573,13 @@ void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
   // enforce its DI properties on stored properties.
   MarkUninitializedInst::Kind MUKind;
 
-  if (isDelegating)
-    MUKind = MarkUninitializedInst::DelegatingSelf;
-  else if (selfClassDecl->requiresStoredPropertyInits() &&
-           usesObjCAllocator) {
+  if (isDelegating) {
+    if (ctor->isObjC())
+      MUKind = MarkUninitializedInst::DelegatingSelfAllocated;
+    else
+      MUKind = MarkUninitializedInst::DelegatingSelf;
+  } else if (selfClassDecl->requiresStoredPropertyInits() &&
+             usesObjCAllocator) {
     // Stored properties will be initialized in a separate
     // .cxx_construct method called by the Objective-C runtime.
     assert(selfClassDecl->hasSuperclass() &&

lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp

@@ -1806,7 +1806,8 @@ collectClassInitSelfLoadUses(MarkUninitializedInst *MUI,
 //===----------------------------------------------------------------------===//
 
 static bool shouldPerformClassInitSelf(const DIMemoryObjectInfo &MemoryInfo) {
-  assert(!MemoryInfo.isDelegatingInit());
+  if (MemoryInfo.MemoryInst->isDelegatingSelfAllocated())
+    return true;
 
   return MemoryInfo.isNonDelegatingInit() &&
          MemoryInfo.getType()->getClassOrBoundGenericClass() != nullptr &&
@@ -1820,6 +1821,12 @@ void swift::ownership::collectDIElementUsesFrom(
     const DIMemoryObjectInfo &MemoryInfo, DIElementUseInfo &UseInfo,
     bool isDIFinished, bool TreatAddressToPointerAsInout) {
 
+  if (shouldPerformClassInitSelf(MemoryInfo)) {
+    ClassInitElementUseCollector UseCollector(MemoryInfo, UseInfo);
+    UseCollector.collectClassInitSelfUses();
+    return;
+  }
+
   if (MemoryInfo.isDelegatingInit()) {
     // When we're analyzing a delegating constructor, we aren't field sensitive
     // at all. Just treat all members of self as uses of the single
@@ -1829,12 +1836,6 @@ void swift::ownership::collectDIElementUsesFrom(
     return;
   }
 
-  if (shouldPerformClassInitSelf(MemoryInfo)) {
-    ClassInitElementUseCollector UseCollector(MemoryInfo, UseInfo);
-    UseCollector.collectClassInitSelfUses();
-    return;
-  }
-
   ElementUseCollector(MemoryInfo, UseInfo, isDIFinished,
                       TreatAddressToPointerAsInout)
       .collectFrom();

lib/SILOptimizer/Mandatory/DIMemoryUseCollector.h

@@ -119,9 +119,9 @@ class DIMemoryObjectInfo {
   /// True if the memory object is the 'self' argument of a class designated
   /// initializer.
   bool isClassInitSelf() const {
-    if (isDelegatingInit())
+    if (MemoryInst->isDelegatingSelf())
       return false;
-      
+
     if (!MemoryInst->isVar()) {
       if (auto decl = getType()->getAnyNominal()) {
         if (isa<ClassDecl>(decl)) {
@@ -153,14 +153,13 @@ class DIMemoryObjectInfo {
     return isClassInitSelf() && !MemoryInst->isRootSelf();
   }
 
-  /// isDelegatingInit - True if this is a delegating initializer, one that
-  /// calls 'self.init'.
+  /// True if this is a delegating initializer, one that calls 'self.init'.
   bool isDelegatingInit() const {
-    return MemoryInst->isDelegatingSelf();
+    return MemoryInst->isDelegatingSelf() ||
+           MemoryInst->isDelegatingSelfAllocated();
   }
 
-  /// isNonDelegatingInit - True if this is an initializer that initializes
-  /// stored properties.
+  /// True if this is an initializer that initializes stored properties.
   bool isNonDelegatingInit() const {
     switch (MemoryInst->getKind()) {
     case MarkUninitializedInst::Var:
@@ -171,6 +170,7 @@ class DIMemoryObjectInfo {
     case MarkUninitializedInst::DerivedSelfOnly:
       return true;
     case MarkUninitializedInst::DelegatingSelf:
+    case MarkUninitializedInst::DelegatingSelfAllocated:
       return false;
     }
     return false;

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

@@ -1927,13 +1927,12 @@ void LifetimeChecker::processUninitializedRelease(SILInstruction *Release,
     auto SILMetatypeTy = SILType::getPrimitiveObjectType(MetatypeTy);
     SILValue Metatype;
 
-    // A convenience initializer should never deal in partially allocated
-    // objects.
-    assert(!TheMemory.isDelegatingInit());
-
-    // In a designated initializer, we know the class of the thing
-    // we're cleaning up statically.
-    Metatype = B.createMetatype(Loc, SILMetatypeTy);
+    // In an inherited convenience initializer, we must use the dynamic
+    // type of the object since nothing is initialized yet.
+    if (TheMemory.isDelegatingInit())
+      Metatype = B.createValueMetatype(Loc, SILMetatypeTy, Pointer);
+    else
+      Metatype = B.createMetatype(Loc, SILMetatypeTy);
 
     // We've already destroyed any instance variables initialized by this
     // constructor, now destroy instance variables initialized by subclass

test/Interpreter/SDK/Foundation_test.swift

@@ -433,6 +433,36 @@ if #available(OSX 10.11, iOS 9.0, *) {
 
     KU.finishDecoding()
   }
+
+  FoundationTestSuite.test("NSKeyedUnarchiver/CycleWithConvenienceInit") {
+    @objc(HasACyclicReference)
+    class HasACyclicReference: NSObject, NSCoding {
+      var ref: AnyObject?
+      override init() {
+        super.init()
+        ref = self
+      }
+
+      required convenience init?(coder: NSCoder) {
+        let decodedRef = coder.decodeObject(forKey: "ref") as AnyObject?
+        self.init(ref: decodedRef)
+      }
+      @objc init(ref: AnyObject?) {
+        self.ref = ref
+        super.init()
+      }
+
+      func encode(with coder: NSCoder) {
+        coder.encode(ref, forKey: "ref")
+      }
+    }
+
+    let data = NSKeyedArchiver.archivedData(withRootObject: HasACyclicReference())
+    let decodedObj = NSKeyedUnarchiver.unarchiveObject(with: data) as! HasACyclicReference
+
+    expectEqual(ObjectIdentifier(decodedObj), ObjectIdentifier(decodedObj.ref!),
+                "cycle was not preserved (due to object replacement?)")
+  }
 }
 
 FoundationTestSuite.test("NotificationCenter/addObserver(_:selector:name:object:)") {

test/Interpreter/SDK/Inputs/convenience_init_peer_delegation.h

@@ -0,0 +1,14 @@
+@import Foundation;
+
+extern NSInteger baseCounter;
+extern NSInteger subCounter;
+
+@interface Base : NSObject
+- (nonnull instancetype)init __attribute__((objc_designated_initializer));
+- (nonnull instancetype)initConveniently;
++ (nonnull instancetype)baseWithConvenientFactory:(_Bool)unused;
++ (nonnull Base *)baseWithNormalFactory:(_Bool)unused;
+@end
+
+@interface Sub : Base
+@end