Merge pull request #14461 from jckarter/open-existential-lvalue-evaluation-order-5.0

[5.0] SILGen: Open existential lvalues on entry into an OpenExistentialExpr again.

Author: jckarter

lib/SILGen/SILGenExpr.cpp

@@ -5091,15 +5091,34 @@ RValue RValueEmitter::emitForceValue(ForceValueExpr *loc, Expr *E,
 void SILGenFunction::emitOpenExistentialExprImpl(
        OpenExistentialExpr *E,
        llvm::function_ref<void(Expr *)> emitSubExpr) {
-  Optional<FormalEvaluationScope> writebackScope;
-
   // Emit the existential value.
   if (E->getExistentialValue()->getType()->is<LValueType>()) {
-    bool inserted = OpaqueValueExprs.insert({E->getOpaqueValue(), E}).second;
+    // Open the existential container right away. We need the dynamic type
+    // to be opened in order to evaluate the subexpression.
+    AccessKind accessKind;
+    if (E->hasLValueAccessKind())
+      accessKind = E->getLValueAccessKind();
+    else
+      accessKind = E->getExistentialValue()->getLValueAccessKind();
+    auto lv = emitLValue(E->getExistentialValue(), accessKind);
+    auto formalOpenedType = E->getOpaqueValue()->getType()
+                                    ->getWithoutSpecifierType()
+                                    ->getCanonicalType();
+    lv = emitOpenExistentialLValue(E, std::move(lv),
+                                   CanArchetypeType(E->getOpenedArchetype()),
+                                   formalOpenedType,
+                                   accessKind);
+    
+    auto addr = emitAddressOfLValue(E, std::move(lv), accessKind);
+    bool inserted = OpaqueLValues.insert({E->getOpaqueValue(),
+                                          {addr.getValue(), formalOpenedType}})
+      .second;
     (void)inserted;
     assert(inserted && "already have this opened existential?");
 
     emitSubExpr(E->getSubExpr());
+    assert(OpaqueLValues.count(E->getOpaqueValue()) == 0
+           && "opened existential not removed?");
     return;
   }
 
@@ -5126,6 +5145,12 @@ RValue RValueEmitter::visitOpenExistentialExpr(OpenExistentialExpr *E,
     return RValue(SGF, E, *result);
   }
 
+  Optional<FormalEvaluationScope> scope;
+  // Begin an evaluation scope for an lvalue existential opened into an
+  // rvalue expression.
+  if (E->getExistentialValue()->getType()->is<LValueType>())
+    scope.emplace(SGF);
+  
   return SGF.emitOpenExistentialExpr<RValue>(E,
                                              [&](Expr *subExpr) -> RValue {
                                                return visit(subExpr, C);

lib/SILGen/SILGenFunction.h

@@ -1417,10 +1417,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   llvm::SmallDenseMap<OpaqueValueExpr *, OpaqueValueState>
     OpaqueValues;
 
-  /// A mapping from opaque value expressions to the open-existential
-  /// expression that determines them, used while lowering lvalues.
-  llvm::SmallDenseMap<OpaqueValueExpr *, OpenExistentialExpr *>
-    OpaqueValueExprs;
+  /// A mapping from opaque value lvalue expressions to the address of the
+  /// opened value in memory.
+  llvm::SmallDenseMap<OpaqueValueExpr *, std::pair<SILValue, CanType>>
+    OpaqueLValues;
 
   /// RAII object that introduces a temporary binding for an opaque value.
   ///

lib/SILGen/SILGenLValue.cpp

@@ -2208,20 +2208,18 @@ LValue SILGenLValue::visitOpaqueValueExpr(OpaqueValueExpr *e,
                                           AccessKind accessKind,
                                           LValueOptions options) {
   // Handle an opaque lvalue that refers to an opened existential.
-  auto known = SGF.OpaqueValueExprs.find(e);
-  if (known != SGF.OpaqueValueExprs.end()) {
-    // Dig the open-existential expression out of the list.
-    OpenExistentialExpr *opened = known->second;
-    SGF.OpaqueValueExprs.erase(known);
-
-    // Do formal evaluation of the underlying existential lvalue.
-    auto lv = visitRec(opened->getExistentialValue(), accessKind, options);
-    lv = SGF.emitOpenExistentialLValue(
-        opened, std::move(lv),
-        CanArchetypeType(opened->getOpenedArchetype()),
-        e->getType()->getWithoutSpecifierType()->getCanonicalType(),
-        accessKind);
-    return lv;
+  auto known = SGF.OpaqueLValues.find(e);
+  if (known != SGF.OpaqueLValues.end()) {
+    // Dig the opened address out of the list.
+    SILValue opened = known->second.first;
+    CanType openedType = known->second.second;
+    SGF.OpaqueLValues.erase(known);
+
+    // Continue formal evaluation of the lvalue from this address.
+    return LValue::forAddress(ManagedValue::forLValue(opened),
+                              None,
+                              AbstractionPattern(openedType),
+                              openedType);
   }
 
   assert(SGF.OpaqueValues.count(e) && "Didn't bind OpaqueValueExpr");
@@ -2513,29 +2511,12 @@ LValue SILGenLValue::visitTupleElementExpr(TupleElementExpr *e,
 LValue SILGenLValue::visitOpenExistentialExpr(OpenExistentialExpr *e,
                                               AccessKind accessKind,
                                               LValueOptions options) {
-  // If the opaque value is not an lvalue, open the existential immediately.
-  if (!e->getOpaqueValue()->getType()->is<LValueType>()) {
-    return SGF.emitOpenExistentialExpr<LValue>(e,
-                                               [&](Expr *subExpr) -> LValue {
-                                                 return visitRec(subExpr,
-                                                                 accessKind,
-                                                                 options);
-                                               });
-  }
-
-  // Record the fact that we're opening this existential. The actual
-  // opening operation will occur when we see the OpaqueValueExpr.
-  bool inserted = SGF.OpaqueValueExprs.insert({e->getOpaqueValue(), e}).second;
-  (void)inserted;
-  assert(inserted && "already have this opened existential?");
-
-  // Visit the subexpression.
-  LValue lv = visitRec(e->getSubExpr(), accessKind, options);
-
-  // Sanity check that we did see the OpaqueValueExpr.
-  assert(SGF.OpaqueValueExprs.count(e->getOpaqueValue()) == 0 &&
-         "opened existential not removed?");
-  return lv;
+  return SGF.emitOpenExistentialExpr<LValue>(e,
+                                             [&](Expr *subExpr) -> LValue {
+                                               return visitRec(subExpr,
+                                                               accessKind,
+                                                               options);
+                                             });
 }
 
 static LValueTypeData

test/SILGen/assignment.swift

@@ -43,11 +43,11 @@ protocol P {
 // CHECK-LABEL: sil hidden @$S10assignment15copyRightToLeft1pyAA1P_pz_tF : $@convention(thin) (@inout P) -> () {
 func copyRightToLeft(p: inout P) {
   // CHECK: bb0(%0 : @trivial $*P):
+  // CHECK:   [[WRITE:%.*]] = begin_access [modify] [unknown] %0 : $*P
+  // CHECK:   [[WRITE_OPEN:%.*]] = open_existential_addr mutable_access [[WRITE]]
   // 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
 }

test/SILGen/class_bound_protocols.swift

@@ -192,22 +192,23 @@ func takesInheritsMutatingMethod(x: inout InheritsMutatingMethod,
   // CHECK-NEXT: [[X_PAYLOAD:%.*]] = open_existential_ref [[X_VALUE]] : $InheritsMutatingMethod to $@opened("{{.*}}") InheritsMutatingMethod
   // CHECK-NEXT: [[TEMPORARY:%.*]] = alloc_stack $@opened("{{.*}}") InheritsMutatingMethod
   // CHECK-NEXT: store [[X_PAYLOAD]] to [init] [[TEMPORARY]] : $*@opened("{{.*}}") InheritsMutatingMethod
-  // CHECK-NEXT: [[X_PAYLOAD_RELOADED:%.*]] = load [take] [[TEMPORARY]]
+  // CHECK-NEXT: [[X_PAYLOAD_RELOADED:%.*]] = load_borrow [[TEMPORARY]]
   //
   // ** *NOTE* This extra copy is here since RValue invariants enforce that all
   // ** loadable objects are actually loaded. So we form the RValue and
   // ** load... only to then need to store the value back in a stack location to
   // ** pass to an in_guaranteed method. PredictableMemOpts is able to handle this
   // ** type of temporary codegen successfully.
   // CHECK-NEXT: [[TEMPORARY_2:%.*]] = alloc_stack $@opened("{{.*}}") InheritsMutatingMethod
-  // CHECK-NEXT: store [[X_PAYLOAD_RELOADED:%.*]] to [init] [[TEMPORARY_2]]
+  // CHECK-NEXT: store_borrow [[X_PAYLOAD_RELOADED:%.*]] to [[TEMPORARY_2]]
   // 
   // CHECK-NEXT: [[METHOD:%.*]] = witness_method $@opened("{{.*}}") InheritsMutatingMethod, #HasMutatingMethod.mutatingCounter!getter.1 : <Self where Self : HasMutatingMethod> (Self) -> () -> Value, [[X_PAYLOAD]] : $@opened("{{.*}}") InheritsMutatingMethod : $@convention(witness_method: HasMutatingMethod) <τ_0_0 where τ_0_0 : HasMutatingMethod> (@in_guaranteed τ_0_0) -> Value
   // CHECK-NEXT: [[RESULT_VALUE:%.*]] = apply [[METHOD]]<@opened("{{.*}}") InheritsMutatingMethod>([[TEMPORARY_2]]) : $@convention(witness_method: HasMutatingMethod) <τ_0_0 where τ_0_0 : HasMutatingMethod> (@in_guaranteed τ_0_0) -> Value
-  // CHECK-NEXT: destroy_addr [[TEMPORARY_2]] : $*@opened("{{.*}}") InheritsMutatingMethod
   // CHECK-NEXT: dealloc_stack  [[TEMPORARY_2]]
-  // CHECK-NEXT: end_access [[X_ADDR]] : $*InheritsMutatingMethod
+  // CHECK-NEXT: end_borrow [[X_PAYLOAD_RELOADED]]
   // CHECK-NEXT: assign [[RESULT_VALUE]] to [[RESULT]] : $*Value
+  // CHECK-NEXT: destroy_addr [[TEMPORARY]]
+  // CHECK-NEXT: end_access [[X_ADDR]]
   // CHECK-NEXT: dealloc_stack [[TEMPORARY]] : $*@opened("{{.*}}") InheritsMutatingMethod
   // CHECK-NEXT: dealloc_stack [[RESULT_BOX]] : $*Value
   _ = x.mutatingCounter

test/SILGen/properties.swift

@@ -1190,12 +1190,12 @@ protocol NonmutatingProtocol {
 // CHECK-NEXT:   [[C_FIELD_PAYLOAD:%.*]] = open_existential_addr immutable_access [[C_FIELD_BOX]] : $*NonmutatingProtocol to $*@opened("{{.*}}") NonmutatingProtocol
 // CHECK-NEXT:   [[C_FIELD_COPY:%.*]] = alloc_stack $@opened("{{.*}}") NonmutatingProtocol
 // CHECK-NEXT:   copy_addr [[C_FIELD_PAYLOAD]] to [initialization] [[C_FIELD_COPY]] : $*@opened("{{.*}}") NonmutatingProtocol
-// CHECK-NEXT:   destroy_addr [[C_FIELD_BOX]] : $*NonmutatingProtocol
-// CHECK-NEXT:   destroy_value [[C]] : $ReferenceType
 // CHECK-NEXT:   [[GETTER:%.*]] = witness_method $@opened("{{.*}}") NonmutatingProtocol, #NonmutatingProtocol.x!getter.1 : <Self where Self : NonmutatingProtocol> (Self) -> () -> Int, [[C_FIELD_PAYLOAD]] : $*@opened("{{.*}}") NonmutatingProtocol : $@convention(witness_method: NonmutatingProtocol) <τ_0_0 where τ_0_0 : NonmutatingProtocol> (@in_guaranteed τ_0_0) -> Int
 // CHECK-NEXT:   [[RESULT_VALUE:%.*]] = apply [[GETTER]]<@opened("{{.*}}") NonmutatingProtocol>([[C_FIELD_COPY]]) : $@convention(witness_method: NonmutatingProtocol) <τ_0_0 where τ_0_0 : NonmutatingProtocol> (@in_guaranteed τ_0_0) -> Int
 // CHECK-NEXT:   assign [[RESULT_VALUE]] to [[UNINIT]] : $*Int
 // CHECK-NEXT:   destroy_addr [[C_FIELD_COPY]] : $*@opened("{{.*}}") NonmutatingProtocol
+// CHECK-NEXT:   destroy_addr [[C_FIELD_BOX]] : $*NonmutatingProtocol
+// CHECK-NEXT:   destroy_value [[C]] : $ReferenceType
 // CHECK-NEXT:   dealloc_stack [[C_FIELD_COPY]] : $*@opened("{{.*}}") NonmutatingProtocol
 // CHECK-NEXT:   dealloc_stack [[C_FIELD_BOX]] : $*NonmutatingProtocol
 // CHECK-NEXT:   dealloc_stack [[RESULT]] : $*Int

test/SILGen/protocols.swift

@@ -26,10 +26,10 @@ func use_subscript_rvalue_get(_ i : Int) -> Int {
 // CHECK: [[PROJ:%[0-9]+]] = open_existential_addr immutable_access [[READ]] : $*SubscriptableGet to $*[[OPENED:@opened(.*) SubscriptableGet]]
 // CHECK: [[ALLOCSTACK:%[0-9]+]] = alloc_stack $[[OPENED]]
 // CHECK: copy_addr [[PROJ]] to [initialization] [[ALLOCSTACK]] : $*[[OPENED]]
-// CHECK-NEXT: end_access [[READ]] : $*SubscriptableGet
 // CHECK-NEXT: [[METH:%[0-9]+]] = witness_method $[[OPENED]], #SubscriptableGet.subscript!getter.1
 // CHECK-NEXT: [[RESULT:%[0-9]+]] = apply [[METH]]<[[OPENED]]>(%0, [[ALLOCSTACK]])
 // CHECK-NEXT: destroy_addr [[ALLOCSTACK]]
+// CHECK-NEXT: end_access [[READ]] : $*SubscriptableGet
 // CHECK-NEXT: dealloc_stack [[ALLOCSTACK]] : $*[[OPENED]]
 // CHECK-NEXT: return [[RESULT]]
 
@@ -133,9 +133,9 @@ func use_property_rvalue_get() -> Int {
 // CHECK: [[PROJ:%[0-9]+]] = open_existential_addr immutable_access [[READ]] : $*PropertyWithGetter to $*[[OPENED:@opened(.*) PropertyWithGetter]]
 // CHECK: [[COPY:%.*]] = alloc_stack $[[OPENED]]
 // CHECK-NEXT: copy_addr [[PROJ]] to [initialization] [[COPY]] : $*[[OPENED]]
-// CHECK-NEXT: end_access [[READ]] : $*PropertyWithGetter
 // CHECK-NEXT: [[METH:%[0-9]+]] = witness_method $[[OPENED]], #PropertyWithGetter.a!getter.1
 // CHECK-NEXT: apply [[METH]]<[[OPENED]]>([[COPY]])
+// CHECK: end_access [[READ]] : $*PropertyWithGetter
 
 func use_property_lvalue_get() -> Int {
   return propertyGetSet.b
@@ -458,3 +458,15 @@ public func test(_ p: Proto) {
 // CHECK-LABEL: sil_witness_table hidden StructWithStoredClassProperty: PropertyWithGetter module protocols {
 // CHECK-NEXT:  method #PropertyWithGetter.a!getter.1: {{.*}} : @$S9protocols29StructWithStoredClassPropertyVAA0fC6GetterA2aDP1aSivgTW
 // CHECK-NEXT: }
+
+//
+// rdar://problem/37031037
+//
+
+protocol MethodWithDefaultArgGenerator {}
+extension MethodWithDefaultArgGenerator {
+  mutating func foo(_ x: Int = 0) {}
+}
+func invokeMethodWithDefaultArg(x: inout MethodWithDefaultArgGenerator) {
+  x.foo()
+}