Merge pull request #40702 from calda/cal--implicit-weak-self

[SE-0365] Allow implicit self for `weak self` captures

Author: xedin

CHANGELOG.md

@@ -3,6 +3,48 @@ CHANGELOG
 
 _**Note:** This is in reverse chronological order, so newer entries are added to the top._
 
+## Swift 6.0
+
+* [SE-0365][]:
+ 
+  Implicit `self` is now permitted for `weak self` captures, after `self` is unwrapped.
+
+  For example, the usage of implicit `self` below is now permitted:
+
+  ```swift
+  class ViewController {
+    let button: Button
+
+    func setup() {
+        button.tapHandler = { [weak self] in
+            guard let self else { return }
+            dismiss() // refers to `self.dismiss()`
+        }
+    }
+
+    func dismiss() { ... }
+  }
+  ```
+
+  In Swift 5 language modes, implicit `self` is permitted for `weak self` captures in _non-escaping_ closures even before `self` is unwrapped. For example, this code compiles successfully in Swift 5 language mode:
+
+  ```swift
+  class ExampleClass {
+    func makeArray() -> [String] {
+      // `Array.map` takes a non-escaping closure:
+      ["foo", "bar", "baaz"].map { [weak self] string in
+        double(string) // implicitly refers to `self!.double(string)`
+      }
+    } 
+
+    func double(_ string: String) -> String {
+      string + string
+    }
+  }
+  ```
+
+  In Swift 6, the above code will no longer compile. `weak self` captures in non-escaping closures now have the same behavior as captures in escaping closures (as described in [SE-0365][]). Code relying on the previous behavior will need to be updated to either unwrap `self` (e.g. by adding a `guard let self else return` statement), or to use a different capture method (e.g. using `[self]` or `[unowned self]` instead of `[weak self]`).
+
 ## Swift 5.8
 
 * [SE-0362][]:
@@ -9559,6 +9601,7 @@ Swift 1.0
 [SE-0357]: <https://github.com/apple/swift-evolution/blob/main/proposals/0357-regex-string-processing-algorithms.md>
 [SE-0358]: <https://github.com/apple/swift-evolution/blob/main/proposals/0358-primary-associated-types-in-stdlib.md>
 [SE-0362]: <https://github.com/apple/swift-evolution/blob/main/proposals/0362-piecemeal-future-features.md>
+[SE-0365]: <https://github.com/apple/swift-evolution/blob/main/proposals/0365-implicit-self-weak-capture.md>
 
 [SR-75]: <https://bugs.swift.org/browse/SR-75>
 [SR-106]: <https://bugs.swift.org/browse/SR-106>

include/swift/AST/DiagnosticsSema.def

