Merge pull request #14450 from jckarter/open-existential-lvalue-evaluation-order
SILGen: Open existential lvalues on entry into an OpenExistentialExpr again.
diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp
index 5d58526..6da1aa7 100644
--- a/lib/SILGen/SILGenExpr.cpp
+++ b/lib/SILGen/SILGenExpr.cpp
@@ -5088,15 +5088,34 @@
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;
}
@@ -5123,6 +5142,12 @@
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);
diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h
index 034aaf4..0c4416f 100644
--- a/lib/SILGen/SILGenFunction.h
+++ b/lib/SILGen/SILGenFunction.h
@@ -1412,10 +1412,10 @@
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.
///
diff --git a/lib/SILGen/SILGenLValue.cpp b/lib/SILGen/SILGenLValue.cpp
index c6d162f..8b25f42 100644
--- a/lib/SILGen/SILGenLValue.cpp
+++ b/lib/SILGen/SILGenLValue.cpp
@@ -2208,20 +2208,18 @@
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);
+ 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);
- // 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;
+ // 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::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
diff --git a/test/SILGen/assignment.swift b/test/SILGen/assignment.swift
index 1e3058c..fbf2ff8 100644
--- a/test/SILGen/assignment.swift
+++ b/test/SILGen/assignment.swift
@@ -43,11 +43,11 @@
// 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
}
diff --git a/test/SILGen/class_bound_protocols.swift b/test/SILGen/class_bound_protocols.swift
index b0dc656..bf9df67 100644
--- a/test/SILGen/class_bound_protocols.swift
+++ b/test/SILGen/class_bound_protocols.swift
@@ -192,7 +192,7 @@
// 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
@@ -200,14 +200,15 @@
// ** 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
diff --git a/test/SILGen/properties.swift b/test/SILGen/properties.swift
index 03571ef..d108a13 100644
--- a/test/SILGen/properties.swift
+++ b/test/SILGen/properties.swift
@@ -1190,12 +1190,12 @@
// 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
diff --git a/test/SILGen/protocols.swift b/test/SILGen/protocols.swift
index 17e98d2..11072b7 100644
--- a/test/SILGen/protocols.swift
+++ b/test/SILGen/protocols.swift
@@ -26,10 +26,10 @@
// 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 @@
// 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 @@
// 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()
+}
