Skip to content

Commit 2338480

Browse files
jckarterjckarter
authored
Merge pull request #66343 from jckarter/noncopyable-switch-require-consume-5.9
[5.9] Require `switch` on a noncopyable-type binding to be explicitly `consume`-d.
2 parents 3798be2 + 255f83e commit 2338480

13 files changed

+397
-111
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4567,6 +4567,9 @@ ERROR(unknown_case_multiple_patterns,none,
45674567
ERROR(unknown_case_must_be_last,none,
45684568
"'@unknown' can only be applied to the last case in a switch", ())
45694569

4570+
WARNING(move_only_pattern_match_not_consumed,none,
4571+
"noncopyable binding being pattern-matched must have the 'consume' operator applied", ())
4572+
45704573
WARNING(where_on_one_item, none,
45714574
"'where' only applies to the second pattern match in this case", ())
45724575

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6156,6 +6156,11 @@ RValue RValueEmitter::visitConsumeExpr(ConsumeExpr *E, SGFContext C) {
61566156
mv = SGF.B.createMoveValue(E, mv);
61576157
// Set the flag so we check this.
61586158
cast<MoveValueInst>(mv.getValue())->setAllowsDiagnostics(true);
6159+
if (subType.isMoveOnly()) {
6160+
// We need to move-only-check the moved value.
6161+
mv = SGF.B.createMarkMustCheckInst(E, mv,
6162+
MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
6163+
}
61596164
return RValue(SGF, {mv}, subType.getASTType());
61606165
}
61616166

‎lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerUtils.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,10 @@ bool swift::siloptimizer::searchForCandidateObjectMarkMustChecks(
133133
}
134134
}
135135

136-
// Any time we have a lexical move_value, we can process it.
136+
// Any time we have a move_value, we can process it.
137137
if (auto *mvi = dyn_cast<MoveValueInst>(mmci->getOperand())) {
138-
if (mvi->isLexical()) {
139-
moveIntroducersToProcess.insert(mmci);
140-
continue;
141-
}
138+
moveIntroducersToProcess.insert(mmci);
139+
continue;
142140
}
143141

144142
if (auto *arg = dyn_cast<SILFunctionArgument>(mmci->getOperand())) {

‎lib/SILOptimizer/Transforms/AllocBoxToStack.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,22 @@ static void replaceProjectBoxUsers(SILValue heapBox, SILValue stackBox) {
526526
}
527527
}
528528

529+
static void replaceAllNonDebugUsesWith(SILValue value,
530+
SILValue with) {
531+
auto useI = value->use_begin();
532+
while (useI != value->use_end()) {
533+
Operand *op = *useI;
534+
++useI;
535+
// Leave debug instructions on the original value.
536+
if (op->getUser()->isDebugInstruction()) {
537+
continue;
538+
}
539+
540+
// Rewrite all other uses.
541+
op->set(with);
542+
}
543+
}
544+
529545
static void hoistMarkMustCheckInsts(SILValue stackBox,
530546
MarkMustCheckInst::CheckKind checkKind) {
531547
StackList<Operand *> worklist(stackBox->getFunction());
@@ -571,7 +587,9 @@ static void hoistMarkMustCheckInsts(SILValue stackBox,
571587
auto *undef = SILUndef::get(stackBox->getType(), *stackBox->getModule());
572588

573589
auto *mmci = builder.createMarkMustCheckInst(loc, undef, checkKind);
574-
stackBox->replaceAllUsesWith(mmci);
590+
// Leave debug uses on the to-be-promoted box, but hoist all other uses to the
591+
// new mark_must_check.
592+
replaceAllNonDebugUsesWith(stackBox, mmci);
575593
mmci->setOperand(stackBox);
576594
}
577595

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4051,9 +4051,40 @@ void swift::performAbstractFuncDeclDiagnostics(AbstractFunctionDecl *AFD) {
40514051
}
40524052
}
40534053

4054+
static void
4055+
diagnoseMoveOnlyPatternMatchSubject(ASTContext &C,
4056+
Expr *subjectExpr) {
4057+
// For now, move-only types must use the `consume` operator to be
4058+
// pattern matched. Pattern matching is only implemented as a consuming
4059+
// operation today, but we don't want to be stuck with that as the default
4060+
// in the fullness of time when we get borrowing pattern matching later.
4061+
4062+
// Don't bother if the subject wasn't given a valid type, or is a copyable
4063+
// type.
4064+
auto subjectType = subjectExpr->getType();
4065+
if (!subjectType
4066+
|| subjectType->hasError()
4067+
|| !subjectType->isPureMoveOnly()) {
4068+
return;
4069+
}
4070+
4071+
// A bare reference to, or load from, a move-only binding must be consumed.
4072+
subjectExpr = subjectExpr->getSemanticsProvidingExpr();
4073+
if (auto load = dyn_cast<LoadExpr>(subjectExpr)) {
4074+
subjectExpr = load->getSubExpr()->getSemanticsProvidingExpr();
4075+
}
4076+
if (isa<DeclRefExpr>(subjectExpr)) {
4077+
C.Diags.diagnose(subjectExpr->getLoc(),
4078+
diag::move_only_pattern_match_not_consumed)
4079+
.fixItInsert(subjectExpr->getStartLoc(), "consume ");
4080+
}
4081+
}
4082+
40544083
// Perform MiscDiagnostics on Switch Statements.
40554084
static void checkSwitch(ASTContext &ctx, const SwitchStmt *stmt,
40564085
DeclContext *DC) {
4086+
diagnoseMoveOnlyPatternMatchSubject(ctx, stmt->getSubjectExpr());
4087+
40574088
// We want to warn about "case .Foo, .Bar where 1 != 100:" since the where
40584089
// clause only applies to the second case, and this is surprising.
40594090
for (auto cs : stmt->getCases()) {
@@ -4815,7 +4846,9 @@ static void checkLabeledStmtConditions(ASTContext &ctx,
48154846

48164847
switch (elt.getKind()) {
48174848
case StmtConditionElement::CK_Boolean:
4849+
break;
48184850
case StmtConditionElement::CK_PatternBinding:
4851+
diagnoseMoveOnlyPatternMatchSubject(ctx, elt.getInitializer());
48194852
break;
48204853
case StmtConditionElement::CK_Availability: {
48214854
auto info = elt.getAvailability();

‎lib/Sema/TypeCheckStmt.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -876,6 +876,7 @@ bool TypeChecker::typeCheckStmtConditionElement(StmtConditionElement &elt,
876876
bool hadError = TypeChecker::typeCheckBinding(pattern, init, dc, patternType);
877877
elt.setPattern(pattern);
878878
elt.setInitializer(init);
879+
879880
isFalsable |= pattern->isRefutablePattern();
880881
return hadError;
881882
}

‎test/SILOptimizer/moveonly_addresschecker_diagnostics.swift

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1741,21 +1741,21 @@ public func enumAssignToVar5Arg2(_ x2: inout EnumTy) { // expected-error {{missi
17411741
public func enumPatternMatchIfLet1() {
17421742
var x2 = EnumTy.klass(Klass()) // expected-error {{'x2' consumed more than once}}
17431743
x2 = EnumTy.klass(Klass())
1744-
if case let EnumTy.klass(x) = x2 { // expected-note {{consumed here}}
1744+
if case let EnumTy.klass(x) = consume x2 { // expected-note {{consumed here}}
17451745
borrowVal(x)
17461746
}
1747-
if case let EnumTy.klass(x) = x2 { // expected-note {{consumed again here}}
1747+
if case let EnumTy.klass(x) = consume x2 { // expected-note {{consumed again here}}
17481748
borrowVal(x)
17491749
}
17501750
}
17511751

17521752
public func enumPatternMatchIfLet1Arg(_ x2: inout EnumTy) {
17531753
// expected-error @-1 {{missing reinitialization of inout parameter 'x2' after consume}}
17541754
// expected-error @-2 {{'x2' consumed more than once}}
1755-
if case let EnumTy.klass(x) = x2 { // expected-note {{consumed here}}
1755+
if case let EnumTy.klass(x) = consume x2 { // expected-note {{consumed here}}
17561756
borrowVal(x)
17571757
}
1758-
if case let EnumTy.klass(x) = x2 { // expected-note {{consumed here}}
1758+
if case let EnumTy.klass(x) = consume x2 { // expected-note {{consumed here}}
17591759
// expected-note @-1 {{consumed again here}}
17601760
borrowVal(x)
17611761
}
@@ -1765,15 +1765,15 @@ public func enumPatternMatchIfLet2() {
17651765
var x2 = EnumTy.klass(Klass()) // expected-error {{'x2' consumed in a loop}}
17661766
x2 = EnumTy.klass(Klass())
17671767
for _ in 0..<1024 {
1768-
if case let EnumTy.klass(x) = x2 { // expected-note {{consumed here}}
1768+
if case let EnumTy.klass(x) = consume x2 { // expected-note {{consumed here}}
17691769
borrowVal(x)
17701770
}
17711771
}
17721772
}
17731773

17741774
public func enumPatternMatchIfLet2Arg(_ x2: inout EnumTy) { // expected-error {{missing reinitialization of inout parameter 'x2' after consume}}
17751775
for _ in 0..<1024 {
1776-
if case let EnumTy.klass(x) = x2 { // expected-note {{consumed here}}
1776+
if case let EnumTy.klass(x) = consume x2 { // expected-note {{consumed here}}
17771777
borrowVal(x)
17781778
}
17791779
}
@@ -1782,7 +1782,7 @@ public func enumPatternMatchIfLet2Arg(_ x2: inout EnumTy) { // expected-error {{
17821782
public func enumPatternMatchSwitch1() {
17831783
var x2 = EnumTy.klass(Klass()) // expected-error {{'x2' used after consume}}
17841784
x2 = EnumTy.klass(Klass())
1785-
switch x2 { // expected-note {{consumed here}}
1785+
switch consume x2 { // expected-note {{consumed here}}
17861786
case let EnumTy.klass(k):
17871787
borrowVal(k)
17881788
borrowVal(x2) // expected-note {{used here}}
@@ -1792,7 +1792,7 @@ public func enumPatternMatchSwitch1() {
17921792
}
17931793

17941794
public func enumPatternMatchSwitch1Arg(_ x2: inout EnumTy) { // expected-error {{missing reinitialization of inout parameter 'x2' after consume}}
1795-
switch x2 { // expected-note {{consumed here}}
1795+
switch consume x2 { // expected-note {{consumed here}}
17961796
case let EnumTy.klass(k):
17971797
borrowVal(k)
17981798
borrowVal(x2)
@@ -1804,7 +1804,7 @@ public func enumPatternMatchSwitch1Arg(_ x2: inout EnumTy) { // expected-error {
18041804
public func enumPatternMatchSwitch2() {
18051805
var x2 = EnumTy.klass(Klass())
18061806
x2 = EnumTy.klass(Klass())
1807-
switch x2 {
1807+
switch consume x2 {
18081808
case let EnumTy.klass(k):
18091809
borrowVal(k)
18101810
case .int:
@@ -1813,7 +1813,7 @@ public func enumPatternMatchSwitch2() {
18131813
}
18141814

18151815
public func enumPatternMatchSwitch2Arg(_ x2: inout EnumTy) { // expected-error {{missing reinitialization of inout parameter 'x2' after consume}}
1816-
switch x2 { // expected-note {{consumed here}}
1816+
switch consume x2 { // expected-note {{consumed here}}
18171817
case let EnumTy.klass(k):
18181818
borrowVal(k)
18191819
case .int:
@@ -1825,7 +1825,7 @@ public func enumPatternMatchSwitch2Arg(_ x2: inout EnumTy) { // expected-error {
18251825
public func enumPatternMatchSwitch2WhereClause() {
18261826
var x2 = EnumTy.klass(Klass()) // expected-error {{'x2' used after consume}}
18271827
x2 = EnumTy.klass(Klass())
1828-
switch x2 { // expected-note {{consumed here}}
1828+
switch consume x2 { // expected-note {{consumed here}}
18291829
case let EnumTy.klass(k)
18301830
where x2.doSomething(): // expected-note {{used here}}
18311831
borrowVal(k)
@@ -1837,7 +1837,7 @@ public func enumPatternMatchSwitch2WhereClause() {
18371837
}
18381838

18391839
public func enumPatternMatchSwitch2WhereClauseArg(_ x2: inout EnumTy) { // expected-error {{missing reinitialization of inout parameter 'x2' after consume}}
1840-
switch x2 { // expected-note {{consumed here}}
1840+
switch consume x2 { // expected-note {{consumed here}}
18411841
case let EnumTy.klass(k)
18421842
where x2.doSomething():
18431843
borrowVal(k)
@@ -1851,7 +1851,7 @@ public func enumPatternMatchSwitch2WhereClauseArg(_ x2: inout EnumTy) { // expec
18511851
public func enumPatternMatchSwitch2WhereClause2() {
18521852
var x2 = EnumTy.klass(Klass())
18531853
x2 = EnumTy.klass(Klass())
1854-
switch x2 {
1854+
switch consume x2 {
18551855
case let EnumTy.klass(k)
18561856
where boolValue:
18571857
borrowVal(k)
@@ -1863,7 +1863,7 @@ public func enumPatternMatchSwitch2WhereClause2() {
18631863
}
18641864

18651865
public func enumPatternMatchSwitch2WhereClause2Arg(_ x2: inout EnumTy) { // expected-error {{missing reinitialization of inout parameter 'x2' after consume}}
1866-
switch x2 { // expected-note {{consumed here}}
1866+
switch consume x2 { // expected-note {{consumed here}}
18671867
case let EnumTy.klass(k)
18681868
where boolValue:
18691869
borrowVal(k)
@@ -3968,7 +3968,7 @@ func fieldSensitiveTestReinitFieldMultiBlock4() {
39683968
func fieldSensitiveTestReinitEnumMultiBlock() {
39693969
var e = NonTrivialEnum.first // expected-error {{'e' used after consume}}
39703970
e = NonTrivialEnum.second(Klass())
3971-
switch e { // expected-note {{consumed here}}
3971+
switch consume e { // expected-note {{consumed here}}
39723972
case .second:
39733973
e = NonTrivialEnum.third(NonTrivialStruct())
39743974
default:
@@ -3980,7 +3980,7 @@ func fieldSensitiveTestReinitEnumMultiBlock() {
39803980
func fieldSensitiveTestReinitEnumMultiBlock1() {
39813981
var e = NonTrivialEnum.first
39823982
e = NonTrivialEnum.second(Klass())
3983-
switch e {
3983+
switch consume e {
39843984
case .second:
39853985
e = NonTrivialEnum.third(NonTrivialStruct())
39863986
default:
@@ -3993,7 +3993,7 @@ func fieldSensitiveTestReinitEnumMultiBlock2() {
39933993
var e = NonTrivialEnum.first
39943994
e = NonTrivialEnum.second(Klass())
39953995
if boolValue {
3996-
switch e {
3996+
switch consume e {
39973997
case .second:
39983998
e = NonTrivialEnum.third(NonTrivialStruct())
39993999
default:
@@ -4398,54 +4398,54 @@ func borrow(_ x: borrowing MyEnum) {}
43984398

43994399
func testMyEnum() {
44004400
func test1(_ x: consuming MyEnum) {
4401-
if case let .first(y) = x {
4401+
if case let .first(y) = consume x {
44024402
_ = y
44034403
}
44044404
}
44054405

44064406
func test1a(_ x: consuming MyEnum) { // expected-error {{'x' consumed more than once}}
4407-
if case let .first(y) = x { // expected-note {{consumed here}}
4407+
if case let .first(y) = consume x { // expected-note {{consumed here}}
44084408
_ = consume x // expected-note {{consumed again here}}
44094409
_ = y
44104410
}
44114411
}
44124412

44134413
func test1b(_ x: consuming MyEnum) { // expected-error {{'x' consumed more than once}}
4414-
if case let .first(y) = x { // expected-note {{consumed here}}
4414+
if case let .first(y) = consume x { // expected-note {{consumed here}}
44154415
_ = y
44164416
}
44174417
_ = consume x // expected-note {{consumed again here}}
44184418
}
44194419

44204420
func test2(_ x: consuming MyEnum) {
4421-
if case let .third(.first(y)) = x {
4421+
if case let .third(.first(y)) = consume x {
44224422
_ = y
44234423
}
44244424
}
44254425

44264426
func test2a(_ x: consuming MyEnum) { // expected-error {{'x' consumed more than once}}
4427-
if case let .third(.first(y)) = x { // expected-note {{consumed here}}
4427+
if case let .third(.first(y)) = consume x { // expected-note {{consumed here}}
44284428
_ = consume x // expected-note {{consumed again here}}
44294429
_ = y
44304430
}
44314431
}
44324432

44334433
func test2b(_ x: consuming MyEnum) { // expected-error {{'x' consumed more than once}}
4434-
if case let .third(.first(y)) = x { // expected-note {{consumed here}}
4434+
if case let .third(.first(y)) = consume x { // expected-note {{consumed here}}
44354435
_ = y
44364436
}
44374437
_ = consume x // expected-note {{consumed again here}}
44384438
}
44394439

44404440
func test2c(_ x: consuming MyEnum) { // expected-error {{'x' used after consume}}
4441-
if case let .third(.first(y)) = x { // expected-note {{consumed here}}
4441+
if case let .third(.first(y)) = consume x { // expected-note {{consumed here}}
44424442
_ = y
44434443
}
44444444
borrow(x) // expected-note {{used here}}
44454445
}
44464446

44474447
func test3(_ x: consuming MyEnum) {
4448-
switch x {
4448+
switch consume x {
44494449
case let .first(y):
44504450
_ = y
44514451
break
@@ -4455,7 +4455,7 @@ func testMyEnum() {
44554455
}
44564456

44574457
func test3a(_ x: consuming MyEnum) { // expected-error {{'x' consumed more than once}}
4458-
switch x { // expected-note {{consumed here}}
4458+
switch consume x { // expected-note {{consumed here}}
44594459
case let .first(y):
44604460
_ = y
44614461
break
@@ -4466,7 +4466,7 @@ func testMyEnum() {
44664466
}
44674467

44684468
func test4(_ x: consuming MyEnum) {
4469-
switch x {
4469+
switch consume x {
44704470
case let .third(.first(y)):
44714471
_ = y
44724472
break
@@ -4476,7 +4476,7 @@ func testMyEnum() {
44764476
}
44774477

44784478
func test4a(_ x: consuming MyEnum) { // expected-error {{'x' consumed more than once}}
4479-
switch x { // expected-note {{consumed here}}
4479+
switch consume x { // expected-note {{consumed here}}
44804480
case let .third(.first(y)):
44814481
_ = y
44824482
break

‎test/SILOptimizer/moveonly_discard.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ final class Wallet {
8484
}
8585

8686
__consuming func inspect() { // expected-error {{'self' consumed more than once}}
87-
switch self { // expected-note {{consumed here}}
87+
switch consume self { // expected-note {{consumed here}}
8888
case .green, .yellow, .red:
8989
discard self // expected-note {{consumed again here}}
9090
default:

0 commit comments

Comments
 (0)