Warn on incorrect usage of implicit self in non-escaping closure that captures self weakly

[calda]

As a follow-up to SE-0365, this PR adds a new warning for incorrect usage of implicit self in non-escaping closures that capture self weakly.

In Swift 5 mode, non-escaping closures are incorrectly allowed to use implicit self, even is self is optional:

// Compiles in Swift 5:
doVoidStuffNonEscaping { [weak self] in
  method()
}

We now emit a warning in this case:

doVoidStuffNonEscaping { [weak self] in 
  // warning: use of implicit 'self' is not allowed before 'self' has been unwrapped; this is an error in Swift 6
  // note: unwrap 'self' before using implicit self (fixit: insert 'guard let self else { return }')
  method()
}

In cases where self is unwrapped, the implicit self decl still doesn't actually refer to the unwrapped self decl. In Swift 5.7 this emits a "value 'self' was defined but never used; consider replacing with boolean test" warning, but we now instead emit a tailored warning with a fix-it to use explicit self:

doVoidStuffNonEscaping { [weak self] in 
  // warning: 'self' unwrap condition does not affect uses of implicit self in non-escaping closure, 
  // because implicit self is always non-optional in Swift 5 mode; this is supported in Swift 6 
  // without a warning
  guard let self else { return }
  // note: reference 'self.' explicitly to silence this warning (fixit: insert 'self.')
  method()
  // note: reference 'self.' explicitly to silence this warning (fixit: insert 'self.')
  anotherMethod()
}

Reviewers: @xedin

[xwu]

Sorry, perhaps there has been some miscommunication, or a misunderstanding on my part—thanks for bearing with us.

It was the consensus of the language workgroup that implicit strong self in a non-escaping closure weakly capturing self is a bug, but it must continue to work for Swift 5 language modes for source compatibility reasons. Thus, we envisioned the warning as implemented here outlined in the first scenario. I think all are in agreement that this is the correct approach.

As to the rest, SE-0365 was envisioned for inclusion without a language mode requirement. The language workgroup concluded that having the feature described in the proposal only available for the Swift 6 language mode would be a material change requiring re-review, and it was decided that exploration was warranted whether it is necessary as an implementation matter.

If, as demonstrated here, it is possible to emit a special warning in the second scenario described in this PR, it is also possible to suppress the warning entirely and to make the feature described in SE-0365 available without regard for language mode. That would be what the language workgroup has envisioned in line with its approval.

By contrast, a warning in Swift 5.8 for what SE-0365 described as something that should work would, to my understanding, require re-review. Is there an implementation-level reason why this approach has been chosen?

cc @rjmccall

[calda]

As to the rest, SE-0365 was envisioned for inclusion without a language mode requirement.

I had an implementation for SE-0365 that worked in both Swift 5 mode and Swift 6 mode. This required doing additional lookups in MiscDiagnostics in Swift 5 mode (since the AST is incorrect in that case). @xedin requested that we not perform these additional lookups, and instead only implement SE-0365 in Swift 6 mode: #40702 (comment)

It is technically feasible to implement SE-0365 in Swift 5.8 (there are commits in #40702 that did this), however we would have to make these additional lookups in MiscDiagnostics.

I personally support the idea of landing SE-0365 in Swift 5.8, and would be happy to work on this -- we just need to reach alignment on whether the implementation is acceptable.

[xedin]

The lookups themselves are not a problem, the problem is that they make it seem like compiler picked a completely different declaration for implicit self. If I understand it correctly - we could enable new behavior everywhere except non-escaping closures for which we have to warn about behavior change in Swift 6?

[xedin]

Let me try to re-phrase concisely - we should enable feature with exception to non-escaping closures which would have the behavior gated on Swift 6 mode and emit all the warnings we have discussed in < Swift 6 modes. Does that sound right to you, @xwu?

[xwu]

If there are not implementation barriers, everything described in SE-0365 would be enabled in all language modes without warnings.

The Swift 5 behavior permitting implicit strong self in a non-escaping closure that weakly captures self should be a warning in Swift 5.8 and an error in Swift 6 language mode. The workgroup understands that this may require the compiler to look like it is deliberately picking the wrong self in the Swift 5 language mode, if that is what it takes to preserve source compatibility, since in effect all prior versions of Swift were picking the wrong self when it allowed this source to compile.

[xedin]

I’m still confused what the group wants, somebody else would have to review these changes.

[calda]

Here's a PR that enables SE-0365 in Swift 5 mode as well: #61520. Who is best to review those changes?

[xwu]

@xedin Clearly I haven’t been the best communicator. I’ve tagged a few others from the workgroup over in the new PR; perhaps reviewing the concrete implementation will be useful.

[calda]

Superseded by #61520