Skip to content

Commit 1711df4

Browse files
committed
Fix the condition for warning about implicit capture of self captures.
We've always emitted an error if we saw an implicit use of a self parameter of class type from an escaping closure. In PR #35898, I fixed this to also emit an error if the reference was to an explicit capture of self that wasn't made in the current closure. That was causing some source incompatibilities that we decided were too severe, so in PR #38947 I weakened that to a warning when the diagnostic walk was within multiple levels of closures, because I have always thought of this as a fix to nested closures. However, this was the wrong condition in two ways. First, the diagnostic walk does not always start from the outermost function declaration; it can also start from a multi-statement closure. In that case, we'll still end up emitting an error when we see uses of explicit captures from the closure when we walk it, and so we still have a source incompatibility. That is rdar://82545600. Second, the old diagnostic did actually fire correctly in nested closures as long as the code was directly referring to the original self parameter and not any intervening captures. Therefore, #38947 actually turned some things into warnings that had always been errors. The fix is to produce a warning exactly when the referenced declaration was an explicit capture.
1 parent 6e35487 commit 1711df4

File tree

3 files changed

+32
-8
lines changed

3 files changed

+32
-8
lines changed

lib/Sema/MiscDiagnostics.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1615,6 +1615,15 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E,
16151615
// Diagnostics should correct the innermost closure
16161616
auto *ACE = Closures[Closures.size() - 1];
16171617
assert(ACE);
1618+
1619+
// Until Swift 6, only emit a warning when we get this with an
1620+
// explicit capture, since we used to not diagnose this at all.
1621+
auto shouldOnlyWarn = [&](Expr *selfRef) {
1622+
// We know that isImplicitSelfParamUseLikelyToCauseCycle is true,
1623+
// which means all these casts are valid.
1624+
return !cast<VarDecl>(cast<DeclRefExpr>(selfRef)->getDecl())
1625+
->isSelfParameter();
1626+
};
16181627

16191628
SourceLoc memberLoc = SourceLoc();
16201629
if (auto *MRE = dyn_cast<MemberRefExpr>(E))
@@ -1624,7 +1633,7 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E,
16241633
Diags.diagnose(memberLoc,
16251634
diag::property_use_in_closure_without_explicit_self,
16261635
baseName.getIdentifier())
1627-
.warnUntilSwiftVersionIf(Closures.size() > 1, 6);
1636+
.warnUntilSwiftVersionIf(shouldOnlyWarn(MRE->getBase()), 6);
16281637
}
16291638

16301639
// Handle method calls with a specific diagnostic + fixit.
@@ -1636,7 +1645,7 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E,
16361645
Diags.diagnose(DSCE->getLoc(),
16371646
diag::method_call_in_closure_without_explicit_self,
16381647
MethodExpr->getDecl()->getBaseIdentifier())
1639-
.warnUntilSwiftVersionIf(Closures.size() > 1, 6);
1648+
.warnUntilSwiftVersionIf(shouldOnlyWarn(DSCE->getBase()), 6);
16401649
}
16411650

16421651
if (memberLoc.isValid()) {
@@ -1647,7 +1656,7 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E,
16471656
// Catch any other implicit uses of self with a generic diagnostic.
16481657
if (isImplicitSelfParamUseLikelyToCauseCycle(E, ACE))
16491658
Diags.diagnose(E->getLoc(), diag::implicit_use_of_self_in_closure)
1650-
.warnUntilSwiftVersionIf(Closures.size() > 1, 6);
1659+
.warnUntilSwiftVersionIf(shouldOnlyWarn(E), 6);
16511660

16521661
return { true, E };
16531662
}

