Skip to content

Commit 4e2b158

Browse files
gottesmmgottesmm
authored
Merge pull request swiftlang#39582 from gottesmm/pr-4e3e9aadde0fd87f5f21dca3acd548f55c7bea18
[silgen] When initializing tuples in SILGen, first evaluate all tuple elts and then perform sub-initialization.
2 parents c2380f7 + f641808 commit 4e2b158

File tree

5 files changed

+65
-40
lines changed

5 files changed

+65
-40
lines changed

‎lib/SILGen/SILGenBuilder.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -869,6 +869,16 @@ void SILGenBuilder::emitDestructureValueOperation(
869869
}
870870
}
871871

872+
void SILGenBuilder::emitDestructureValueOperation(
873+
SILLocation loc, ManagedValue value,
874+
SmallVectorImpl<ManagedValue> &destructuredValues) {
875+
CleanupCloner cloner(*this, value);
876+
emitDestructureValueOperation(
877+
loc, value.forward(SGF), [&](unsigned index, SILValue subValue) {
878+
destructuredValues.push_back(cloner.clone(subValue));
879+
});
880+
}
881+
872882
ManagedValue SILGenBuilder::createProjectBox(SILLocation loc, ManagedValue mv,
873883
unsigned index) {
874884
auto *pbi = createProjectBox(loc, mv.getValue(), index);

‎lib/SILGen/SILGenBuilder.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,9 @@ class SILGenBuilder : public SILBuilder {
389389
void emitDestructureValueOperation(
390390
SILLocation loc, ManagedValue value,
391391
function_ref<void(unsigned, ManagedValue)> func);
392+
void emitDestructureValueOperation(
393+
SILLocation loc, ManagedValue value,
394+
SmallVectorImpl<ManagedValue> &destructuredValues);
392395

393396
using SILBuilder::createProjectBox;
394397
ManagedValue createProjectBox(SILLocation loc, ManagedValue mv,

‎lib/SILGen/SILGenDecl.cpp

Lines changed: 30 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -39,50 +39,42 @@ using namespace Lowering;
3939
void Initialization::_anchor() {}
4040
void SILDebuggerClient::anchor() {}
4141

42-
static void copyOrInitValueIntoHelper(
43-
SILGenFunction &SGF, SILLocation loc, ManagedValue value, bool isInit,
44-
ArrayRef<InitializationPtr> subInitializations,
45-
llvm::function_ref<ManagedValue(ManagedValue, unsigned, SILType)> func) {
46-
auto sourceType = value.getType().castTo<TupleType>();
47-
auto sourceSILType = value.getType();
48-
for (unsigned i = 0, e = sourceType->getNumElements(); i != e; ++i) {
49-
SILType fieldTy = sourceSILType.getTupleElementType(i);
50-
ManagedValue elt = func(value, i, fieldTy);
51-
subInitializations[i]->copyOrInitValueInto(SGF, loc, elt, isInit);
52-
subInitializations[i]->finishInitialization(SGF);
53-
}
54-
}
55-
5642
void TupleInitialization::copyOrInitValueInto(SILGenFunction &SGF,
5743
SILLocation loc,
5844
ManagedValue value, bool isInit) {
45+
// Process all values before initialization all at once to ensure all cleanups
46+
// are setup on all tuple elements before a potential early exit.
47+
SmallVector<ManagedValue, 8> destructuredValues;
48+
5949
// In the object case, emit a destructure operation and return.
6050
if (value.getType().isObject()) {
61-
return SGF.B.emitDestructureValueOperation(
62-
loc, value, [&](unsigned i, ManagedValue subValue) {
63-
auto &subInit = SubInitializations[i];
64-
subInit->copyOrInitValueInto(SGF, loc, subValue, isInit);
65-
subInit->finishInitialization(SGF);
66-
});
67-
}
68-
69-
// In the address case, we forward the underlying value and store it
70-
// into memory and then create a +1 cleanup. since we assume here
71-
// that we have a +1 value since we are forwarding into memory.
72-
assert(value.isPlusOne(SGF) && "Can not store a +0 value into memory?!");
73-
value = ManagedValue::forUnmanaged(value.forward(SGF));
74-
return copyOrInitValueIntoHelper(
75-
SGF, loc, value, isInit, SubInitializations,
76-
[&](ManagedValue aggregate, unsigned i,
77-
SILType fieldType) -> ManagedValue {
78-
ManagedValue elt =
79-
SGF.B.createTupleElementAddr(loc, value, i, fieldType);
80-
if (!fieldType.isAddressOnly(SGF.F)) {
81-
return SGF.B.createLoadTake(loc, elt);
82-
}
51+
SGF.B.emitDestructureValueOperation(loc, value, destructuredValues);
52+
} else {
53+
// In the address case, we forward the underlying value and store it
54+
// into memory and then create a +1 cleanup. since we assume here
55+
// that we have a +1 value since we are forwarding into memory.
56+
assert(value.isPlusOne(SGF) && "Can not store a +0 value into memory?!");
57+
CleanupCloner cloner(SGF, value);
58+
SILValue v = value.forward(SGF);
59+
60+
auto sourceType = value.getType().castTo<TupleType>();
61+
auto sourceSILType = value.getType();
62+
for (unsigned i : range(sourceType->getNumElements())) {
63+
SILType fieldTy = sourceSILType.getTupleElementType(i);
64+
SILValue elt = SGF.B.createTupleElementAddr(loc, v, i, fieldTy);
65+
if (!fieldTy.isAddressOnly(SGF.F)) {
66+
elt = SGF.B.emitLoadValueOperation(loc, elt,
67+
LoadOwnershipQualifier::Take);
68+
}
69+
destructuredValues.push_back(cloner.clone(elt));
70+
}
71+
}
8372

84-
return SGF.emitManagedRValueWithCleanup(elt.getValue());
85-
});
73+
for (unsigned i : indices(destructuredValues)) {
74+
SubInitializations[i]->copyOrInitValueInto(SGF, loc, destructuredValues[i],
75+
isInit);
76+
SubInitializations[i]->finishInitialization(SGF);
77+
}
8678
}
8779

8880
void TupleInitialization::finishUninitialized(SILGenFunction &SGF) {

‎test/SILGen/enum.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,3 +219,23 @@ func sr7799_1(bar: SR7799??) {
219219
default: print("default")
220220
}
221221
}
222+
223+
// Make sure that we handle enum, tuple initialization composed
224+
// correctly. Previously, we leaked down a failure path due to us misusing
225+
// scopes.
226+
enum rdar81817725 {
227+
case localAddress
228+
case setOption(Int, Any)
229+
230+
static func takeAny(_:Any) -> Bool { return true }
231+
232+
static func testSwitchCleanup(syscall: rdar81817725, expectedLevel: Int,
233+
valueMatcher: (Any) -> Bool)
234+
throws -> Bool {
235+
if case .setOption(expectedLevel, let value) = syscall {
236+
return rdar81817725.takeAny(value)
237+
} else {
238+
return false
239+
}
240+
}
241+
}

‎test/SILGen/indirect_enum.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,8 +448,8 @@ func guardTreeB<T>(_ tree: TreeB<T>) {
448448
// CHECK: [[TUPLE_ADDR:%.*]] = project_box [[BOX]]
449449
// CHECK: copy_addr [[TUPLE_ADDR]] to [initialization] [[TUPLE_COPY:%.*]] :
450450
// CHECK: [[L_COPY:%.*]] = tuple_element_addr [[TUPLE_COPY]]
451-
// CHECK: copy_addr [take] [[L_COPY]] to [initialization] [[L]]
452451
// CHECK: [[R_COPY:%.*]] = tuple_element_addr [[TUPLE_COPY]]
452+
// CHECK: copy_addr [take] [[L_COPY]] to [initialization] [[L]]
453453
// CHECK: copy_addr [take] [[R_COPY]] to [initialization] [[R]]
454454
// CHECK: destroy_value [[BOX]]
455455
guard case .Branch(left: let l, right: let r) = tree else { return }
@@ -492,8 +492,8 @@ func guardTreeB<T>(_ tree: TreeB<T>) {
492492
// CHECK: [[TUPLE_ADDR:%.*]] = project_box [[BOX]]
493493
// CHECK: copy_addr [[TUPLE_ADDR]] to [initialization] [[TUPLE_COPY:%.*]] :
494494
// CHECK: [[L_COPY:%.*]] = tuple_element_addr [[TUPLE_COPY]]
495-
// CHECK: copy_addr [take] [[L_COPY]] to [initialization] [[L]]
496495
// CHECK: [[R_COPY:%.*]] = tuple_element_addr [[TUPLE_COPY]]
496+
// CHECK: copy_addr [take] [[L_COPY]] to [initialization] [[L]]
497497
// CHECK: copy_addr [take] [[R_COPY]] to [initialization] [[R]]
498498
// CHECK: destroy_value [[BOX]]
499499
// CHECK: destroy_addr [[R]]

0 commit comments

Comments
 (0)