Skip to content

Commit 684c908

Browse files
committed
Prevent nested closure in 'extension Optional' from unexpectedly being allowed
1 parent be0e68e commit 684c908

File tree

2 files changed

+81
-5
lines changed

2 files changed

+81
-5
lines changed

lib/Sema/MiscDiagnostics.cpp

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1681,12 +1681,21 @@ static void diagRecursivePropertyAccess(const Expr *E, const DeclContext *DC) {
16811681
}
16821682

16831683
/// The `weak self` capture of this closure if present
1684-
static VarDecl *weakSelfCapture(const AbstractClosureExpr *ACE) {
1684+
static VarDecl *selfCapture(const AbstractClosureExpr *ACE) {
16851685
if (auto closureExpr = dyn_cast<ClosureExpr>(ACE)) {
16861686
if (auto selfDecl = closureExpr->getCapturedSelfDecl()) {
1687-
if (selfDecl->getInterfaceType()->is<WeakStorageType>()) {
1688-
return selfDecl;
1689-
}
1687+
return selfDecl;
1688+
}
1689+
}
1690+
1691+
return nullptr;
1692+
}
1693+
1694+
/// The `weak self` capture of this closure if present
1695+
static VarDecl *weakSelfCapture(const AbstractClosureExpr *ACE) {
1696+
if (auto selfDecl = selfCapture(ACE)) {
1697+
if (selfDecl->getInterfaceType()->is<WeakStorageType>()) {
1698+
return selfDecl;
16901699
}
16911700
}
16921701

@@ -2034,6 +2043,25 @@ class ImplicitSelfUsageChecker : public BaseDiagnosticWalker {
20342043
return nullptr;
20352044
}
20362045

2046+
// If this closure has a non-weak capture, the parent closure
2047+
// has a weak capture, the implicit self decl refers directly
2048+
// to this closure's self capture, and that capture directly
2049+
// refers to the parent closure's weak self capture, then
2050+
// this is invalid.
2051+
// - Normally this is rejected by the type checker because
2052+
// self is optional here, but that doesn't happen when calling
2053+
// a method defined on `Optional<Self>`.
2054+
if (!closureHasWeakSelfCapture(inClosure)) {
2055+
if (auto closureSelfCapture = selfCapture(inClosure)) {
2056+
if (auto parentWeakSelfCapture = weakSelfCapture(outerClosure)) {
2057+
if (selfDecl == closureSelfCapture &&
2058+
parentWeakSelfCapture == outerSelfDecl) {
2059+
return outerClosure;
2060+
}
2061+
}
2062+
}
2063+
}
2064+
20372065
// Check if this closure contains the self decl.
20382066
// - If the self decl is defined in the closure's body, its
20392067
// decl context will be the closure itself.

test/expr/closure/closures_swift6.swift

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -959,14 +959,62 @@ extension TestExtensionOnOptionalSelf? {
959959

960960
_ = { [weak self] in
961961
_ = { [self] in
962-
foo()
962+
foo() // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}}
963963
self.foo()
964964
self?.bar()
965965
}
966966
}
967967
}
968968
}
969969

970+
// non-optional self in this extension, but on a type with members defined on optional self
971+
extension TestExtensionOnOptionalSelf {
972+
func foo() {
973+
_ = { [weak self] in
974+
foo() // expected-error {{implicit use of 'self' in closure; use 'self.' to make capture semantics explicit}}
975+
self.foo()
976+
self?.bar()
977+
}
978+
979+
_ = { // expected-note {{capture 'self' explicitly to enable implicit 'self' in this closure}}
980+
foo() // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note {{reference 'self.' explicitly}}
981+
self.foo()
982+
}
983+
984+
_ = { [weak self] in
985+
_ = {
986+
foo()
987+
self.foo()
988+
}
989+
}
990+
991+
_ = { [weak self] in
992+
_ = { [self] in
993+
foo() // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}}
994+
self.foo()
995+
}
996+
}
997+
998+
_ = { [weak self] in
999+
_ = { [self] in
1000+
_ = { [self] in
1001+
foo() // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}}
1002+
self.foo()
1003+
}
1004+
}
1005+
}
1006+
1007+
_ = { [weak self] in
1008+
doVoidStuffNonEscaping {
1009+
_ = { [self] in
1010+
foo() // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}}
1011+
self.foo()
1012+
}
1013+
}
1014+
}
1015+
}
1016+
}
1017+
9701018
actor TestActor {
9711019
func setUp() {
9721020
doVoidStuff { [weak self] in

0 commit comments

Comments
 (0)