Merge pull request #24758 from slavapestov/dynamic-self-in-convenience-init

 IRGen: Fix DynamicSelfType metadata recovery in @objc convenience inializers

Author: slavapestov

lib/IRGen/GenHeap.cpp

@@ -1684,17 +1684,43 @@ void IRGenFunction::emit##ID(llvm::Value *value, Address src) {       \
 
 llvm::Value *IRGenFunction::getLocalSelfMetadata() {
   assert(LocalSelf && "no local self metadata");
+
+  // If we already have a metatype, just return it.
+  if (SelfKind == SwiftMetatype)
+    return LocalSelf;
+
+  // We need to materialize a metatype. Emit the code for that once at the
+  // top of the function and cache the result.
+
+  // This is a slight optimization in the case of repeated access, but also
+  // needed for correctness; when an @objc convenience initializer replaces
+  // the 'self' value, we don't keep track of what the new 'self' value is
+  // in IRGen, so we can't just grab the first function argument and assume
+  // it's a valid 'self' at the point where DynamicSelfType metadata is needed.
+
+  // Note that if DynamicSelfType was modeled properly as an opened archetype,
+  // none of this would be an issue since it would be always be associated
+  // with the correct value.
+
+  llvm::IRBuilderBase::InsertPointGuard guard(Builder);
+  Builder.SetInsertPoint(&CurFn->getEntryBlock(),
+                         CurFn->getEntryBlock().begin());
+
   switch (SelfKind) {
   case SwiftMetatype:
-    return LocalSelf;
+    llvm_unreachable("Already handled");
   case ObjCMetatype:
-    return emitObjCMetadataRefForMetadata(*this, LocalSelf);
+    LocalSelf = emitObjCMetadataRefForMetadata(*this, LocalSelf);
+    SelfKind = SwiftMetatype;
+    break;
   case ObjectReference:
-    return emitDynamicTypeOfOpaqueHeapObject(*this, LocalSelf,
+    LocalSelf = emitDynamicTypeOfOpaqueHeapObject(*this, LocalSelf,
                                              MetatypeRepresentation::Thick);
+    SelfKind = SwiftMetatype;
+    break;
   }
 
-  llvm_unreachable("Not a valid LocalSelfKind.");
+  return LocalSelf;
 }
 
 /// Given a non-tagged object pointer, load a pointer to its class object.

lib/IRGen/IRBuilder.h

@@ -109,67 +109,6 @@ class IRBuilder : public IRBuilderBase {
     IRBuilderBase::SetInsertPoint(I);
   }
 
-  /// A stable insertion point in the function.  "Stable" means that
-  /// it will point to the same location in the function, even if
-  /// instructions are subsequently added to the current basic block.
-  class StableIP {
-    /// Either an instruction that we're inserting after or the basic
-    /// block that we're inserting at the beginning of.
-    using UnionTy = llvm::PointerUnion<llvm::Instruction *, llvm::BasicBlock *>;
-    UnionTy After;
-  public:
-    StableIP() = default;
-    explicit StableIP(const IRBuilder &Builder) {
-      if (!Builder.hasValidIP()) {
-        After = UnionTy();
-        assert(!isValid());
-        return;
-      }
-
-      llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
-      assert(Builder.GetInsertPoint() == curBlock->end());
-      if (curBlock->empty())
-        After = curBlock;
-      else
-        After = &curBlock->back();
-    }
-
-    /// Does this stable IP point to a valid location?
-    bool isValid() const {
-      return !After.isNull();
-    }
-
-    /// Insert an unparented instruction at this insertion point.
-    /// Note that inserting multiple instructions at an IP will cause
-    /// them to end up in reverse order.
-    void insert(llvm::Instruction *I) {
-      assert(isValid() && "inserting at invalid location!");
-      assert(I->getParent() == nullptr);
-      if (auto *block = After.dyn_cast<llvm::BasicBlock*>()) {
-        block->getInstList().push_front(I);
-      } else {
-        llvm::Instruction *afterInsn = After.get<llvm::Instruction*>();
-        afterInsn->getParent()->getInstList().insertAfter(
-            afterInsn->getIterator(), I);
-      }
-    }
-
-    // Support for being placed in pointer unions.
-    void *getOpaqueValue() const { return After.getOpaqueValue(); }
-    static StableIP getFromOpaqueValue(void *p) {
-      StableIP result;
-      result.After = UnionTy::getFromOpaqueValue(p);
-      return result;
-    }
-    enum { NumLowBitsAvailable
-             = llvm::PointerLikeTypeTraits<UnionTy>::NumLowBitsAvailable };
-  };
-
-  /// Capture a stable reference to the current IP.
-  StableIP getStableIP() const {
-    return StableIP(*this);
-  }
-
   /// Return the LLVM module we're inserting into.
   llvm::Module *getModule() const {
     if (auto BB = GetInsertBlock())
@@ -397,23 +336,4 @@ class IRBuilder : public IRBuilderBase {
 } // end namespace irgen
 } // end namespace swift
 
-namespace llvm {
-  template <> struct PointerLikeTypeTraits<swift::irgen::IRBuilder::StableIP> {
-    using type = swift::irgen::IRBuilder::StableIP;
-
-  public:
-    static void *getAsVoidPointer(type IP) {
-      return IP.getOpaqueValue();
-    }
-    static type getFromVoidPointer(void *p) {
-      return type::getFromOpaqueValue(p);
-    }
-
-    // The number of bits available are the min of the two pointer types.
-    enum {
-      NumLowBitsAvailable = type::NumLowBitsAvailable
-    };
-  };
-}
-
 #endif

test/IRGen/dynamic_self_metadata.swift

@@ -50,8 +50,6 @@ class C {
   // CHECK-LABEL: define hidden swiftcc i64 @"$s21dynamic_self_metadata1CC0A18SelfConformingTypeACXDSgyF"(%T21dynamic_self_metadata1CC* swiftself)
   // CHECK: [[SELF:%.*]] = bitcast %T21dynamic_self_metadata1CC* %0 to %objc_object*
   // CHECK: [[SELF_TYPE:%.*]] = call %swift.type* @swift_getObjectType(%objc_object* [[SELF]])
-  // CHECK: [[SELF:%.*]] = bitcast %T21dynamic_self_metadata1CC* %0 to %objc_object*
-  // CHECK: [[SELF_TYPE:%.*]] = call %swift.type* @swift_getObjectType(%objc_object* [[SELF]])
   // CHECK: [[METADATA_RESPONSE:%.*]] = call swiftcc %swift.metadata_response @"$s21dynamic_self_metadata1GVMa"(i64 0, %swift.type* [[SELF_TYPE]])
   // CHECK: [[METADATA:%.*]] =  extractvalue %swift.metadata_response [[METADATA_RESPONSE]], 0
   // CHECK: call i8** @swift_getWitnessTable(%swift.protocol_conformance_descriptor* bitcast ({{.*}} @"$s21dynamic_self_metadata1GVyxGAA1PAAMc" to %swift.protocol_conformance_descriptor*), %swift.type* [[METADATA]], i8*** undef)

test/Interpreter/convenience_init_swift5.swift

@@ -0,0 +1,37 @@
+// RUN: %empty-directory(%t)
+
+// RUN: %target-build-swift %s -o %t/main -swift-version 5
+// RUN: %target-codesign %t/main
+// RUN: %target-run %t/main
+
+// REQUIRES: executable_test
+// REQUIRES: objc_interop
+
+import Foundation
+import StdlibUnittest
+
+func foo(_ x: @escaping () -> ()) { x() }
+
+public class C {
+  var x: Int = 0
+
+  init(blah: ()) {}
+
+  @objc convenience init() {
+    self.init(blah: ())
+
+    foo { [weak self] in
+      guard let `self` = self else { return }
+      self.x += 1
+    }
+  }
+}
+
+var ConvenienceInitSelfTest = TestSuite("ConvenienceInitSelf")
+
+ConvenienceInitSelfTest.test("SelfMetadata") {
+  let c = C()
+  expectEqual(c.x, 1)
+}
+
+runAllTests()