Merge pull request #80440 from nickolas-pohilets/mpokhylets/weak-let

Author: nickolas-pohilets

include/swift/AST/Stmt.h

@@ -727,10 +727,7 @@ class alignas(1 << PatternAlignInBits) StmtConditionElement {
   /// or `let self = self` condition.
   ///  - If `requiresCaptureListRef` is `true`, additionally requires that the
   ///    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;
@@ -839,10 +836,7 @@ class LabeledConditionalStmt : public LabeledStmt {
   /// or `let self = self` condition.
   ///  - If `requiresCaptureListRef` is `true`, additionally requires that the
   ///    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/Expr.cpp

@@ -1366,11 +1366,8 @@ CaptureListEntry CaptureListEntry::createParsed(
     SourceRange ownershipRange, Identifier name, SourceLoc nameLoc,
     SourceLoc equalLoc, Expr *initializer, DeclContext *DC) {
 
-  auto introducer =
-      (ownershipKind != ReferenceOwnership::Weak ? VarDecl::Introducer::Let
-                                                 : VarDecl::Introducer::Var);
   auto *VD =
-      new (Ctx) VarDecl(/*isStatic==*/false, introducer, nameLoc, name, DC);
+      new (Ctx) VarDecl(/*isStatic==*/false, VarDecl::Introducer::Let, nameLoc, name, DC);
 
   if (ownershipKind != ReferenceOwnership::Strong)
     VD->getAttrs().add(

lib/AST/Stmt.cpp

@@ -514,26 +514,19 @@ void LabeledConditionalStmt::setCond(StmtCondition e) {
 /// or `let self = self` condition.
 ///  - If `requiresCaptureListRef` is `true`, additionally requires that the
 ///    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 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);
   });
 }
 
 /// Whether or not this conditional stmt rebinds self with a `let self`
 /// or `let self = self` condition.
 ///  - If `requiresCaptureListRef` is `true`, additionally requires that the
 ///    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 StmtConditionElement::rebindsSelf(ASTContext &Ctx,
-                                       bool requiresCaptureListRef,
-                                       bool requireLoadExpr) const {
+                                       bool requiresCaptureListRef) const {
   auto pattern = getPatternOrNull();
   if (!pattern) {
     return false;
@@ -561,10 +554,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/SIL/IR/TypeLowering.cpp

@@ -170,15 +170,6 @@ CaptureKind TypeConverter::getDeclCaptureKind(CapturedValue capture,
       return CaptureKind::StorageAddress;
   }
 
-  // Reference storage types can appear in a capture list, which means
-  // we might allocate boxes to store the captures. However, those boxes
-  // have the same lifetime as the closure itself, so we must capture
-  // the box itself and not the payload, even if the closure is noescape,
-  // otherwise they will be destroyed when the closure is formed.
-  if (var->getInterfaceType()->is<ReferenceStorageType>()) {
-    return CaptureKind::Box;
-  }
-
   // For 'let' constants
   if (!var->supportsMutation()) {
     assert(getTypeLowering(

lib/Sema/MiscDiagnostics.cpp

@@ -1748,15 +1748,23 @@ class ImplicitSelfUsageChecker : public BaseDiagnosticWalker {
       return false;
     }
 
-    // Require `LoadExpr`s when validating the self binding.
+    // Require that the RHS of the `let self = self` condition
+    // refers to a variable defined in a capture list.
     // This lets us reject invalid examples like:
     //
-    //   let `self` = self ?? .somethingElse
+    //   var `self` = self ?? .somethingElse
     //   guard let self = self else { return }
     //   method() // <- implicit self is not allowed
     //
-    return conditionalStmt->rebindsSelf(Ctx, /*requiresCaptureListRef*/ false,
-                                        /*requireLoadExpr*/ true);
+    // In 5.10, instead of this check, compiler was checking that RHS of the
+    // self binding is loaded from a mutable variable. This is incorrect, but
+    // before SE-0481 compiler was trying to maintain this behavior in Swift 5
+    // mode for source compatibility. After SE-0481 this does not work
+    // anymore, because even in Swift 5 mode `weak self` capture is not mutable.
+    // So we have to introduce a breaking change as part of the SE-0481, and use
+    // proper check for capture list even in Swift 5 mode.
+    //
+    return conditionalStmt->rebindsSelf(Ctx, /*requiresCaptureListRef*/ true);
   }
 
   static bool
@@ -4003,11 +4011,6 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
       access &= ~RK_Written;
     }
 
-    // If this variable has WeakStorageType, then it can be mutated in ways we
-    // don't know.
-    if (var->getInterfaceType()->is<WeakStorageType>())
-      access |= RK_Written;
-    
     // Diagnose variables that were never used (other than their
     // initialization).
     //

lib/Sema/TypeCheckAttr.cpp

@@ -5302,11 +5302,6 @@ Type TypeChecker::checkReferenceOwnershipAttr(VarDecl *var, Type type,
   case ReferenceOwnershipOptionality::Allowed:
     break;
   case ReferenceOwnershipOptionality::Required:
-    if (var->isLet()) {
-      var->diagnose(diag::invalid_ownership_is_let, ownershipKind);
-      attr->setInvalid();
-    }
-
     if (!isOptional) {
       attr->setInvalid();