Skip to content

Commit e46168c

Browse files
committed
SILGen: Improve handling of copyable subpatterns in borrowing switches.
A `let` binding of a copyable subpattern can create an independent variable which should be copyable and consumable without affecting a borrowed move-only base. In the same way that `borrowing` parameters are no-implicit-copy, though, explicitly `_borrowing` subpatterns of copyable type should be no-implicit-copy as well.
1 parent 2f519f4 commit e46168c

File tree

4 files changed

+110
-25
lines changed

4 files changed

+110
-25
lines changed

lib/AST/Pattern.cpp

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -775,13 +775,19 @@ Pattern::getOwnership(
775775
}
776776

777777
void visitNamedPattern(NamedPattern *p) {
778-
// `var` and `let` bindings consume the matched value.
779-
// TODO: borrowing/mutating/consuming parameters
780778
switch (p->getDecl()->getIntroducer()) {
781779
case VarDecl::Introducer::Let:
782780
case VarDecl::Introducer::Var:
783-
// `let` and `var` consume the bound value to move it into a new
784-
// independent variable.
781+
// If the subpattern type is copyable, then we can bind the variable
782+
// by copying without requiring more than a borrow of the original.
783+
if (!p->hasType() || !p->getType()->isNoncopyable()) {
784+
break;
785+
}
786+
// TODO: An explicit `consuming` binding kind consumes regardless of
787+
// type.
788+
789+
// Noncopyable `let` and `var` consume the bound value to move it into
790+
// a new independent variable.
785791
increaseOwnership(ValueOwnership::Owned, p);
786792
break;
787793

@@ -805,9 +811,9 @@ Pattern::getOwnership(
805811
}
806812

807813
void visitIsPattern(IsPattern *p) {
808-
// Casting currently always consumes.
809-
// TODO: Sometimes maybe it doesn't need to be.
810-
increaseOwnership(ValueOwnership::Owned, p);
814+
// Casting has to either be possible by borrowing or copying the subject,
815+
// or can't be supported in a pattern match.
816+
/* no change */
811817
}
812818

813819
void visitEnumElementPattern(EnumElementPattern *p) {
@@ -821,11 +827,9 @@ Pattern::getOwnership(
821827
}
822828

823829
void visitExprPattern(ExprPattern *p) {
824-
// We can't get the ownership reliably if the pattern hasn't been resolved.
825-
if (!p->isResolved()) {
826-
return;
827-
}
828-
increaseOwnership(p->getMatchOperandOwnership(), p);
830+
// A `~=` operator has to be able to either borrow or copy the operand,
831+
// or can't be used.
832+
/* no change */
829833
}
830834
};
831835

lib/SILGen/SILGenPattern.cpp

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,8 @@ class PatternMatchEmission {
531531
bool forIrrefutableRow, bool hasMultipleItems);
532532

533533
// Bind noncopyable variable bindings as borrows.
534-
void bindIrrefutableBorrows(const ClauseRow &row, ArgArray args);
534+
void bindIrrefutableBorrows(const ClauseRow &row, ArgArray args,
535+
bool forIrrefutableRow, bool hasMultipleItems);
535536

536537
// End the borrow of the subject and derived values during a move-only match.
537538
void unbindAndEndBorrows(const ClauseRow &row, ArgArray args);
@@ -1187,7 +1188,8 @@ void PatternMatchEmission::emitWildcardDispatch(ClauseMatrix &clauses,
11871188
// If the final pattern match is only borrowing as well,
11881189
// we can bind the variables immediately here too.
11891190
if (*ownership <= ValueOwnership::Shared) {
1190-
bindIrrefutableBorrows(clauses[row], args);
1191+
bindIrrefutableBorrows(clauses[row], args,
1192+
!hasGuard, hasMultipleItems);
11911193
}
11921194

11931195
if (hasGuard) {
@@ -1286,7 +1288,9 @@ void PatternMatchEmission::bindIrrefutablePatterns(const ClauseRow &row,
12861288
}
12871289

12881290
void PatternMatchEmission::bindIrrefutableBorrows(const ClauseRow &row,
1289-
ArgArray args) {
1291+
ArgArray args,
1292+
bool forIrrefutableRow,
1293+
bool hasMultipleItems) {
12901294
assert(row.columns() == args.size());
12911295
for (unsigned i = 0, e = args.size(); i != e; ++i) {
12921296
if (!row[i]) // We use null patterns to mean artificial AnyPatterns
@@ -1301,7 +1305,16 @@ void PatternMatchEmission::bindIrrefutableBorrows(const ClauseRow &row,
13011305
break;
13021306
case PatternKind::Named: {
13031307
NamedPattern *named = cast<NamedPattern>(pattern);
1304-
bindBorrow(pattern, named->getDecl(), args[i]);
1308+
// If the subpattern matches a copyable type, and the match isn't
1309+
// explicitly `borrowing`, then we can bind it as a normal copyable
1310+
// value.
1311+
if (named->getDecl()->getIntroducer() != VarDecl::Introducer::Borrowing
1312+
&& !named->getType()->isNoncopyable()) {
1313+
bindVariable(pattern, named->getDecl(), args[i], forIrrefutableRow,
1314+
hasMultipleItems);
1315+
} else {
1316+
bindBorrow(pattern, named->getDecl(), args[i]);
1317+
}
13051318
break;
13061319
}
13071320
default:
@@ -1391,14 +1404,25 @@ void PatternMatchEmission::bindBorrow(Pattern *pattern, VarDecl *var,
13911404
assert(value.getFinalConsumption() == CastConsumptionKind::BorrowAlways);
13921405

13931406
auto bindValue = value.asBorrowedOperand2(SGF, pattern).getFinalManagedValue();
1394-
if (bindValue.getType().isMoveOnly()) {
1395-
if (bindValue.getType().isObject()) {
1396-
// Create a notional copy for the borrow checker to use.
1397-
bindValue = bindValue.copy(SGF, pattern);
1407+
1408+
// Borrow bindings of copyable type should still be no-implicit-copy.
1409+
if (!bindValue.getType().isMoveOnly()) {
1410+
if (bindValue.getType().isAddress()) {
1411+
bindValue = ManagedValue::forBorrowedAddressRValue(
1412+
SGF.B.createCopyableToMoveOnlyWrapperAddr(pattern, bindValue.getValue()));
1413+
} else {
1414+
bindValue =
1415+
SGF.B.createGuaranteedCopyableToMoveOnlyWrapperValue(pattern, bindValue);
13981416
}
1399-
bindValue = SGF.B.createMarkUnresolvedNonCopyableValueInst(pattern, bindValue,
1400-
MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign);
14011417
}
1418+
1419+
if (bindValue.getType().isObject()) {
1420+
// Create a notional copy for the borrow checker to use.
1421+
bindValue = bindValue.copy(SGF, pattern);
1422+
}
1423+
bindValue = SGF.B.createMarkUnresolvedNonCopyableValueInst(pattern, bindValue,
1424+
MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign);
1425+
14021426
SGF.VarLocs[var] = SILGenFunction::VarLoc::get(bindValue.getValue());
14031427
}
14041428

@@ -1420,7 +1444,9 @@ void PatternMatchEmission::emitGuardBranch(SILLocation loc, Expr *guard,
14201444
// subject yet.
14211445
if (auto ownership = getNoncopyableOwnership()) {
14221446
if (*ownership > ValueOwnership::Shared) {
1423-
bindIrrefutableBorrows(row, args);
1447+
bindIrrefutableBorrows(row, args,
1448+
/*irrefutable*/ false,
1449+
/*multiple items*/ false);
14241450
}
14251451
}
14261452
testBool = SGF.emitRValueAsSingleValue(guard).getUnmanagedValue();
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// RUN: %target-swift-frontend -emit-sil -verify -enable-experimental-feature BorrowingSwitch -disable-experimental-parser-round-trip %s
2+
3+
struct Payload: ~Copyable {
4+
var x: Int
5+
var y: String
6+
}
7+
8+
enum Foo: ~Copyable {
9+
case nonCopyablePayload(Payload)
10+
case copyablePayload(String)
11+
}
12+
13+
func hungryCondition(_: consuming String) -> Bool { fatalError() }
14+
func condition(_: borrowing String) -> Bool { fatalError() }
15+
16+
func eat(_: consuming String) {}
17+
func nibble(_: borrowing String) {}
18+
19+
func test(borrowing foo: borrowing Foo) {
20+
switch foo {
21+
case .nonCopyablePayload(_borrowing x): // expected-warning{{}}
22+
break
23+
24+
// OK to form a normal `let` binding when the payload is copyable.
25+
// Then it's OK to consume copies of it in the condition clause
26+
// and in the body.
27+
case .copyablePayload(let x) where hungryCondition(x):
28+
eat(x)
29+
nibble(x)
30+
case .copyablePayload(let x) where condition(x):
31+
eat(x)
32+
nibble(x)
33+
34+
// `_borrowing` match variables impose the no-implicit-copy constraint
35+
// like `borrowing` parameters do.
36+
case .copyablePayload(_borrowing x) // expected-error{{'x' is borrowed and cannot be consumed}}
37+
where hungryCondition(x): // expected-note{{consumed here}}
38+
eat(x) // expected-note{{consumed here}}
39+
nibble(x)
40+
41+
case .copyablePayload(_borrowing x) // expected-error{{'x' is borrowed and cannot be consumed}}
42+
where condition(x):
43+
eat(x) // expected-note{{consumed here}}
44+
nibble(x)
45+
46+
// Explicit copies are OK.
47+
case .copyablePayload(_borrowing x)
48+
where hungryCondition(copy x):
49+
eat(copy x)
50+
nibble(x)
51+
52+
case .copyablePayload(_borrowing x):
53+
nibble(x)
54+
}
55+
}

test/Sema/switch-ownership.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func test(value: A) {
3131
case C():
3232
break
3333
// CHECK: (case_stmt
34-
// CHECK: (case_label_item ownership=consuming
34+
// CHECK: (case_label_item ownership=borrowing
3535
// CHECK: (pattern_expr type="A" ownership=consuming
3636
case D():
3737
break
@@ -46,7 +46,7 @@ func test(value: A) {
4646
case F():
4747
break
4848
// CHECK: (case_stmt
49-
// CHECK: (case_label_item ownership=consuming
49+
// CHECK: (case_label_item ownership=borrowing
5050
// CHECK: (pattern_expr type="A" ownership=consuming
5151
case G():
5252
break

0 commit comments

Comments
 (0)