Skip to content

Fix the condition for warning about implicit capture of self captures. #39118

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

Merged

Conversation

rjmccall
Copy link
Contributor

@rjmccall rjmccall commented Sep 1, 2021

We've always emitted an error if we saw an implicit use of a self parameter of class type from an escaping closure. In #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.

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 swiftlang#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 swiftlang#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, swiftlang#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.
@rjmccall
Copy link
Contributor Author

rjmccall commented Sep 1, 2021

@swift-ci Please test and merge

@rjmccall
Copy link
Contributor Author

rjmccall commented Sep 1, 2021

@swift-ci Please test macOS

@rjmccall rjmccall merged commit a00ed9f into swiftlang:main Sep 2, 2021
@rjmccall rjmccall deleted the implicit-self-capture-capture-warning branch September 2, 2021 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant