Fix issue where implicit self was unexpectedly not allowed in nested weak self closure in Swift 6 mode #78738

calda · 2025-01-19T02:27:13Z

This PR fixes an issue that @harlanhaskins reported where implicit self is unexpectedly rejected in Swift 6 mode:

func acceptsClosure(_ x: @escaping () -> Void) {}

@MainActor
final class A {
    func nonisolatedMethod() {}

    func createsNestedClosure() {
        acceptsClosure { [weak self] in
            Task { @MainActor [weak self] in
                guard let self else { return }
                nonisolatedMethod() // error: implicit use of 'self' in closure; use 'self.' to make capture semantics explicit
            }
        }
    }
}

This is allowed in Swift 5 mode and should continue being supported.

Please review: @hamishknight @xedin @Jumhyn

…weak self closure in Swift 6 mode

calda · 2025-01-19T02:37:41Z

test/expr/closure/closures_swift6.swift

+      doVoidStuff { [weak self] in
+        doVoidStuff { [self] in
+          guard let self else { return }
+          self.x += 1
+        }
+      }


This looks funny but has been allowed since at least Swift 4.2: https://swiftfiddle.com/irzj67qcezajlj7u5hyq4te2fq

func withEscapingClosure(_ closure: @escaping () -> Void) { closure() } class Test { var x = 0 func test() { withEscapingClosure { [weak self] in withEscapingClosure { [self] in self?.x += 1 guard let self = self else { return } self.x += 1 } } } }

calda · 2025-01-19T02:39:04Z

test/expr/closure/closures_swift6.swift

