Skip to content

Commit 3888b5b

Browse files
committed
[+0-normal-args] Use SILGenBuilder in SILGenConvert when injecting an optional so we propagate through +0 vs +1.
This commit does two different things: 1. Changes some code paths in SILGenConvert to use SILGenBuilder APIs. 2. It eliminates a few bugs in this path exposed by running the SILGen tests with +0 enabled. The bug specifically is that we are assuming in this code that the input value is always at +1, so we unconditionally forward the input value and then create destroys on the output. This is not correct in general (since we have guaranteed values) and occurs all the time with +0 arguments enabled. This is tested by running the SILGen tests with +0 arguments enabled. rdar://34222540
1 parent 8ed891a commit 3888b5b

File tree

3 files changed

+21
-17
lines changed

3 files changed

+21
-17
lines changed

lib/SILGen/SILGenBuilder.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -691,6 +691,14 @@ ManagedValue SILGenBuilder::createOpenExistentialBoxValue(SILLocation loc,
691691
return ManagedValue::forUnmanaged(openedExistential);
692692
}
693693

694+
ManagedValue SILGenBuilder::createOpenExistentialMetatype(SILLocation loc,
695+
ManagedValue value,
696+
SILType openedType) {
697+
SILValue result = SILGenBuilder::createOpenExistentialMetatype(
698+
loc, value.getValue(), openedType);
699+
return ManagedValue::forTrivialRValue(result);
700+
}
701+
694702
ManagedValue SILGenBuilder::createStore(SILLocation loc, ManagedValue value,
695703
SILValue address,
696704
StoreOwnershipQualifier qualifier) {

lib/SILGen/SILGenBuilder.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,11 @@ class SILGenBuilder : public SILBuilder {
313313
ManagedValue createOpenExistentialBoxValue(SILLocation loc,
314314
ManagedValue original, SILType type);
315315

316+
using SILBuilder::createOpenExistentialMetatype;
317+
ManagedValue createOpenExistentialMetatype(SILLocation loc,
318+
ManagedValue value,
319+
SILType openedType);
320+
316321
/// Convert a @convention(block) value to AnyObject.
317322
ManagedValue createBlockToAnyObject(SILLocation loc, ManagedValue block,
318323
SILType type);

lib/SILGen/SILGenConvert.cpp

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ SILGenFunction::emitInjectOptional(SILLocation loc,
5050
// TODO: honor +0 contexts?
5151
if (optTL.isLoadable() || !silConv.useLoweredAddresses()) {
5252
ManagedValue objectResult = generator(SGFContext());
53-
auto some = B.createEnum(loc, objectResult.forward(*this), someDecl, optTy);
54-
return emitManagedRValueWithCleanup(some, optTL);
53+
return B.createEnum(loc, objectResult, someDecl, optTy);
5554
}
5655

5756
// Otherwise it's address-only; try to avoid spurious copies by
@@ -131,10 +130,8 @@ getOptionalSomeValue(SILLocation loc, ManagedValue value,
131130

132131
assert(formalOptType.getOptionalObjectType());
133132
auto someDecl = getASTContext().getOptionalSomeDecl();
134-
135-
SILValue result =
136-
B.createEnum(loc, value.forward(*this), someDecl, optTL.getLoweredType());
137-
return emitManagedRValueWithCleanup(result, optTL);
133+
134+
return B.createEnum(loc, value, someDecl, optTL.getLoweredType());
138135
}
139136

140137
auto SILGenFunction::emitSourceLocationArgs(SourceLoc sourceLoc,
@@ -864,22 +861,16 @@ SILGenFunction::emitOpenExistential(
864861
}
865862
case ExistentialRepresentation::Metatype:
866863
assert(existentialType.isObject());
867-
archetypeMV =
868-
ManagedValue::forUnmanaged(
869-
B.createOpenExistentialMetatype(
870-
loc, existentialValue.forward(*this),
871-
loweredOpenedType));
864+
archetypeMV = B.createOpenExistentialMetatype(
865+
loc, existentialValue, loweredOpenedType);
872866
// Metatypes are always trivial. Consuming would be a no-op.
873867
canConsume = false;
874868
break;
875869
case ExistentialRepresentation::Class: {
876870
assert(existentialType.isObject());
877-
SILValue archetypeValue = B.createOpenExistentialRef(
878-
loc, existentialValue.forward(*this),
879-
loweredOpenedType);
880-
canConsume = existentialValue.hasCleanup();
881-
archetypeMV = (canConsume ? emitManagedRValueWithCleanup(archetypeValue)
882-
: ManagedValue::forUnmanaged(archetypeValue));
871+
archetypeMV =
872+
B.createOpenExistentialRef(loc, existentialValue, loweredOpenedType);
873+
canConsume = archetypeMV.hasCleanup();
883874
break;
884875
}
885876
case ExistentialRepresentation::Boxed:

0 commit comments

Comments
 (0)