@@ -1204,6 +1204,12 @@ NOTE(optional_key_path_root_base_chain, none,
 NOTE(optional_key_path_root_base_unwrap, none,
      "unwrap the optional using '!.' to access unwrapped type member %0",
      (DeclNameRef))
+
+ERROR(optional_self_not_unwrapped,none,
+      "explicit use of 'self' is required when 'self' is optional, "
+      "to make control flow explicit", ())
+NOTE(optional_self_chain,none,
+     "reference 'self?.' explicitly", ())
 
 ERROR(missing_unwrap_optional_try,none,
       "value of optional type %0 not unwrapped; did you mean to use 'try!' "
@@ -3939,8 +3945,6 @@ NOTE(note_reference_self_explicitly,none,
 NOTE(note_other_self_capture,none,
       "variable other than 'self' captured here under the name 'self' does not "
       "enable implicit 'self'", ())
-NOTE(note_self_captured_weakly,none,
-      "weak capture of 'self' here does not enable implicit 'self'", ())
 ERROR(implicit_use_of_self_in_closure,none,
       "implicit use of 'self' in closure; use 'self.' to make"
       " capture semantics explicit", ())

include/swift/AST/NameLookup.h

@@ -107,15 +107,20 @@ struct LookupResultEntry {
   /// extension (if it found something at that level).
   DeclContext *BaseDC;
 
+  /// The declaration that defines the base of the call to `Value`.
+  /// This is always available, as long as `BaseDC` is not null.
+  ValueDecl *BaseDecl;
+
   /// The declaration corresponds to the given name; i.e. the decl we are
   /// looking up.
   ValueDecl *Value;
 
 public:
-  LookupResultEntry(ValueDecl *value) : BaseDC(nullptr), Value(value) {}
+  LookupResultEntry(ValueDecl *value)
+      : BaseDC(nullptr), BaseDecl(nullptr), Value(value) {}
 
-  LookupResultEntry(DeclContext *baseDC, ValueDecl *value)
-    : BaseDC(baseDC), Value(value) {}
+  LookupResultEntry(DeclContext *baseDC, ValueDecl *baseDecl, ValueDecl *value)
+      : BaseDC(baseDC), BaseDecl(baseDecl), Value(value) {}
 
   ValueDecl *getValueDecl() const { return Value; }
 

include/swift/AST/Stmt.h

@@ -635,7 +635,7 @@ class DoStmt : public LabeledStmt {
 };
 
 /// Either an "if let" case or a simple boolean expression can appear as the
-/// condition of an 'if' or 'while' statement.
+/// condition of an 'if', 'guard', or 'while' statement.
 using StmtCondition = MutableArrayRef<StmtConditionElement>;
 
 /// This is the common base class between statements that can have labels, and

lib/AST/NameLookup.cpp

@@ -57,25 +57,7 @@ ValueDecl *LookupResultEntry::getBaseDecl() const {
   if (BaseDC == nullptr)
     return nullptr;
 
-  if (auto *AFD = dyn_cast<AbstractFunctionDecl>(BaseDC))
-    return AFD->getImplicitSelfDecl();
-
-  if (auto *PBI = dyn_cast<PatternBindingInitializer>(BaseDC)) {
-    auto *selfDecl = PBI->getImplicitSelfDecl();
-    assert(selfDecl);
-    return selfDecl;
-  }
-
-  if (auto *CE = dyn_cast<ClosureExpr>(BaseDC)) {
-    auto *selfDecl = CE->getCapturedSelfDecl();
-    assert(selfDecl);
-    assert(selfDecl->isSelfParamCapture());
-    return selfDecl;
-  }
-
-  auto *nominalDecl = BaseDC->getSelfNominalTypeDecl();
-  assert(nominalDecl);
-  return nominalDecl;
+  return BaseDecl;
 }
 
 void LookupResult::filter(

lib/AST/UnqualifiedLookup.cpp

@@ -19,6 +19,7 @@
 #include "swift/AST/ASTVisitor.h"
 #include "swift/AST/DebuggerClient.h"
 #include "swift/AST/ImportCache.h"
+#include "swift/AST/Initializer.h"
 #include "swift/AST/ModuleNameLookup.h"
 #include "swift/AST/NameLookup.h"
 #include "swift/AST/NameLookupRequests.h"
@@ -76,6 +77,8 @@ namespace {
 
     private:
       SelfBounds findSelfBounds(const DeclContext *dc);
+      ValueDecl *lookupBaseDecl(const DeclContext *baseDC) const;
+      ValueDecl *getBaseDeclForResult(const DeclContext *baseDC) const;
 
       // Classify this declaration.
       // Types are formally members of the metatype.
@@ -343,14 +346,84 @@ void UnqualifiedLookupFactory::ResultFinderForTypeContext::findResults(
   SmallVector<ValueDecl *, 4> Lookup;
   contextForLookup->lookupQualified(selfBounds, Name, baseNLOptions, Lookup);
   for (auto Result : Lookup) {
-    results.emplace_back(const_cast<DeclContext *>(whereValueIsMember(Result)),
-                         Result);
+    auto baseDC = const_cast<DeclContext *>(whereValueIsMember(Result));
+    auto baseDecl = getBaseDeclForResult(baseDC);
+    results.emplace_back(baseDC, baseDecl, Result);
 #ifndef NDEBUG
     factory->addedResult(results.back());
 #endif
   }
 }
 
+ValueDecl *
+UnqualifiedLookupFactory::ResultFinderForTypeContext::getBaseDeclForResult(
+    const DeclContext *baseDC) const {
+  if (baseDC == nullptr) {
+    return nullptr;
+  }
+
+  if (auto localBaseDecl = lookupBaseDecl(baseDC)) {
+    return localBaseDecl;
+  }
+
+  if (auto *AFD = dyn_cast<AbstractFunctionDecl>(baseDC)) {
+    return const_cast<ParamDecl *>(AFD->getImplicitSelfDecl());
+  }
+
+  if (auto *PBI = dyn_cast<PatternBindingInitializer>(baseDC)) {
+    auto *selfDecl = PBI->getImplicitSelfDecl();
+    assert(selfDecl);
+    return selfDecl;
+  }
+
+  else if (auto *CE = dyn_cast<ClosureExpr>(baseDC)) {
+    auto *selfDecl = CE->getCapturedSelfDecl();
+    assert(selfDecl);
+    assert(selfDecl->isSelfParamCapture());
+    return selfDecl;
+  }
+
+  auto *nominalDecl = baseDC->getSelfNominalTypeDecl();
+  assert(nominalDecl);
+  return nominalDecl;
+}
+
+ValueDecl *UnqualifiedLookupFactory::ResultFinderForTypeContext::lookupBaseDecl(
+    const DeclContext *baseDC) const {
+  // Perform an unqualified lookup for the base decl of this result. This
+  // handles cases where self was rebound (e.g. `guard let self = self`)
+  // earlier in the scope.
+  //
+  // Only do this in closures that capture self weakly, since implicit self
+  // isn't allowed to be rebound in other contexts. In other contexts, implicit
+  // self _always_ refers to the context's self `ParamDecl`, even if there
+  // is another local decl with the name `self` that would be found by
+  // `lookupSingleLocalDecl`.
+  bool isInWeakSelfClosure = false;
+  if (auto closureExpr = dyn_cast<ClosureExpr>(factory->DC)) {
+    if (auto decl = closureExpr->getCapturedSelfDecl()) {
+      if (auto a = decl->getAttrs().getAttribute<ReferenceOwnershipAttr>()) {
+        isInWeakSelfClosure = a->get() == ReferenceOwnership::Weak;
+      }
+    }
+  }
+
+  // We can only change the behavior of lookup in Swift 6 and later,
+  // due to a bug in Swift 5 where implicit self is always allowed
+  // for weak self captures in non-escaping closures.
+  if (!factory->Ctx.LangOpts.isSwiftVersionAtLeast(6)) {
+    return nullptr;
+  }
+
+  if (isInWeakSelfClosure) {
+    return ASTScope::lookupSingleLocalDecl(factory->DC->getParentSourceFile(),
+                                           DeclName(factory->Ctx.Id_self),
+                                           factory->Loc);
+  }
+
+  return nullptr;
+}
+
 // TODO (someday): Instead of adding unavailable entries to Results,
 // then later shunting them aside, just put them in the right place
 // to begin with.

lib/Sema/CSDiagnostics.cpp

@@ -1371,6 +1371,22 @@ bool MemberAccessOnOptionalBaseFailure::diagnoseAsError() {
           .fixItInsert(sourceRange.End, "!.");
     }
   } else {
+    // Check whether or not the base of this optional unwrap is implicit self
+    // This can only happen with a [weak self] capture, and is not permitted.
+    if (auto dotExpr = getAsExpr<UnresolvedDotExpr>(locator->getAnchor())) {
+      if (auto baseDeclRef = dyn_cast<DeclRefExpr>(dotExpr->getBase())) {
+        ASTContext &Ctx = baseDeclRef->getDecl()->getASTContext();
+        if (baseDeclRef->isImplicit() &&
+            baseDeclRef->getDecl()->getName().isSimpleName(Ctx.Id_self)) {
+          emitDiagnostic(diag::optional_self_not_unwrapped);
+
+          emitDiagnostic(diag::optional_self_chain)
+              .fixItInsertAfter(sourceRange.End, "self?.");
+          return true;
+        }
+      }
+    }
+    
     emitDiagnostic(diag::optional_base_not_unwrapped, baseType, Member,
                    unwrappedBaseType);