Skip to content

Commit 4840ef8

Browse files
committed
[move-only] Verify drop_deinit
drop_deinit only exists in ownership SIL. Remove IRGen support. A drop_deinit can only ever be destroyed or destructured. A destructure of a struct-with-deinit requires a drop_deinit operand.
1 parent 8f8fa83 commit 4840ef8

File tree

4 files changed

+69
-5
lines changed

4 files changed

+69
-5
lines changed

include/swift/SIL/InstructionUtils.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ SILValue stripCastsWithoutMarkDependence(SILValue V);
4040
/// begin_borrow instructions.
4141
SILValue lookThroughOwnershipInsts(SILValue v);
4242

43+
/// Reverse of lookThroughOwnershipInsts.
44+
///
45+
/// Return true if \p visitor returned true for all uses.
46+
bool visitNonOwnershipUses(SILValue value,
47+
function_ref<bool(Operand *)> visitor);
48+
4349
/// Return the underlying SILValue after looking through all copy_value
4450
/// instructions.
4551
SILValue lookThroughCopyValueInsts(SILValue v);

lib/IRGen/IRGenSIL.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1230,8 +1230,7 @@ class IRGenSILFunction :
12301230
setLoweredExplosion(i, e);
12311231
}
12321232
void visitDropDeinitInst(DropDeinitInst *i) {
1233-
auto e = getLoweredExplosion(i->getOperand());
1234-
setLoweredExplosion(i, e);
1233+
llvm_unreachable("only valid in ownership SIL");
12351234
}
12361235
void visitMarkMustCheckInst(MarkMustCheckInst *i) {
12371236
llvm_unreachable("Invalid in Lowered SIL");

lib/SIL/Utils/InstructionUtils.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,30 @@ SILValue swift::lookThroughOwnershipInsts(SILValue v) {
3939
}
4040
}
4141

42+
bool swift::visitNonOwnershipUses(SILValue value,
43+
function_ref<bool(Operand *)> visitor) {
44+
// All ownership insts have a single operand, so a recursive walk is
45+
// sufficient and cannot revisit operands.
46+
for (Operand *use : value->getUses()) {
47+
auto *user = use->getUser();
48+
switch (user->getKind()) {
49+
default:
50+
if (!visitor(use))
51+
return false;
52+
53+
break;
54+
case SILInstructionKind::MoveValueInst:
55+
case SILInstructionKind::CopyValueInst:
56+
case SILInstructionKind::BeginBorrowInst:
57+
if (!visitNonOwnershipUses(cast<SingleValueInstruction>(user), visitor))
58+
return false;
59+
60+
break;
61+
}
62+
}
63+
return true;
64+
}
65+
4266
SILValue swift::lookThroughCopyValueInsts(SILValue val) {
4367
while (auto *cvi =
4468
dyn_cast_or_null<CopyValueInst>(val->getDefiningInstruction())) {

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3556,6 +3556,13 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
35563556
"destructure with none ownership kind operand and non-none "
35573557
"ownership kind result?!");
35583558
}
3559+
if (operandTy.getNominalOrBoundGenericNominal()
3560+
->getValueTypeDestructor()) {
3561+
require(
3562+
isa<DropDeinitInst>(lookThroughOwnershipInsts(DSI->getOperand())),
3563+
"a destructure of a move-only-type-with-deinit requires a "
3564+
"drop_deinit");
3565+
}
35593566
}
35603567
}
35613568

@@ -5978,12 +5985,40 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
59785985
"Result and operand must have the same type, today.");
59795986
}
59805987

5988+
// check that a drop_deinit can only ever be destroyed or destructured
5989+
void checkDropDeinitUses(DropDeinitInst *ddi) {
5990+
// Address-type drop_deinit has no special structural requirements. It just
5991+
// sits there and blocks optimization on the allocation and downstream uses
5992+
// of the address. If we want to optimize around address-type drop_deinit,
5993+
// then we need a seperate verifier for its requirements.
5994+
if (ddi->getType().isAddress())
5995+
return;
5996+
5997+
visitNonOwnershipUses(ddi, [&](Operand *use) {
5998+
auto *user = use->getUser();
5999+
require(isa<DestroyValueInst>(user)
6000+
|| isa<EndLifetimeInst>(user)
6001+
|| isa<DestructureStructInst>(user)
6002+
|| isa<SwitchEnumInst>(user),
6003+
"A drop_deinit can only be destroyed or destructured");
6004+
return true;
6005+
});
6006+
}
6007+
59816008
void checkDropDeinitInst(DropDeinitInst *ddi) {
5982-
require(ddi->getType() == ddi->getOperand()->getType(),
6009+
require(F.hasOwnership(), "drop_deinit only allowed in OSSA");
6010+
6011+
auto type = ddi->getType();
6012+
require(type == ddi->getOperand()->getType(),
59836013
"Result and operand must have the same type.");
5984-
require(ddi->getType().isMoveOnlyNominalType(),
6014+
require(type.isMoveOnlyNominalType(),
59856015
"drop_deinit only allowed for move-only types");
5986-
require(F.hasOwnership(), "drop_deinit only allowed in OSSA");
6016+
require(type.getNominalOrBoundGenericNominal()
6017+
->getValueTypeDestructor(), "drop_deinit only allowed for "
6018+
"struct/enum types that define a deinit");
6019+
assert(type.isTrivial(F) && "a type with a deinit is nontrivial");
6020+
6021+
checkDropDeinitUses(ddi);
59876022
}
59886023

59896024
void checkMarkMustCheckInst(MarkMustCheckInst *i) {

0 commit comments

Comments
 (0)