Skip to content

Commit e8b0398

Browse files
committed
Use the BorrowingSwitch implementation for all noncopyable switches.
It works well enough now that it should be an acceptable replacement for both borrowing and consuming switches that works in more correct situations than the previous implementation. This does however expose a few known issues that I'll try to fix in follow ups: - overconsumes cause verifier errors instead of raising diagnostics (rdar://125381446) - cases with multiple pattern labels aren't yet supported (rdar://125188955) - copyable types with the `borrowing` or `consuming` modifiers should probably use noncopyable pattern matching. The `BorrowingSwitch` flag is still necessary to enable the surface-level syntax changes (switches without `consume` and the `_borrowing` modifier, for instance).
1 parent ffdb5db commit e8b0398

9 files changed

+143
-876
lines changed

lib/SILGen/SILGenPattern.cpp

Lines changed: 101 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -3352,21 +3352,19 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
33523352
// the match, we'll have to perform the initial match as a borrow and then
33533353
// reproject the value to consume it.
33543354
auto ownership = ValueOwnership::Default;
3355-
if (getASTContext().LangOpts.hasFeature(Feature::BorrowingSwitch)) {
3356-
if (subjectTy->isNoncopyable()) {
3357-
// Determine the overall ownership behavior of the switch, based on the
3358-
// subject expression and the patterns' ownership behavior.
3359-
3360-
// If the subject expression is borrowable, then perform the switch as
3361-
// a borrow. (A `consume` expression would render the expression
3362-
// non-borrowable.) Otherwise, perform it as a consume.
3363-
ownership = isBorrowableSubject(*this, subjectExpr)
3364-
? ValueOwnership::Shared
3365-
: ValueOwnership::Owned;
3366-
for (auto caseLabel : S->getCases()) {
3367-
for (auto item : caseLabel->getCaseLabelItems()) {
3368-
ownership = std::max(ownership, item.getPattern()->getOwnership());
3369-
}
3355+
if (subjectTy->isNoncopyable()) {
3356+
// Determine the overall ownership behavior of the switch, based on the
3357+
// subject expression and the patterns' ownership behavior.
3358+
3359+
// If the subject expression is borrowable, then perform the switch as
3360+
// a borrow. (A `consume` expression would render the expression
3361+
// non-borrowable.) Otherwise, perform it as a consume.
3362+
ownership = isBorrowableSubject(*this, subjectExpr)
3363+
? ValueOwnership::Shared
3364+
: ValueOwnership::Owned;
3365+
for (auto caseLabel : S->getCases()) {
3366+
for (auto item : caseLabel->getCaseLabelItems()) {
3367+
ownership = std::max(ownership, item.getPattern()->getOwnership());
33703368
}
33713369
}
33723370
}
@@ -3505,103 +3503,101 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
35053503

35063504
// Inline constructor for subject.
35073505
auto subject = ([&]() -> ConsumableManagedValue {
3508-
if (subjectMV.getType().isMoveOnly()) {
3509-
if (ownership > ValueOwnership::Default) { // should become an assert when BorrowingSwitch is enabled
3510-
// Based on the ownership behavior, prepare the subject.
3511-
// The pattern match itself will always be performed on a borrow, to
3512-
// ensure that the order of pattern evaluation doesn't prematurely
3513-
// consume or modify the value until we commit to a match. But if the
3514-
// match consumes the value, then we need a +1 value to go back to in
3515-
// order to consume the parts we match to, so we force a +1 value then
3516-
// borrow that for the pattern match.
3517-
switch (ownership) {
3518-
case ValueOwnership::Default:
3519-
llvm_unreachable("invalid");
3506+
// TODO: Move-only-wrapped subjects should also undergo a noncopying switch.
3507+
if (subjectMV.getType().isMoveOnly(/*or wrapped*/ false)) {
3508+
assert(ownership > ValueOwnership::Default);
3509+
// Based on the ownership behavior, prepare the subject.
3510+
// The pattern match itself will always be performed on a borrow, to
3511+
// ensure that the order of pattern evaluation doesn't prematurely
3512+
// consume or modify the value until we commit to a match. But if the
3513+
// match consumes the value, then we need a +1 value to go back to in
3514+
// order to consume the parts we match to, so we force a +1 value then
3515+
// borrow that for the pattern match.
3516+
switch (ownership) {
3517+
case ValueOwnership::Default:
3518+
llvm_unreachable("invalid");
3519+
3520+
case ValueOwnership::Shared:
3521+
emission.setNoncopyableBorrowingOwnership();
3522+
if (subjectMV.getType().isAddress()) {
3523+
// Initiate a read access on the memory, to ensure that even
3524+
// if the underlying memory is mutable or consumable, the pattern
3525+
// match is not allowed to modify it.
3526+
subjectMV = B.createOpaqueBorrowBeginAccess(S, subjectMV);
3527+
if (subjectMV.getType().isLoadable(F)) {
3528+
// Load a borrow if the type is loadable.
3529+
subjectMV = subjectUndergoesFormalAccess
3530+
? B.createFormalAccessLoadBorrow(S, subjectMV)
3531+
: B.createLoadBorrow(S, subjectMV);
3532+
}
3533+
} else {
3534+
// Initiate a fixed borrow on the subject, so that it's treated as
3535+
// opaque by the move checker.
3536+
subjectMV =
3537+
subjectUndergoesFormalAccess
3538+
? B.createFormalAccessBeginBorrow(
3539+
S, subjectMV, IsNotLexical, BeginBorrowInst::IsFixed)
3540+
: B.createBeginBorrow(S, subjectMV, IsNotLexical,
3541+
BeginBorrowInst::IsFixed);
3542+
}
3543+
return {subjectMV, CastConsumptionKind::BorrowAlways};
35203544

3521-
case ValueOwnership::Shared:
3522-
emission.setNoncopyableBorrowingOwnership();
3523-
if (subjectMV.getType().isAddress()) {
3524-
// Initiate a read access on the memory, to ensure that even
3525-
// if the underlying memory is mutable or consumable, the pattern
3526-
// match is not allowed to modify it.
3527-
subjectMV = B.createOpaqueBorrowBeginAccess(S, subjectMV);
3528-
if (subjectMV.getType().isLoadable(F)) {
3529-
// Load a borrow if the type is loadable.
3530-
subjectMV = subjectUndergoesFormalAccess
3531-
? B.createFormalAccessLoadBorrow(S, subjectMV)
3532-
: B.createLoadBorrow(S, subjectMV);
3533-
}
3534-
} else {
3535-
// Initiate a fixed borrow on the subject, so that it's treated as
3536-
// opaque by the move checker.
3537-
subjectMV =
3538-
subjectUndergoesFormalAccess
3539-
? B.createFormalAccessBeginBorrow(
3540-
S, subjectMV, IsNotLexical, BeginBorrowInst::IsFixed)
3541-
: B.createBeginBorrow(S, subjectMV, IsNotLexical,
3545+
case ValueOwnership::InOut:
3546+
// TODO: mutating switches
3547+
llvm_unreachable("not implemented");
3548+
3549+
case ValueOwnership::Owned:
3550+
// Make sure we own the subject value.
3551+
subjectMV = subjectMV.ensurePlusOne(*this, S);
3552+
if (subjectMV.getType().isAddress() &&
3553+
subjectMV.getType().isLoadable(F)) {
3554+
// Move the value into memory if it's loadable.
3555+
subjectMV = B.createLoadTake(S, subjectMV);
3556+
}
3557+
3558+
// Unwind cleanups to this point when we're ready to reproject
3559+
// the value for consumption or mutation.
3560+
emission.setNoncopyableConsumingOwnership(
3561+
Cleanups.getCleanupsDepth(),
3562+
subjectMV);
3563+
3564+
// Perform the pattern match on an opaque borrow or read access of the
3565+
// subject.
3566+
if (subjectMV.getType().isAddress()) {
3567+
subjectMV = B.createOpaqueBorrowBeginAccess(S, subjectMV);
3568+
} else {
3569+
subjectMV = B.createBeginBorrow(S, subjectMV, IsNotLexical,
35423570
BeginBorrowInst::IsFixed);
3543-
}
3544-
return {subjectMV, CastConsumptionKind::BorrowAlways};
3545-
3546-
case ValueOwnership::InOut:
3547-
// TODO: mutating switches
3548-
llvm_unreachable("not implemented");
3549-
3550-
case ValueOwnership::Owned:
3551-
// Make sure we own the subject value.
3552-
subjectMV = subjectMV.ensurePlusOne(*this, S);
3553-
if (subjectMV.getType().isAddress() &&
3554-
subjectMV.getType().isLoadable(F)) {
3555-
// Move the value into memory if it's loadable.
3556-
subjectMV = B.createLoadTake(S, subjectMV);
3557-
}
3558-
3559-
// Unwind cleanups to this point when we're ready to reproject
3560-
// the value for consumption or mutation.
3561-
emission.setNoncopyableConsumingOwnership(
3562-
Cleanups.getCleanupsDepth(),
3563-
subjectMV);
3564-
3565-
// Perform the pattern match on an opaque borrow or read access of the
3566-
// subject.
3567-
if (subjectMV.getType().isAddress()) {
3568-
subjectMV = B.createOpaqueBorrowBeginAccess(S, subjectMV);
3569-
} else {
3570-
subjectMV = B.createBeginBorrow(S, subjectMV, IsNotLexical,
3571-
BeginBorrowInst::IsFixed);
3572-
}
3573-
return {subjectMV, CastConsumptionKind::BorrowAlways};
35743571
}
3575-
llvm_unreachable("unhandled value ownership");
3572+
return {subjectMV, CastConsumptionKind::BorrowAlways};
3573+
}
3574+
llvm_unreachable("unhandled value ownership");
3575+
}
3576+
3577+
// TODO: Move-only-wrapped subjects should also undergo a noncopying switch.
3578+
// For now, unwrap them and perform a normal switch over them.
3579+
if (subjectMV.getType().isMoveOnlyWrapped()) {
3580+
if (subjectMV.getType().isAddress()) {
3581+
auto isPlusOne = subjectMV.isPlusOne(*this);
3582+
auto subjectAddr
3583+
= B.createMoveOnlyWrapperToCopyableAddr(S, subjectMV.forward(*this));
3584+
3585+
if (isPlusOne) {
3586+
subjectMV = emitManagedRValueWithCleanup(subjectAddr);
3587+
} else {
3588+
subjectMV = ManagedValue::forBorrowedAddressRValue(subjectAddr);
3589+
}
35763590
} else {
3577-
// TODO: Remove.
3578-
// If we have a noImplicitCopy value, ensure plus one and convert
3579-
// it. Switches always consume move only values.
3580-
//
3581-
// NOTE: We purposely do not do this for pure move only types since for them
3582-
// we emit everything at +0 and then run the BorrowToDestructure transform
3583-
// upon them. The reason that we do this is that internally to
3584-
// SILGenPattern, we always attempt to move from +1 -> +0 meaning that even
3585-
// if we start at +1, we will go back to +0 given enough patterns to go
3586-
// through. It is simpler to just let SILGenPattern do what it already wants
3587-
// to do, rather than fight it or try to resusitate the "fake owned borrow"
3588-
// path that we still use for address only types (and that we want to delete
3589-
// once we have opaque values).
3590-
if (subjectMV.getType().isObject()) {
3591-
if (subjectMV.getType().isMoveOnlyWrapped()) {
3592-
subjectMV = B.createOwnedMoveOnlyWrapperToCopyableValue(
3593-
S, subjectMV.ensurePlusOne(*this, S));
3594-
} else {
3595-
// If we have a pure move only type and it is owned, borrow it so that
3596-
// BorrowToDestructure can handle it.
3597-
if (subjectMV.getOwnershipKind() == OwnershipKind::Owned) {
3598-
subjectMV = subjectMV.borrow(*this, S);
3599-
}
3600-
}
3591+
if (subjectMV.isPlusOne(*this)) {
3592+
subjectMV
3593+
= B.createOwnedMoveOnlyWrapperToCopyableValue(S, subjectMV);
3594+
} else {
3595+
subjectMV
3596+
= B.createGuaranteedMoveOnlyWrapperToCopyableValue(S, subjectMV);
36013597
}
36023598
}
36033599
}
3604-
3600+
36053601
// If we have a plus one value...
36063602
if (subjectMV.isPlusOne(*this)) {
36073603
// And we have an address that is loadable, perform a load [take].

0 commit comments

Comments
 (0)