Skip to content

Commit 29ee02d

Browse files
committed
Fix CastEmitter ownership.
Rewrite ownership forwarding through switch_enums. It's unclear what the original code was doing, but enabling OSSA verification for terminators revealed the issue.
1 parent 8f53a92 commit 29ee02d

File tree

1 file changed

+27
-25
lines changed

1 file changed

+27
-25
lines changed

lib/SIL/Utils/DynamicCasts.cpp

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -848,18 +848,10 @@ namespace {
848848

849849
SILValue getOwnedScalar(Source source, const TypeLowering &srcTL) {
850850
assert(!source.isAddress());
851-
return source.Value;
852-
}
853-
854-
Source putOwnedScalar(SILValue scalar, Target target) {
855-
assert(scalar->getType() == target.LoweredType.getObjectType());
856-
if (!target.isAddress())
857-
return target.asScalarSource(scalar);
858-
859-
auto &targetTL = getTypeLowering(target.LoweredType);
860-
targetTL.emitStoreOfCopy(B, Loc, scalar, target.Address,
861-
IsInitialization);
862-
return target.asAddressSource();
851+
auto value = source.Value;
852+
if (value.getOwnershipKind() == OwnershipKind::Guaranteed)
853+
value = B.emitCopyValueOperation(Loc, value);
854+
return value;
863855
}
864856

865857
Source emitSameType(Source source, Target target) {
@@ -917,7 +909,8 @@ namespace {
917909
if (source.isAddress()) {
918910
value = srcTL.emitLoadOfCopy(B, Loc, source.Value, IsTake);
919911
} else {
920-
value = getOwnedScalar(source, srcTL);
912+
// May have any valid ownership.
913+
value = source.Value;
921914
}
922915
auto targetFormalTy = target.FormalType;
923916
auto targetLoweredTy =
@@ -929,7 +922,15 @@ namespace {
929922
} else {
930923
value = B.createUpcast(Loc, value, targetLoweredTy);
931924
}
932-
return putOwnedScalar(value, target);
925+
// If the target is an address, then scalar must be Owned. Otherwise, it
926+
// may be Guaranteed.
927+
assert(value->getType() == target.LoweredType.getObjectType());
928+
if (!target.isAddress())
929+
return target.asScalarSource(value);
930+
931+
auto &targetTL = getTypeLowering(target.LoweredType);
932+
targetTL.emitStoreOfCopy(B, Loc, value, target.Address, IsInitialization);
933+
return target.asAddressSource();
933934
}
934935

935936
Source emitAndInjectIntoOptionals(Source source, Target target,
@@ -961,7 +962,9 @@ namespace {
961962
if (source.isAddress()) {
962963
B.createSwitchEnumAddr(Loc, source.Value, /*default*/ nullptr, cases);
963964
} else {
964-
B.createSwitchEnum(Loc, source.Value, /*default*/ nullptr, cases);
965+
auto *switchEnum =
966+
B.createSwitchEnum(Loc, source.Value, /*default*/ nullptr, cases);
967+
switchEnum->createOptionalSomeResult();
965968
}
966969

967970
// Create the Some block, which recurses.
@@ -988,10 +991,7 @@ namespace {
988991
sourceSomeDecl, loweredSourceObjectType);
989992
objectSource = Source(sourceAddr, sourceObjectType);
990993
} else {
991-
// switch enum always start as @owned.
992-
SILValue sourceObjectValue = someBB->createPhiArgument(
993-
loweredSourceObjectType, OwnershipKind::Owned);
994-
objectSource = Source(sourceObjectValue, sourceObjectType);
994+
objectSource = Source(someBB->getArgument(0), sourceObjectType);
995995
}
996996

997997
Source resultObject = emit(objectSource, objectTarget);
@@ -1006,7 +1006,9 @@ namespace {
10061006
if (target.isAddress()) {
10071007
B.createBranch(Loc, contBB);
10081008
} else {
1009-
B.createBranch(Loc, contBB, { result.Value });
1009+
auto &resultTL = getTypeLowering(result.Value->getType());
1010+
SILValue resultVal = getOwnedScalar(source, resultTL);
1011+
B.createBranch(Loc, contBB, {resultVal});
10101012
}
10111013
}
10121014

@@ -1057,18 +1059,18 @@ namespace {
10571059
}
10581060
}
10591061

1062+
// May return an Owned or Guaranteed result. If source is has ownership
1063+
// None, then the result may still be Guaranteed for nontrivial types.
10601064
Source emitSome(Source source, Target target, EmitSomeState &state) {
10611065
// If our target is an address, prepareForEmitSome should have set this
10621066
// up so that we emitted directly into
10631067
if (target.isAddress()) {
10641068
B.createInjectEnumAddr(Loc, target.Address, state.SomeDecl);
10651069
return target.asAddressSource();
10661070
} else {
1067-
auto &srcTL = getTypeLowering(source.Value->getType());
1068-
auto sourceObject = getOwnedScalar(source, srcTL);
1069-
auto source = B.createEnum(Loc, sourceObject, state.SomeDecl,
1070-
target.LoweredType);
1071-
return target.asScalarSource(source);
1071+
auto someEnum =
1072+
B.createEnum(Loc, source.Value, state.SomeDecl, target.LoweredType);
1073+
return target.asScalarSource(someEnum);
10721074
}
10731075
}
10741076

0 commit comments

Comments
 (0)