Skip to content

[5.9] Require switch on a noncopyable-type binding to be explicitly consume-d. #66343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -4567,6 +4567,9 @@ ERROR(unknown_case_multiple_patterns,none,
ERROR(unknown_case_must_be_last,none,
"'@unknown' can only be applied to the last case in a switch", ())

WARNING(move_only_pattern_match_not_consumed,none,
"noncopyable binding being pattern-matched must have the 'consume' operator applied", ())

WARNING(where_on_one_item, none,
"'where' only applies to the second pattern match in this case", ())

Expand Down
5 changes: 5 additions & 0 deletions lib/SILGen/SILGenExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6156,6 +6156,11 @@ RValue RValueEmitter::visitConsumeExpr(ConsumeExpr *E, SGFContext C) {
mv = SGF.B.createMoveValue(E, mv);
// Set the flag so we check this.
cast<MoveValueInst>(mv.getValue())->setAllowsDiagnostics(true);
if (subType.isMoveOnly()) {
// We need to move-only-check the moved value.
mv = SGF.B.createMarkMustCheckInst(E, mv,
MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
}
return RValue(SGF, {mv}, subType.getASTType());
}

Expand Down
8 changes: 3 additions & 5 deletions lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,10 @@ bool swift::siloptimizer::searchForCandidateObjectMarkMustChecks(
}
}

// Any time we have a lexical move_value, we can process it.
// Any time we have a move_value, we can process it.
if (auto *mvi = dyn_cast<MoveValueInst>(mmci->getOperand())) {
if (mvi->isLexical()) {
moveIntroducersToProcess.insert(mmci);
continue;
}
moveIntroducersToProcess.insert(mmci);
continue;
}

if (auto *arg = dyn_cast<SILFunctionArgument>(mmci->getOperand())) {
Expand Down
20 changes: 19 additions & 1 deletion lib/SILOptimizer/Transforms/AllocBoxToStack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,22 @@ static void replaceProjectBoxUsers(SILValue heapBox, SILValue stackBox) {
}
}

static void replaceAllNonDebugUsesWith(SILValue value,
SILValue with) {
auto useI = value->use_begin();
while (useI != value->use_end()) {
Operand *op = *useI;
++useI;
// Leave debug instructions on the original value.
if (op->getUser()->isDebugInstruction()) {
continue;
}

// Rewrite all other uses.
op->set(with);
}
}