@@ -670,7 +756,7 @@ final class AutoclosureTests {
    doVoidStuff { [weak self] in
      doVoidStuff { [self] in
        guard let self else { return }
-        method() // expected-error {{call to method 'method' in closure requires explicit use of 'self' to make capture semantics explicit}}


The explicit self version of this has been supported since at least Swift 4.2 so it seems perfectly fine for the implicit self version to be supported

Yeah this seems reasonable to me, this is the case that came up last time

hamishknight · 2025-01-23T11:50:12Z

test/expr/closure/closures_swift6.swift

@@ -670,7 +756,7 @@ final class AutoclosureTests {
    doVoidStuff { [weak self] in
      doVoidStuff { [self] in
        guard let self else { return }
-        method() // expected-error {{call to method 'method' in closure requires explicit use of 'self' to make capture semantics explicit}}


Yeah this seems reasonable to me, this is the case that came up last time

lib/Sema/MiscDiagnostics.cpp

…ftlang/swift into cal--fix-nested-weak-self-issue Tag build swift-DEVELOPMENT-SNAPSHOT-2025-01-31-a

calda · 2025-02-03T03:10:31Z

test/expr/closure/closures_swift6.swift

+actor TestActor {
+    func setUp() {
+        doVoidStuff { [weak self] in
+            Task { [weak self] in
+                guard let self else { return }
+                await test()
+            }
+        }
+    }
+
+    @MainActor
+    func test() { }
+}


Here's a test case from another issue that was filed recently about this same bug: #79014

…g allowed

hamishknight

Thanks for all the additional tests!

lib/Sema/MiscDiagnostics.cpp

test/expr/closure/closures_swift6.swift

lib/Sema/MiscDiagnostics.cpp

test/expr/closure/closures_swift6.swift

calda

Updated @hamishknight! -- simplified the code like you suggested while also making sure to reject these other cases we were talking about.

hamishknight

Thanks! I think we can further simplify the logic here, but otherwise this is looking good!

lib/Sema/MiscDiagnostics.cpp

test/expr/closure/closures_swift6.swift

lib/Sema/MiscDiagnostics.cpp

…ftlang/swift into cal--fix-nested-weak-self-issue Tag build swift-DEVELOPMENT-SNAPSHOT-2025-02-06-a

calda · 2025-02-10T16:14:30Z

Ok should be good now @hamishknight, thanks for the great suggestions as always!

hamishknight

Very nice, thanks!

hamishknight · 2025-02-10T16:52:56Z

@swift-ci please test

hamishknight · 2025-02-10T17:07:22Z

@swift-ci please test source compatibility

Fix issue where implicit self was unexpectedly not allowed in nested …

e6a2230

…weak self closure in Swift 6 mode

calda requested review from hborla, slavapestov and xedin as code owners January 19, 2025 02:27

Merge branch 'main' into cal--fix-nested-weak-self-issue

8557e23

calda commented Jan 19, 2025

View reviewed changes

hamishknight self-requested a review January 19, 2025 11:22

hamishknight reviewed Jan 23, 2025

View reviewed changes

Merge tag 'swift-DEVELOPMENT-SNAPSHOT-2025-01-31-a' of github.com:swi…

f2cda5e

…ftlang/swift into cal--fix-nested-weak-self-issue Tag build swift-DEVELOPMENT-SNAPSHOT-2025-01-31-a

calda force-pushed the cal--fix-nested-weak-self-issue branch 3 times, most recently from 02d0e9b to f55b571 Compare February 3, 2025 03:09

calda commented Feb 3, 2025

View reviewed changes

calda force-pushed the cal--fix-nested-weak-self-issue branch 2 times, most recently from 3708ef0 to b648b0c Compare February 3, 2025 03:28

Add more test cases, simplify logic

be0e68e

calda force-pushed the cal--fix-nested-weak-self-issue branch from b648b0c to be0e68e Compare February 3, 2025 03:28

Prevent nested closure in 'extension Optional' from unexpectedly bein…

684c908

…g allowed

hamishknight reviewed Feb 5, 2025

View reviewed changes

Simplify diagnostics

041c685

calda commented Feb 6, 2025

View reviewed changes

hamishknight reviewed Feb 10, 2025

View reviewed changes

lib/Sema/MiscDiagnostics.cpp Outdated Show resolved Hide resolved

test/expr/closure/closures_swift6.swift Outdated Show resolved Hide resolved

lib/Sema/MiscDiagnostics.cpp Outdated Show resolved Hide resolved

calda added 3 commits February 10, 2025 06:29

Further simplify conditions

31528da

Merge tag 'swift-DEVELOPMENT-SNAPSHOT-2025-02-06-a' of github.com:swi…

64fc43e

…ftlang/swift into cal--fix-nested-weak-self-issue Tag build swift-DEVELOPMENT-SNAPSHOT-2025-02-06-a

Update tests after pulling fix from master

14632b7

calda force-pushed the cal--fix-nested-weak-self-issue branch from b8e9c23 to 14632b7 Compare February 10, 2025 16:13

hamishknight approved these changes Feb 10, 2025

View reviewed changes

hamishknight merged commit f828882 into swiftlang:main Feb 11, 2025
7 checks passed

calda mentioned this pull request Feb 11, 2025

[🍒] [6.1] Fix issue where implicit self was unexpectedly not allowed in nested weak self closure in Swift 6 mode #79295

Closed

Fix issue where implicit self was unexpectedly not allowed in nested weak self closure in Swift 6 mode #78738

Fix issue where implicit self was unexpectedly not allowed in nested weak self closure in Swift 6 mode #78738

Uh oh!

Conversation

calda commented Jan 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

calda Jan 19, 2025

Choose a reason for hiding this comment

Uh oh!

calda Jan 19, 2025

Choose a reason for hiding this comment

Uh oh!

hamishknight Jan 23, 2025

Choose a reason for hiding this comment

Uh oh!

hamishknight Jan 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

calda Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hamishknight left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

calda left a comment

Choose a reason for hiding this comment

Uh oh!

hamishknight left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

calda commented Feb 10, 2025

Uh oh!

hamishknight left a comment

Choose a reason for hiding this comment

Uh oh!

hamishknight commented Feb 10, 2025

Uh oh!

hamishknight commented Feb 10, 2025

Uh oh!

Uh oh!

Uh oh!

calda commented Jan 19, 2025 •

edited

Loading

calda Feb 3, 2025 •

edited

Loading