[DNM] Revert SE-0481 implementation

include/swift/AST/Stmt.h

@@ -727,7 +727,10 @@ 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.
-  bool rebindsSelf(ASTContext &Ctx, bool requiresCaptureListRef = false) const;
+  ///  - 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;
 
   SourceLoc getStartLoc() const;
   SourceLoc getEndLoc() const;
@@ -836,7 +839,10 @@ 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.
-  bool rebindsSelf(ASTContext &Ctx, bool requiresCaptureListRef = false) const;
+  ///  - 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;
 
   static bool classof(const Stmt *S) {
     return S->getKind() >= StmtKind::First_LabeledConditionalStmt &&

lib/AST/Expr.cpp

@@ -1373,8 +1373,11 @@ 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, VarDecl::Introducer::Let, nameLoc, name, DC);
+      new (Ctx) VarDecl(/*isStatic==*/false, introducer, nameLoc, name, DC);
 
   if (ownershipKind != ReferenceOwnership::Strong)
     VD->getAttrs().add(

lib/AST/Stmt.cpp

@@ -514,19 +514,26 @@ 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) const {
-  return llvm::any_of(getCond(), [&Ctx, requiresCaptureListRef](const auto &cond) {
-    return cond.rebindsSelf(Ctx, requiresCaptureListRef);
+                                         bool requiresCaptureListRef,
+                                         bool requireLoadExpr) const {
+  return llvm::any_of(getCond(), [&Ctx, requiresCaptureListRef,
+                                  requireLoadExpr](const auto &cond) {
+    return cond.rebindsSelf(Ctx, requiresCaptureListRef, requireLoadExpr);
   });
 }
 
 /// 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) const {
+                                       bool requiresCaptureListRef,
+                                       bool requireLoadExpr) const {
   auto pattern = getPatternOrNull();
   if (!pattern) {
     return false;
@@ -554,6 +561,10 @@ 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,6 +170,15 @@ 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,23 +1748,15 @@ class ImplicitSelfUsageChecker : public BaseDiagnosticWalker {
       return false;
     }
 
-    // Require that the RHS of the `let self = self` condition
-    // refers to a variable defined in a capture list.
+    // Require `LoadExpr`s when validating the self binding.
     // This lets us reject invalid examples like:
     //
-    //   var `self` = self ?? .somethingElse
+    //   let `self` = self ?? .somethingElse
     //   guard let self = self else { return }
     //   method() // <- implicit self is not allowed
     //
-    // 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);
+    return conditionalStmt->rebindsSelf(Ctx, /*requiresCaptureListRef*/ false,
+                                        /*requireLoadExpr*/ true);
   }
 
   static bool
@@ -4013,6 +4005,11 @@ 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,6 +5302,11 @@ 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();