Skip to content

[move-only] Fix a thinko where we are treating inout convention as a consuming use instead of liveness use. #65864

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.

Sign up for GitHub

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
merged 1 commit into from
May 11, 2023
Merged

[move-only] Fix a thinko where we are treating inout convention as a consuming use instead of liveness use. #65864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ static bool memInstMustConsume(Operand *memOper) {
return false;
ApplySite applySite(pai);
auto convention = applySite.getArgumentConvention(*memOper);
return convention.isInoutConvention();
return !convention.isInoutConvention();
}
case SILInstructionKind::DestroyAddrInst:
return true;
Expand Down Expand Up @@ -1802,8 +1802,9 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
}

if (auto *pas = dyn_cast<PartialApplyInst>(user)) {
if (pas->isOnStack()) {
LLVM_DEBUG(llvm::dbgs() << "Found on stack partial apply!\n");
if (pas->isOnStack() ||
ApplySite(pas).getArgumentConvention(*op).isInoutConvention()) {
LLVM_DEBUG(llvm::dbgs() << "Found on stack partial apply or inout usage!\n");
// On-stack partial applications and their final consumes are always a
// liveness use of their captures.
auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
Expand Down
18 changes: 4 additions & 14 deletions test/SILGen/moveonly_escaping_closure.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,10 +607,8 @@ func testConsumingEscapeClosureCaptureLet(_ f: consuming @escaping () -> ()) {
// CHECK: } // end sil function '$s16moveonly_closure29testGlobalClosureCaptureInOutyyAA9SingleEltVzFyycfU_'
var globalClosureCaptureInOut: () -> () = {}
func testGlobalClosureCaptureInOut(_ x: inout SingleElt) {
// expected-error @-1 {{'x' consumed but not reinitialized before end of function}}
// expected-note @-2 {{'x' is declared 'inout'}}
// expected-note @-1 {{'x' is declared 'inout'}}
globalClosureCaptureInOut = { // expected-error {{escaping closure captures 'inout' parameter 'x'}}
// expected-note @-1 {{consuming use here}}
borrowVal(x) // expected-note {{captured here}}
consumeVal(x) // expected-note {{captured here}}
consumeVal(x) // expected-note {{captured here}}
Expand Down Expand Up @@ -652,9 +650,7 @@ func testGlobalClosureCaptureInOut(_ x: inout SingleElt) {
// CHECK: } // end sil function '$s16moveonly_closure31testLocalLetClosureCaptureInOutyyAA9SingleEltVzFyycfU_'
func testLocalLetClosureCaptureInOut(_ x: inout SingleElt) {
// expected-note @-1 {{'x' is declared 'inout'}}
// expected-error @-2 {{'x' consumed but not reinitialized before end of function}}
let f = { // expected-error {{escaping closure captures 'inout' parameter 'x'}}
// expected-note @-1 {{consuming use here}}
borrowVal(x) // expected-note {{captured here}}
consumeVal(x) // expected-note {{captured here}}
consumeVal(x) // expected-note {{captured here}}
Expand Down Expand Up @@ -700,10 +696,8 @@ func testLocalLetClosureCaptureInOut(_ x: inout SingleElt) {
// CHECK: apply {{%.*}}([[LOADED_READ]], [[LOADED_TAKE]])
// CHECK: } // end sil function '$s16moveonly_closure31testLocalVarClosureCaptureInOutyyAA9SingleEltVzFyycfU_'
func testLocalVarClosureCaptureInOut(_ x: inout SingleElt) {
// expected-error @-1 {{'x' consumed but not reinitialized before end of function}}
// expected-note @-2 {{'x' is declared 'inout'}}
var f = { // expected-note {{consuming use here}}
// expected-error @-1 {{escaping closure captures 'inout' parameter 'x'}}
// expected-note @-1 {{'x' is declared 'inout'}}
var f = { // expected-error {{escaping closure captures 'inout' parameter 'x'}}
borrowVal(x) // expected-note {{captured here}}
consumeVal(x) // expected-note {{captured here}}
consumeVal(x) // expected-note {{captured here}}
Expand Down Expand Up @@ -750,10 +744,8 @@ func testLocalVarClosureCaptureInOut(_ x: inout SingleElt) {
// CHECK: apply {{%.*}}([[LOADED_READ]], [[LOADED_TAKE]])
// CHECK: } // end sil function '$s16moveonly_closure026testInOutVarClosureCapturedE0yyyycz_AA9SingleEltVztFyycfU_'
func testInOutVarClosureCaptureInOut(_ f: inout () -> (), _ x: inout SingleElt) {
// expected-error @-1 {{'x' consumed but not reinitialized before end of function}}
// expected-note @-2 {{'x' is declared 'inout'}}
// expected-note @-1 {{'x' is declared 'inout'}}
f = { // expected-error {{escaping closure captures 'inout' parameter 'x'}}
// expected-note @-1 {{consuming use here}}
borrowVal(x) // expected-note {{captured here}}
consumeVal(x) // expected-note {{captured here}}
consumeVal(x) // expected-note {{captured here}}
Expand Down Expand Up @@ -805,9 +797,7 @@ func testInOutVarClosureCaptureInOut(_ f: inout () -> (), _ x: inout SingleElt)
// CHECK: } // end sil function '$s16moveonly_closure38testConsumingEscapeClosureCaptureInOutyyyycn_AA9SingleEltVztFyycfU_'
func testConsumingEscapeClosureCaptureInOut(_ f: consuming @escaping () -> (), _ x: inout SingleElt) {
// expected-note @-1 {{'x' is declared 'inout'}}
// expected-error @-2 {{'x' consumed but not reinitialized before end of function}}
f = { // expected-error {{escaping closure captures 'inout' parameter 'x'}}
// expected-note @-1 {{consuming use here}}
borrowVal(x) // expected-note {{captured here}}
consumeVal(x) // expected-note {{captured here}}
consumeVal(x) // expected-note {{captured here}}
Expand Down
17 changes: 6 additions & 11 deletions test/SILOptimizer/moveonly_addresschecker_diagnostics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3116,9 +3116,8 @@ public func closureCaptureClassUseAfterConsumeError() {
}

public func closureCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
// expected-error @-1 {{'x2' consumed but not reinitialized before end of function}}
// expected-note @-2 {{'x2' is declared 'inout'}}
let f = { // expected-note {{consuming use here}}
// expected-note @-1 {{'x2' is declared 'inout'}}
let f = {
// expected-error @-1 {{escaping closure captures 'inout' parameter 'x2'}}
borrowVal(x2) // expected-note {{captured here}}
consumeVal(x2) // expected-note {{captured here}}
Expand Down Expand Up @@ -3230,12 +3229,10 @@ public func closureAndDeferCaptureClassUseAfterConsume3() {
}

public func closureAndDeferCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
// expected-error @-1 {{'x2' consumed but not reinitialized before end of function}}
// expected-error @-2 {{'x2' consumed in closure but not reinitialized before end of closure}}
// expected-error @-3 {{'x2' consumed more than once}}
// expected-note @-4 {{'x2' is declared 'inout'}}
// expected-error @-1 {{'x2' consumed in closure but not reinitialized before end of closure}}
// expected-error @-2 {{'x2' consumed more than once}}
// expected-note @-3 {{'x2' is declared 'inout'}}
let f = { // expected-error {{escaping closure captures 'inout' parameter 'x2'}}
// expected-note @-1 {{consuming use here}}
defer { // expected-note {{captured indirectly by this call}}
borrowVal(x2) // expected-note {{captured here}}
consumeVal(x2) // expected-note {{captured here}}
Expand Down Expand Up @@ -3280,11 +3277,9 @@ public func closureAndClosureCaptureClassUseAfterConsume2() {


public func closureAndClosureCaptureClassArgUseAfterConsume(_ x2: inout Klass) {
// expected-error @-1 {{'x2' consumed but not reinitialized before end of function}}
// expected-note @-1 {{'x2' is declared 'inout'}}
// expected-note @-2 {{'x2' is declared 'inout'}}
// expected-note @-3 {{'x2' is declared 'inout'}}
let f = { // expected-error {{escaping closure captures 'inout' parameter 'x2'}}
// expected-note @-1 {{consuming use here}}
let g = { // expected-error {{escaping closure captures 'inout' parameter 'x2'}}
// expected-note @-1 {{captured indirectly by this call}}
borrowVal(x2)
Expand Down
2 changes: 0 additions & 2 deletions test/SILOptimizer/moveonly_objectchecker_diagnostics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2787,9 +2787,7 @@ public func closureCaptureClassUseAfterConsume1(_ x: borrowing Klass) { // expec

public func closureCaptureClassUseAfterConsume2(_ x2: inout Klass) {
// expected-note @-1 {{'x2' is declared 'inout'}}
// expected-error @-2 {{'x2' consumed but not reinitialized before end of function}}
let f = { // expected-error {{escaping closure captures 'inout' parameter 'x2'}}
// expected-note @-1 {{consuming use here}}
borrowVal(x2) // expected-note {{captured here}}
consumeVal(x2) // expected-note {{captured here}}
consumeVal(x2) // expected-note {{captured here}}
Expand Down
17 changes: 6 additions & 11 deletions test/SILOptimizer/moveonly_trivial_addresschecker_diagnostics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1227,9 +1227,8 @@ public func closureCaptureClassUseAfterConsumeError() {
}

public func closureCaptureClassArgUseAfterConsume(_ x2: inout NonTrivialStruct) {
// expected-error @-1 {{'x2' consumed but not reinitialized before end of function}}
// expected-note @-2 {{'x2' is declared 'inout'}}
let f = { // expected-note {{consuming use here}}
// expected-note @-1 {{'x2' is declared 'inout'}}
let f = {
// expected-error @-1 {{escaping closure captures 'inout' parameter 'x2'}}
borrowVal(x2) // expected-note {{captured here}}
consumeVal(x2) // expected-note {{captured here}}
Expand Down Expand Up @@ -1341,12 +1340,10 @@ public func closureAndDeferCaptureClassUseAfterConsume3() {
}

public func closureAndDeferCaptureClassArgUseAfterConsume(_ x2: inout NonTrivialStruct) {
// expected-error @-1 {{'x2' consumed but not reinitialized before end of function}}
// expected-error @-2 {{'x2' consumed in closure but not reinitialized before end of closure}}
// expected-error @-3 {{'x2' consumed more than once}}
// expected-note @-4 {{'x2' is declared 'inout'}}
// expected-error @-1 {{'x2' consumed in closure but not reinitialized before end of closure}}
// expected-error @-2 {{'x2' consumed more than once}}
// expected-note @-3 {{'x2' is declared 'inout'}}
let f = { // expected-error {{escaping closure captures 'inout' parameter 'x2'}}
// expected-note @-1 {{consuming use here}}
defer { // expected-note {{captured indirectly by this call}}
borrowVal(x2) // expected-note {{captured here}}
consumeVal(x2) // expected-note {{captured here}}
Expand Down Expand Up @@ -1391,11 +1388,9 @@ public func closureAndClosureCaptureClassUseAfterConsume2() {


public func closureAndClosureCaptureClassArgUseAfterConsume(_ x2: inout NonTrivialStruct) {
// expected-error @-1 {{'x2' consumed but not reinitialized before end of function}}
// expected-note @-1 {{'x2' is declared 'inout'}}
// expected-note @-2 {{'x2' is declared 'inout'}}
// expected-note @-3 {{'x2' is declared 'inout'}}
let f = { // expected-error {{escaping closure captures 'inout' parameter 'x2'}}
// expected-note @-1 {{consuming use here}}
let g = { // expected-error {{escaping closure captures 'inout' parameter 'x2'}}
// expected-note @-1 {{captured indirectly by this call}}
borrowVal(x2)
Expand Down