static void hoistMarkMustCheckInsts(SILValue stackBox,
MarkMustCheckInst::CheckKind checkKind) {
StackList<Operand *> worklist(stackBox->getFunction());
Expand Down Expand Up @@ -571,7 +587,9 @@ static void hoistMarkMustCheckInsts(SILValue stackBox,
auto *undef = SILUndef::get(stackBox->getType(), *stackBox->getModule());

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

Expand Down
33 changes: 33 additions & 0 deletions lib/Sema/MiscDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4051,9 +4051,40 @@ void swift::performAbstractFuncDeclDiagnostics(AbstractFunctionDecl *AFD) {
}
}

static void
diagnoseMoveOnlyPatternMatchSubject(ASTContext &C,
Expr *subjectExpr) {
// For now, move-only types must use the `consume` operator to be
// pattern matched. Pattern matching is only implemented as a consuming
// operation today, but we don't want to be stuck with that as the default
// in the fullness of time when we get borrowing pattern matching later.

// Don't bother if the subject wasn't given a valid type, or is a copyable
// type.
auto subjectType = subjectExpr->getType();
if (!subjectType
|| subjectType->hasError()
|| !subjectType->isPureMoveOnly()) {
return;
}

// A bare reference to, or load from, a move-only binding must be consumed.
subjectExpr = subjectExpr->getSemanticsProvidingExpr();
if (auto load = dyn_cast<LoadExpr>(subjectExpr)) {
subjectExpr = load->getSubExpr()->getSemanticsProvidingExpr();
}
if (isa<DeclRefExpr>(subjectExpr)) {
C.Diags.diagnose(subjectExpr->getLoc(),
diag::move_only_pattern_match_not_consumed)
.fixItInsert(subjectExpr->getStartLoc(), "consume ");
}
}

// Perform MiscDiagnostics on Switch Statements.
static void checkSwitch(ASTContext &ctx, const SwitchStmt *stmt,
DeclContext *DC) {
diagnoseMoveOnlyPatternMatchSubject(ctx, stmt->getSubjectExpr());

// We want to warn about "case .Foo, .Bar where 1 != 100:" since the where
// clause only applies to the second case, and this is surprising.
for (auto cs : stmt->getCases()) {
Expand Down Expand Up @@ -4815,7 +4846,9 @@ static void checkLabeledStmtConditions(ASTContext &ctx,

switch (elt.getKind()) {
case StmtConditionElement::CK_Boolean:
break;
case StmtConditionElement::CK_PatternBinding:
diagnoseMoveOnlyPatternMatchSubject(ctx, elt.getInitializer());
break;
case StmtConditionElement::CK_Availability: {
auto info = elt.getAvailability();
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/TypeCheckStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,7 @@ bool TypeChecker::typeCheckStmtConditionElement(StmtConditionElement &elt,
bool hadError = TypeChecker::typeCheckBinding(pattern, init, dc, patternType);
elt.setPattern(pattern);
elt.setInitializer(init);

isFalsable |= pattern->isRefutablePattern();
return hadError;
}
Expand Down
56 changes: 28 additions & 28 deletions test/SILOptimizer/moveonly_addresschecker_diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1741,21 +1741,21 @@ public func enumAssignToVar5Arg2(_ x2: inout EnumTy) { // expected-error {{missi
public func enumPatternMatchIfLet1() {
var x2 = EnumTy.klass(Klass()) // expected-error {{'x2' consumed more than once}}
x2 = EnumTy.klass(Klass())
if case let EnumTy.klass(x) = x2 { // expected-note {{consumed here}}
if case let EnumTy.klass(x) = consume x2 { // expected-note {{consumed here}}
borrowVal(x)
}
if case let EnumTy.klass(x) = x2 { // expected-note {{consumed again here}}
if case let EnumTy.klass(x) = consume x2 { // expected-note {{consumed again here}}
borrowVal(x)
}
}

public func enumPatternMatchIfLet1Arg(_ x2: inout EnumTy) {
// expected-error @-1 {{missing reinitialization of inout parameter 'x2' after consume}}
// expected-error @-2 {{'x2' consumed more than once}}
if case let EnumTy.klass(x) = x2 { // expected-note {{consumed here}}
if case let EnumTy.klass(x) = consume x2 { // expected-note {{consumed here}}
borrowVal(x)
}
if case let EnumTy.klass(x) = x2 { // expected-note {{consumed here}}
if case let EnumTy.klass(x) = consume x2 { // expected-note {{consumed here}}
// expected-note @-1 {{consumed again here}}
borrowVal(x)
}
Expand All @@ -1765,15 +1765,15 @@ public func enumPatternMatchIfLet2() {
var x2 = EnumTy.klass(Klass()) // expected-error {{'x2' consumed in a loop}}
x2 = EnumTy.klass(Klass())
for _ in 0..<1024 {
if case let EnumTy.klass(x) = x2 { // expected-note {{consumed here}}
if case let EnumTy.klass(x) = consume x2 { // expected-note {{consumed here}}
borrowVal(x)
}
}
}

public func enumPatternMatchIfLet2Arg(_ x2: inout EnumTy) { // expected-error {{missing reinitialization of inout parameter 'x2' after consume}}
for _ in 0..<1024 {
if case let EnumTy.klass(x) = x2 { // expected-note {{consumed here}}
if case let EnumTy.klass(x) = consume x2 { // expected-note {{consumed here}}
borrowVal(x)
}
}
Expand All @@ -1782,7 +1782,7 @@ public func enumPatternMatchIfLet2Arg(_ x2: inout EnumTy) { // expected-error {{
public func enumPatternMatchSwitch1() {
var x2 = EnumTy.klass(Klass()) // expected-error {{'x2' used after consume}}
x2 = EnumTy.klass(Klass())
switch x2 { // expected-note {{consumed here}}
switch consume x2 { // expected-note {{consumed here}}
case let EnumTy.klass(k):
borrowVal(k)
borrowVal(x2) // expected-note {{used here}}
Expand All @@ -1792,7 +1792,7 @@ public func enumPatternMatchSwitch1() {
}

public func enumPatternMatchSwitch1Arg(_ x2: inout EnumTy) { // expected-error {{missing reinitialization of inout parameter 'x2' after consume}}
switch x2 { // expected-note {{consumed here}}
switch consume x2 { // expected-note {{consumed here}}
case let EnumTy.klass(k):
borrowVal(k)
borrowVal(x2)
Expand All @@ -1804,7 +1804,7 @@ public func enumPatternMatchSwitch1Arg(_ x2: inout EnumTy) { // expected-error {
public func enumPatternMatchSwitch2() {
var x2 = EnumTy.klass(Klass())
x2 = EnumTy.klass(Klass())
switch x2 {
switch consume x2 {
case let EnumTy.klass(k):
borrowVal(k)
case .int:
Expand All @@ -1813,7 +1813,7 @@ public func enumPatternMatchSwitch2() {
}

public func enumPatternMatchSwitch2Arg(_ x2: inout EnumTy) { // expected-error {{missing reinitialization of inout parameter 'x2' after consume}}
switch x2 { // expected-note {{consumed here}}
switch consume x2 { // expected-note {{consumed here}}
case let EnumTy.klass(k):
borrowVal(k)
case .int:
Expand All @@ -1825,7 +1825,7 @@ public func enumPatternMatchSwitch2Arg(_ x2: inout EnumTy) { // expected-error {
public func enumPatternMatchSwitch2WhereClause() {
var x2 = EnumTy.klass(Klass()) // expected-error {{'x2' used after consume}}
x2 = EnumTy.klass(Klass())
switch x2 { // expected-note {{consumed here}}
switch consume x2 { // expected-note {{consumed here}}
case let EnumTy.klass(k)
where x2.doSomething(): // expected-note {{used here}}
borrowVal(k)
Expand All @@ -1837,7 +1837,7 @@ public func enumPatternMatchSwitch2WhereClause() {
}

public func enumPatternMatchSwitch2WhereClauseArg(_ x2: inout EnumTy) { // expected-error {{missing reinitialization of inout parameter 'x2' after consume}}
switch x2 { // expected-note {{consumed here}}
switch consume x2 { // expected-note {{consumed here}}
case let EnumTy.klass(k)
where x2.doSomething():
borrowVal(k)
Expand All @@ -1851,7 +1851,7 @@ public func enumPatternMatchSwitch2WhereClauseArg(_ x2: inout EnumTy) { // expec
public func enumPatternMatchSwitch2WhereClause2() {
var x2 = EnumTy.klass(Klass())
x2 = EnumTy.klass(Klass())
switch x2 {
switch consume x2 {
case let EnumTy.klass(k)
where boolValue:
borrowVal(k)
Expand All @@ -1863,7 +1863,7 @@ public func enumPatternMatchSwitch2WhereClause2() {
}

public func enumPatternMatchSwitch2WhereClause2Arg(_ x2: inout EnumTy) { // expected-error {{missing reinitialization of inout parameter 'x2' after consume}}
switch x2 { // expected-note {{consumed here}}
switch consume x2 { // expected-note {{consumed here}}
case let EnumTy.klass(k)
where boolValue:
borrowVal(k)
Expand Down Expand Up @@ -3968,7 +3968,7 @@ func fieldSensitiveTestReinitFieldMultiBlock4() {
func fieldSensitiveTestReinitEnumMultiBlock() {
var e = NonTrivialEnum.first // expected-error {{'e' used after consume}}
e = NonTrivialEnum.second(Klass())
switch e { // expected-note {{consumed here}}
switch consume e { // expected-note {{consumed here}}
case .second:
e = NonTrivialEnum.third(NonTrivialStruct())
default:
Expand All @@ -3980,7 +3980,7 @@ func fieldSensitiveTestReinitEnumMultiBlock() {
func fieldSensitiveTestReinitEnumMultiBlock1() {
var e = NonTrivialEnum.first
e = NonTrivialEnum.second(Klass())
switch e {
switch consume e {
case .second:
e = NonTrivialEnum.third(NonTrivialStruct())
default:
Expand All @@ -3993,7 +3993,7 @@ func fieldSensitiveTestReinitEnumMultiBlock2() {
var e = NonTrivialEnum.first
e = NonTrivialEnum.second(Klass())
if boolValue {
switch e {
switch consume e {
case .second:
e = NonTrivialEnum.third(NonTrivialStruct())
default:
Expand Down Expand Up @@ -4398,54 +4398,54 @@ func borrow(_ x: borrowing MyEnum) {}

func testMyEnum() {
func test1(_ x: consuming MyEnum) {
if case let .first(y) = x {
if case let .first(y) = consume x {
_ = y
}
}

func test1a(_ x: consuming MyEnum) { // expected-error {{'x' consumed more than once}}
if case let .first(y) = x { // expected-note {{consumed here}}
if case let .first(y) = consume x { // expected-note {{consumed here}}
_ = consume x // expected-note {{consumed again here}}
_ = y
}
}

func test1b(_ x: consuming MyEnum) { // expected-error {{'x' consumed more than once}}
if case let .first(y) = x { // expected-note {{consumed here}}
if case let .first(y) = consume x { // expected-note {{consumed here}}
_ = y
}
_ = consume x // expected-note {{consumed again here}}
}

func test2(_ x: consuming MyEnum) {
if case let .third(.first(y)) = x {
if case let .third(.first(y)) = consume x {
_ = y
}
}

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

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

func test2c(_ x: consuming MyEnum) { // expected-error {{'x' used after consume}}
if case let .third(.first(y)) = x { // expected-note {{consumed here}}
if case let .third(.first(y)) = consume x { // expected-note {{consumed here}}
_ = y
}
borrow(x) // expected-note {{used here}}
}

func test3(_ x: consuming MyEnum) {
switch x {
switch consume x {
case let .first(y):
_ = y
break
Expand All @@ -4455,7 +4455,7 @@ func testMyEnum() {
}

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

func test4(_ x: consuming MyEnum) {
switch x {
switch consume x {
case let .third(.first(y)):
_ = y
break
Expand All @@ -4476,7 +4476,7 @@ func testMyEnum() {
}

func test4a(_ x: consuming MyEnum) { // expected-error {{'x' consumed more than once}}
switch x { // expected-note {{consumed here}}
switch consume x { // expected-note {{consumed here}}
case let .third(.first(y)):
_ = y
break
Expand Down
2 changes: 1 addition & 1 deletion test/SILOptimizer/moveonly_discard.swift
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ final class Wallet {
}

__consuming func inspect() { // expected-error {{'self' consumed more than once}}
switch self { // expected-note {{consumed here}}
switch consume self { // expected-note {{consumed here}}
case .green, .yellow, .red:
discard self // expected-note {{consumed again here}}
default:
Expand Down
Loading