Updated logic for diagnosting implicit self capture to handle immutable weak capture (minor source-breaking change)

Author: nickolas-pohilets

include/swift/AST/Stmt.h

@@ -729,8 +729,7 @@ class alignas(1 << PatternAlignInBits) StmtConditionElement {
   ///    RHS of the self condition references a var defined in a capture list.
   ///  - If `requireLoadExpr` is `true`, additionally requires that the RHS of
   ///    the self condition is a `LoadExpr`.
-  bool rebindsSelf(ASTContext &Ctx, bool requiresCaptureListRef = false,
-                   bool requireLoadExpr = false) const;
+  bool rebindsSelf(ASTContext &Ctx, bool requiresCaptureListRef = false) const;
 
   SourceLoc getStartLoc() const;
   SourceLoc getEndLoc() const;
@@ -841,8 +840,7 @@ class LabeledConditionalStmt : public LabeledStmt {
   ///    RHS of the self condition references a var defined in a capture list.
   ///  - If `requireLoadExpr` is `true`, additionally requires that the RHS of
   ///    the self condition is a `LoadExpr`.
-  bool rebindsSelf(ASTContext &Ctx, bool requiresCaptureListRef = false,
-                   bool requireLoadExpr = false) const;
+  bool rebindsSelf(ASTContext &Ctx, bool requiresCaptureListRef = false) const;
 
   static bool classof(const Stmt *S) {
     return S->getKind() >= StmtKind::First_LabeledConditionalStmt &&

lib/AST/Stmt.cpp

@@ -517,11 +517,9 @@ void LabeledConditionalStmt::setCond(StmtCondition e) {
 ///  - If `requireLoadExpr` is `true`, additionally requires that the RHS of
 ///    the self condition is a `LoadExpr`.
 bool LabeledConditionalStmt::rebindsSelf(ASTContext &Ctx,
-                                         bool requiresCaptureListRef,
-                                         bool requireLoadExpr) const {
-  return llvm::any_of(getCond(), [&Ctx, requiresCaptureListRef,
-                                  requireLoadExpr](const auto &cond) {
-    return cond.rebindsSelf(Ctx, requiresCaptureListRef, requireLoadExpr);
+                                         bool requiresCaptureListRef) const {
+  return llvm::any_of(getCond(), [&Ctx, requiresCaptureListRef](const auto &cond) {
+    return cond.rebindsSelf(Ctx, requiresCaptureListRef);
   });
 }
 
@@ -532,8 +530,7 @@ bool LabeledConditionalStmt::rebindsSelf(ASTContext &Ctx,
 ///  - If `requireLoadExpr` is `true`, additionally requires that the RHS of
 ///    the self condition is a `LoadExpr`.
 bool StmtConditionElement::rebindsSelf(ASTContext &Ctx,
-                                       bool requiresCaptureListRef,
-                                       bool requireLoadExpr) const {
+                                       bool requiresCaptureListRef) const {
   auto pattern = getPatternOrNull();
   if (!pattern) {
     return false;
@@ -561,10 +558,6 @@ bool StmtConditionElement::rebindsSelf(ASTContext &Ctx,
     return false;
   }
 
-  if (requireLoadExpr && !isa<LoadExpr>(exprToCheckForDRE)) {
-    return false;
-  }
-
   if (auto *load = dyn_cast<LoadExpr>(exprToCheckForDRE)) {
     exprToCheckForDRE = load->getSubExpr();
   }

lib/Sema/MiscDiagnostics.cpp

@@ -1755,8 +1755,7 @@ class ImplicitSelfUsageChecker : public BaseDiagnosticWalker {
     //   guard let self = self else { return }
     //   method() // <- implicit self is not allowed
     //
-    return conditionalStmt->rebindsSelf(Ctx, /*requiresCaptureListRef*/ false,
-                                        /*requireLoadExpr*/ true);
+    return conditionalStmt->rebindsSelf(Ctx, /*requiresCaptureListRef*/ true);
   }
 
   static bool

test/expr/closure/closures.swift

@@ -1604,9 +1604,7 @@ final class AutoclosureTests {
       let someOptional: Self? = Self()
       var `self` = self ?? someOptional // expected-warning {{'self' was never mutated; consider changing to 'let' constant}}
       guard let self = self else { return }
-      // This is not supposed to be permitted, but has been allowed since Swift 5.8,
-      // so we have to continue allowing it to maintain source compatibility.
-      method()
+      method() // expected-error{{call to method 'method' in closure requires explicit use of 'self' to make capture semantics explicit}}
     }
 
     doVoidStuff { // expected-note {{capture 'self' explicitly to enable implicit 'self' in this closure}}