Restore initializing entry points for @objc convenience initializers

This undoes some of Joe's work in 8665342 to add a guarantee: if an
@objc convenience initializer only calls other @objc initializers that
eventually call a designated initializer, it won't result in an extra
allocation. While Objective-C /allows/ returning a different object
from an initializer than the allocation you were given, doing so
doesn't play well with some very hairy implementation details of
compiled nib files (or NSCoding archives with cyclic references in
general).

This guarantee only applies to
(1) calling `self.init`
(2) where the delegated-to initializer is @objc
because convenience initializers must do dynamic dispatch when they
delegate, and Swift only stores allocating entry points for
initializers in a class's vtable. To dynamically find an initializing
entry point, ObjC dispatch must be used instead.

(It's worth noting that this patch does NOT check that the calling
initializer is a convenience initializer when deciding whether to use
ObjC dispatch for `self.init`. If we ever add peer delegation to
designated initializers, which is totally a valid feature, that should
use static dispatch and therefore should not go through objc_msgSend.)

This change doesn't /always/ result in fewer allocations; if the
delegated-to initializer ends up returning a different object after
all, the original allocation was wasted. Objective-C has the same
problem (one of the reasons why factory methods exist for things like
NSNumber and NSArray).

We do still get most of the benefits of Joe's original change. In
particular, vtables only ever contain allocating initializer entry
points, never the initializing ones, and never /both/ (which was a
thing that could happen with 'required' before).

rdar://problem/46823518

Author: jrose-apple

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