test/attr/attr_noescape.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,25 +91,25 @@ class SomeClass {
9191

9292
let _: () -> Void = { // expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{26-26= [self] in}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{26-26= [self] in}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{26-26= [self] in}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{26-26= [self] in}}
9393
func inner() { foo() } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{reference 'self.' explicitly}} {{22-22=self.}}
94-
let inner2 = { foo() } // expected-warning {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{21-21= [self] in}} expected-note{{reference 'self.' explicitly}} {{22-22=self.}}
94+
let inner2 = { foo() } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{21-21= [self] in}} expected-note{{reference 'self.' explicitly}} {{22-22=self.}}
9595
let _ = inner2
9696
func multi() -> Int { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{reference 'self.' explicitly}} {{29-29=self.}}
9797
let multi2: () -> Int = { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{32-32= [self] in}} expected-note{{reference 'self.' explicitly}} {{33-33=self.}}
9898
let _ = multi2
99-
doesEscape { foo() } // expected-warning {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{19-19= [self] in}} expected-note{{reference 'self.' explicitly}} {{20-20=self.}}
99+
doesEscape { foo() } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{19-19= [self] in}} expected-note{{reference 'self.' explicitly}} {{20-20=self.}}
100100
takesNoEscapeClosure { foo() } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{reference 'self.' explicitly}} {{30-30=self.}}
101101
doesEscape { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{19-19= [self] in}} expected-note{{reference 'self.' explicitly}} {{20-20=self.}}
102102
takesNoEscapeClosure { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{reference 'self.' explicitly}} {{30-30=self.}}
103103
}
104104

105105
doesEscape { //expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{17-17= [self] in}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{17-17= [self] in}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{17-17= [self] in}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{17-17= [self] in}}
106106
func inner() { foo() } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{reference 'self.' explicitly}} {{22-22=self.}}
107-
let inner2 = { foo() } // expected-warning {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{21-21= [self] in}} expected-note{{reference 'self.' explicitly}} {{22-22=self.}}
107+
let inner2 = { foo() } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{21-21= [self] in}} expected-note{{reference 'self.' explicitly}} {{22-22=self.}}
108108
_ = inner2
109109
func multi() -> Int { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{reference 'self.' explicitly}} {{29-29=self.}}
110110
let multi2: () -> Int = { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{32-32= [self] in}} expected-note{{reference 'self.' explicitly}} {{33-33=self.}}
111111
_ = multi2
112-
doesEscape { foo() } // expected-warning {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{19-19= [self] in}} expected-note{{reference 'self.' explicitly}} {{20-20=self.}}
112+
doesEscape { foo() } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{19-19= [self] in}} expected-note{{reference 'self.' explicitly}} {{20-20=self.}}
113113
takesNoEscapeClosure { foo() } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{reference 'self.' explicitly}} {{30-30=self.}}
114114
doesEscape { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{19-19= [self] in}} expected-note{{reference 'self.' explicitly}} {{20-20=self.}}
115115
takesNoEscapeClosure { foo(); return 0 } // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{reference 'self.' explicitly}} {{30-30=self.}}

test/expr/closure/closures.swift

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ class ExplicitSelfRequiredTest {
172172
doStuff({ [unowned(unsafe) self] in x+1 })
173173
doStuff({ [unowned self = self] in x+1 })
174174
doStuff({ x+1 }) // expected-error {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{14-14= [self] in}} expected-note{{reference 'self.' explicitly}} {{15-15=self.}}
175-
doVoidStuff({ doStuff({ x+1 })}) // expected-warning {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{28-28= [self] in}} expected-note{{reference 'self.' explicitly}} {{29-29=self.}}
175+
doVoidStuff({ doStuff({ x+1 })}) // expected-error {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{28-28= [self] in}} expected-note{{reference 'self.' explicitly}} {{29-29=self.}}
176176
doVoidStuff({ x += 1 }) // expected-error {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{18-18= [self] in}} expected-note{{reference 'self.' explicitly}} {{19-19=self.}}
177177
doVoidStuff({ _ = "\(x)"}) // expected-error {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{18-18= [self] in}} expected-note{{reference 'self.' explicitly}} {{26-26=self.}}
178178
doVoidStuff({ [y = self] in x += 1 }) // expected-warning {{capture 'y' was never used}} expected-error {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{20-20=self, }} expected-note{{reference 'self.' explicitly}} {{33-33=self.}}
@@ -694,3 +694,18 @@ func testSR13239_Variadic_Twos() -> Int {
694694
print("hello")
695695
})
696696
}
697+
698+
// rdar://82545600: this should just be a warning until Swift 6
699+
public class TestImplicitCaptureOfExplicitCaptureOfSelfInEscapingClosure {
700+
var property = false
701+
702+
private init() {
703+
doVoidStuff { [unowned self] in
704+
doVoidStuff {}
705+
doVoidStuff { // expected-note {{capture 'self' explicitly to enable implicit 'self' in this closure}}
706+
doVoidStuff {}
707+
property = false // expected-warning {{reference to property 'property' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note {{reference 'self.' explicitly}}
708+
}
709+
}
710+
}
711+
}

0 commit comments

Comments
 (0)