Skip to content

Commit 4591e39

Browse files
committed
[move-only] Fix borrowing address only no consume diagnostic to not say can't capture.
We previously were emitting a consuming partial_apply diagnostic. I had to reformulate slightly the way we pattern match the diagnostics to make sure that we get the proper no consume diagnostic. rdar://111461837
1 parent 12ebe0a commit 4591e39

File tree

5 files changed

+61
-6
lines changed

5 files changed

+61
-6
lines changed

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1852,11 +1852,28 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
18521852
assert(checkKind == MarkMustCheckInst::CheckKind::NoConsumeOrAssign);
18531853
moveChecker.diagnosticEmitter.emitObjectGuaranteedDiagnostic(
18541854
markedValue);
1855-
} else {
1856-
// Otherwise, we need to emit an escaping closure error.
1855+
return true;
1856+
}
1857+
1858+
// If we have a function argument that is no_consume_or_assign and we do
1859+
// not have any partial apply uses, then we know that we have a use of
1860+
// an address only borrowed parameter that we need to
1861+
if (fArg &&
1862+
checkKind == MarkMustCheckInst::CheckKind::NoConsumeOrAssign &&
1863+
!moveChecker.canonicalizer.hasPartialApplyConsumingUse()) {
1864+
moveChecker.diagnosticEmitter.emitObjectGuaranteedDiagnostic(
1865+
markedValue);
1866+
return true;
1867+
}
1868+
1869+
// Finally try to emit either a global or class field error...
1870+
if (!moveChecker.diagnosticEmitter
1871+
.emitGlobalOrClassFieldLoadedAndConsumed(markedValue)) {
1872+
// And otherwise if we failed emit an escaping closure error.
18571873
moveChecker.diagnosticEmitter
18581874
.emitAddressEscapingClosureCaptureLoadedAndConsumed(markedValue);
18591875
}
1876+
18601877
return true;
18611878
}
18621879

lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -775,7 +775,7 @@ void DiagnosticEmitter::emitObjectInstConsumesAndUsesValue(
775775
registerDiagnosticEmitted(markedValue);
776776
}
777777

778-
void DiagnosticEmitter::emitAddressEscapingClosureCaptureLoadedAndConsumed(
778+
bool DiagnosticEmitter::emitGlobalOrClassFieldLoadedAndConsumed(
779779
MarkMustCheckInst *markedValue) {
780780
SmallString<64> varName;
781781
getVariableNameForValue(markedValue, varName);
@@ -789,7 +789,7 @@ void DiagnosticEmitter::emitAddressEscapingClosureCaptureLoadedAndConsumed(
789789
diag::sil_movechecking_notconsumable_but_assignable_was_consumed,
790790
varName, /*isGlobal=*/false);
791791
registerDiagnosticEmitted(markedValue);
792-
return;
792+
return true;
793793
}
794794

795795
// is it a global?
@@ -799,10 +799,16 @@ void DiagnosticEmitter::emitAddressEscapingClosureCaptureLoadedAndConsumed(
799799
diag::sil_movechecking_notconsumable_but_assignable_was_consumed,
800800
varName, /*isGlobal=*/true);
801801
registerDiagnosticEmitted(markedValue);
802-
return;
802+
return true;
803803
}
804804

805-
// remaining cases must be a closure capture.
805+
return false;
806+
}
807+
808+
void DiagnosticEmitter::emitAddressEscapingClosureCaptureLoadedAndConsumed(
809+
MarkMustCheckInst *markedValue) {
810+
SmallString<64> varName;
811+
getVariableNameForValue(markedValue, varName);
806812
diagnose(markedValue->getModule().getASTContext(),
807813
markedValue,
808814
diag::sil_movechecking_capture_consumed,

lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,19 @@ class DiagnosticEmitter {
160160
Operand *consumingUse,
161161
Operand *nonConsumingUse);
162162

163+
/// Emit a diagnostic for a case where we have one of the following cases:
164+
///
165+
/// 1. A partial_apply formed from a borrowed address only value.
166+
/// 2. A use of a captured value in a closure callee.
163167
void emitAddressEscapingClosureCaptureLoadedAndConsumed(
164168
MarkMustCheckInst *markedValue);
169+
170+
/// Try to emit a diagnostic for a load/consume from an
171+
/// assignable_but_not_consumable access to a global or a class field. Returns
172+
/// false if we did not find something we pattern matched as being either of
173+
/// those cases. Returns true if we emitted a diagnostic.
174+
bool emitGlobalOrClassFieldLoadedAndConsumed(MarkMustCheckInst *markedValue);
175+
165176
void emitPromotedBoxArgumentError(MarkMustCheckInst *markedValue,
166177
SILFunctionArgument *arg);
167178

test/SILOptimizer/moveonly_addresschecker_diagnostics.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2476,6 +2476,25 @@ public func addressOnlyGenericAccessConsumeGrandFieldArg4a<T>(_ x2: consuming Ad
24762476
}
24772477
}
24782478

2479+
public func addressOnlyGenericBorrowingConsume<T>(_ x: borrowing AddressOnlyGeneric<T>) {
2480+
// expected-error @-1 {{'x' is borrowed and cannot be consumed}}
2481+
let _ = x // expected-note {{consumed here}}
2482+
}
2483+
2484+
public func addressOnlyGenericBorrowingConsumeField<T>(_ x: borrowing AddressOnlyGeneric<T>) {
2485+
// expected-error @-1 {{'x' is borrowed and cannot be consumed}}
2486+
let _ = x.moveOnly // expected-note {{consumed here}}
2487+
}
2488+
2489+
public func addressOnlyGenericBorrowingConsumeField2<T>(_ x: borrowing AddressOnlyGeneric<T>) {
2490+
let _ = x.copyable
2491+
}
2492+
2493+
public func addressOnlyGenericBorrowingConsumeGrandField<T>(_ x: borrowing AddressOnlyGeneric<T>) {
2494+
// expected-error @-1 {{'x' is borrowed and cannot be consumed}}
2495+
let _ = x.moveOnly.k // expected-note {{consumed here}}
2496+
}
2497+
24792498
extension AddressOnlyGeneric {
24802499
func testNoUseSelf() { // expected-error {{'self' is borrowed and cannot be consumed}}
24812500
let x = self // expected-note {{consumed here}}

test/SILOptimizer/moveonly_partial_consumption.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,8 @@ func addressOnlyTestArg(_ x: borrowing AddressOnlyType) {
216216
// expected-error @-1 {{'x' is borrowed and cannot be consumed}}
217217
// expected-error @-2 {{'x' is borrowed and cannot be consumed}}
218218
// expected-error @-3 {{'x' is borrowed and cannot be consumed}}
219+
// expected-error @-4 {{'x' is borrowed and cannot be consumed}}
220+
let _ = x.e // expected-note {{consumed here}}
219221
let _ = x.k
220222
let _ = x.l.e // expected-note {{consumed here}}
221223
let _ = x.l.k // expected-note {{consumed here}}

0 commit comments

Comments
 (0)