Merge pull request #9584 from rjmccall/existential-lvalue-access-ordering

Fix the access order for open-existential l-values

Author: rjmccall

lib/SILGen/SILGenLValue.cpp

@@ -589,22 +589,12 @@ namespace {
                         AccessKind accessKind) && override {
       assert(base.getType().isExistentialType() &&
              "base for open existential component must be an existential");
-      auto addr = SGF.B.createOpenExistentialAddr(
-          loc, base.getLValueAddress(), getTypeOfRValue().getAddressType(),
-          getOpenedExistentialAccessFor(accessKind));
+      assert(base.getType().isAddress() &&
+             "base value of open-existential component was not an address?");
 
-      if (base.hasCleanup()) {
-        assert(false && "I believe that we should never end up here. One, we "
-                        "assert above that base is an l-value address and we "
-                        "state l-values don't have associated cleanup. Two, we "
-                        "enter deinit of the buffer but don't have "
-                        "book-keeping for the value. Three, I believe that "
-                        "would mean to have a l-value passed at +1 which I "
-                        "don't believe we do.");
-        // Leave a cleanup to deinit the existential container.
-        SGF.enterDeinitExistentialCleanup(base.getValue(), CanType(),
-                                          ExistentialRepresentation::Opaque);
-      }
+      SILValue addr = SGF.B.createOpenExistentialAddr(
+          loc, base.getValue(), getTypeOfRValue().getAddressType(),
+          getOpenedExistentialAccessFor(accessKind));
 
       SGF.setArchetypeOpeningSite(cast<ArchetypeType>(getSubstFormalType()),
                                   addr);
@@ -1926,14 +1916,7 @@ LValue SILGenLValue::visitOpaqueValueExpr(OpaqueValueExpr *e,
     openedExistentials.erase(known);
 
     // Do formal evaluation of the underlying existential lvalue.
-    LValue existentialLV = visitRec(opened->getExistentialValue(), accessKind);
-
-    ManagedValue existentialAddr
-      = SGF.emitAddressOfLValue(e, std::move(existentialLV), accessKind);
-
-    // Open up the existential.
-    LValue lv;
-    lv.add<ValueComponent>(existentialAddr, None, existentialLV.getTypeData());
+    LValue lv = visitRec(opened->getExistentialValue(), accessKind);
     lv.add<OpenOpaqueExistentialComponent>(
       cast<ArchetypeType>(opened->getOpenedArchetype()->getCanonicalType()));
     return lv;

test/SILGen/assignment.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -emit-silgen %s | %FileCheck %s
+// RUN: %target-swift-frontend -emit-silgen -enforce-exclusivity=checked %s | %FileCheck %s
 
 class C {}
 
@@ -14,7 +14,7 @@ var a = A()
 class D { var child: C = C() }
 
 // Verify that the LHS is formally evaluated before the RHS.
-// CHECK: sil hidden @_T010assignment5test1yyF : $@convention(thin) () -> () {
+// CHECK-LABEL: sil hidden @_T010assignment5test1yyF : $@convention(thin) () -> () {
 func test1() {
   // CHECK: [[CTOR:%.*]] = function_ref @_T010assignment1DC{{[_0-9a-zA-Z]*}}fC
   // CHECK: [[T0:%.*]] = metatype $@thick D.Type
@@ -26,3 +26,23 @@ func test1() {
   // CHECK: apply [[SETTER]]([[C]], [[D]])
   D().child = C()
 }
+
+// rdar://32039566
+protocol P {
+  var left: Int {get set}
+  var right: Int {get set}
+}
+
+// Verify that the access to the LHS does not begin until after the
+// RHS is formally evaluated.
+// CHECK-LABEL: sil hidden @_T010assignment15copyRightToLeftyAA1P_pz1p_tF : $@convention(thin) (@inout P) -> () {
+func copyRightToLeft(p: inout P) {
+  // CHECK: bb0(%0 : $*P):
+  // CHECK:   [[READ:%.*]] = begin_access [read] [unknown] %0 : $*P
+  // CHECK:   [[READ_OPEN:%.*]] = open_existential_addr immutable_access [[READ]]
+  // CHECK:   end_access [[READ]] : $*P
+  // CHECK:   [[WRITE:%.*]] = begin_access [modify] [unknown] %0 : $*P
+  // CHECK:   [[WRITE_OPEN:%.*]] = open_existential_addr mutable_access [[WRITE]]
+  // CHECK:   end_access [[WRITE]] : $*P
+  p.left = p.right
+}