Skip to content
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.

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

[SR-10218] Explicit self capture in closure capture list #52618

Closed
swift-ci opened this issue Mar 28, 2019 · 8 comments
Closed

[SR-10218] Explicit self capture in closure capture list #52618

swift-ci opened this issue Mar 28, 2019 · 8 comments

Comments

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Mar 28, 2019

Previous ID SR-10218
Radar rdar://problem/17028716
Original Reporter paiv (JIRA User)
Type Bug
Status Resolved
Resolution Done
Environment

Apple Swift version 5.0 (swiftlang-1001.0.69.5 clang-1001.0.46.3)
Target: x86_64-apple-darwin18.5.0

Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, StarterBug, StarterProposal
Assignee jumhyn (JIRA)
Priority Medium

md5: 870ff38e542c2501df92775e3c63ff22

Issue Description:

In this code:

class A {
    init() {
        some(f: { [self] in
            foo()
        })
    }
    func some(f: @escaping () -> Void) {}
    func foo() {}
}

The `self` capturing intention is explicitly specified in the capture list, but compiler still emits errors:

main.swift:4:13: error: call to method 'foo' in closure requires explicit 'self.' to make capture semantics explicit
            foo()
            ^
            self.
main.swift:3:20: warning: capture 'self' was never used
        some(f: { [self] in
                   ^

There should be no errors, nor warnings in that code.

Discussion: https://forums.swift.org/t/review-capture-semantics-of-self/22017

@belkadan
Copy link
Contributor

@belkadan belkadan commented Mar 28, 2019

I think this is reasonable to take as a simple enhancement, but a core team person should confirm because it's on the boundary of being a language change. @jckarter, @lattner?

Should also apply when using [unowned self].

@jckarter
Copy link
Member

@jckarter jckarter commented Mar 28, 2019

It makes sense to me, since the whole point of requiring `self.` was to make the capture semantics explicit, and using the capture list also achieves this. This does strike me as something that deserves going through the evolution process, though.

@belkadan
Copy link
Contributor

@belkadan belkadan commented Mar 28, 2019

Okay, tagged as StarterProposal…and also StarterBug, because I think this could be implemented by changing isClosureRequiringSelfQualification in MiscDiagnostics.cpp to check for a capture of self.

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Mar 28, 2019

Comment by Frederick Kellison-Linn (JIRA)

I've started looking at this so I can write up a proposal in the next couple days.

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Apr 3, 2019

Comment by Adam Shin (JIRA)

With this change, should there also be a new fix-it for inserting `self` into the closure capture list?

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Apr 3, 2019

Comment by Frederick Kellison-Linn (JIRA)

I was planning to investigate that, yeah. It’s a little hairy since you have to handle the case where there’s a capture list already, the case where there’s no capture list or params (so you have to insert `in`) and the case where there’s no capture list but there are params, and the fix-it applied in each case is different. I’ll make sure to include a section on this when I put the pitch up.

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Apr 3, 2019

Comment by Frederick Kellison-Linn (JIRA)

(Also, in the case where there’s a capture list already, we would want to avoid offering the fix-it altogether if there’s already a variable captured under the name “self”)

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Jan 17, 2020

Comment by Frederick Kellison-Linn (JIRA)

Resolved by PR #23934

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants