Merge pull request #69287 from jckarter/enable-move-only-resilient-types-5.10

[5.10] Enable noncopyable resilient types.

Author: jckarter

include/swift/Basic/Features.def

@@ -107,6 +107,7 @@ LANGUAGE_FEATURE(
 LANGUAGE_FEATURE(AttachedMacros, 389, "Attached macros", hasSwiftSwiftParser)
 LANGUAGE_FEATURE(ExtensionMacros, 402, "Extension macros", hasSwiftSwiftParser)
 LANGUAGE_FEATURE(MoveOnly, 390, "noncopyable types", true)
+LANGUAGE_FEATURE(MoveOnlyResilientTypes, 390, "non-@frozen noncopyable types with library evolution", true)
 LANGUAGE_FEATURE(ParameterPacks, 393, "Value and type parameter packs", true)
 SUPPRESSIBLE_LANGUAGE_FEATURE(LexicalLifetimes, 0, "@_eagerMove/@_noEagerMove/@_lexicalLifetimes annotations", true)
 LANGUAGE_FEATURE(FreestandingMacros, 397, "freestanding declaration macros", true)
@@ -140,7 +141,6 @@ EXPERIMENTAL_FEATURE(NoImplicitCopy, true)
 EXPERIMENTAL_FEATURE(OldOwnershipOperatorSpellings, true)
 EXPERIMENTAL_FEATURE(MoveOnlyEnumDeinits, true)
 EXPERIMENTAL_FEATURE(MoveOnlyTuples, true)
-EXPERIMENTAL_FEATURE(MoveOnlyResilientTypes, true)
 EXPERIMENTAL_FEATURE(MoveOnlyPartialConsumption, true)
 
 EXPERIMENTAL_FEATURE(OneWayClosureParameters, false)

include/swift/SIL/OwnershipUtils.h

@@ -254,6 +254,7 @@ class BorrowingOperandKind {
   enum Kind : uint8_t {
     Invalid = 0,
     BeginBorrow,
+    StoreBorrow,
     BeginApply,
     Branch,
     Apply,
@@ -277,6 +278,8 @@ class BorrowingOperandKind {
       return Kind::Invalid;
     case SILInstructionKind::BeginBorrowInst:
       return Kind::BeginBorrow;
+    case SILInstructionKind::StoreBorrowInst:
+      return Kind::StoreBorrow;
     case SILInstructionKind::BeginApplyInst:
       return Kind::BeginApply;
     case SILInstructionKind::BranchInst:
@@ -386,6 +389,7 @@ struct BorrowingOperand {
     case BorrowingOperandKind::Invalid:
       llvm_unreachable("Using invalid case?!");
     case BorrowingOperandKind::BeginBorrow:
+    case BorrowingOperandKind::StoreBorrow:
     case BorrowingOperandKind::BeginApply:
     case BorrowingOperandKind::Apply:
     case BorrowingOperandKind::TryApply:
@@ -418,6 +422,7 @@ struct BorrowingOperand {
     case BorrowingOperandKind::BeginBorrow:
     case BorrowingOperandKind::Branch:
       return true;
+    case BorrowingOperandKind::StoreBorrow:
     case BorrowingOperandKind::BeginApply:
     case BorrowingOperandKind::Apply:
     case BorrowingOperandKind::TryApply:
@@ -790,7 +795,6 @@ class InteriorPointerOperandKind {
     RefTailAddr,
     OpenExistentialBox,
     ProjectBox,
-    StoreBorrow,
   };
 
 private:
@@ -819,8 +823,6 @@ class InteriorPointerOperandKind {
       return Kind::OpenExistentialBox;
     case SILInstructionKind::ProjectBoxInst:
       return Kind::ProjectBox;
-    case SILInstructionKind::StoreBorrowInst:
-      return Kind::StoreBorrow;
     }
   }
 
@@ -839,8 +841,6 @@ class InteriorPointerOperandKind {
       return Kind::OpenExistentialBox;
     case ValueKind::ProjectBoxInst:
       return Kind::ProjectBox;
-    case ValueKind::StoreBorrowInst:
-      return Kind::StoreBorrow;
     }
   }
 
@@ -897,8 +897,7 @@ struct InteriorPointerOperand {
     case InteriorPointerOperandKind::RefElementAddr:
     case InteriorPointerOperandKind::RefTailAddr:
     case InteriorPointerOperandKind::OpenExistentialBox:
-    case InteriorPointerOperandKind::ProjectBox:
-    case InteriorPointerOperandKind::StoreBorrow: {
+    case InteriorPointerOperandKind::ProjectBox: {
       // Ok, we have a valid instruction. Return the relevant operand.
       auto *op =
           &cast<SingleValueInstruction>(resultValue)->getAllOperands()[0];
@@ -941,8 +940,6 @@ struct InteriorPointerOperand {
       return cast<OpenExistentialBoxInst>(operand->getUser());
     case InteriorPointerOperandKind::ProjectBox:
       return cast<ProjectBoxInst>(operand->getUser());
-    case InteriorPointerOperandKind::StoreBorrow:
-      return cast<StoreBorrowInst>(operand->getUser());
     }
     llvm_unreachable("Covered switch isn't covered?!");
   }

lib/SIL/IR/OperandOwnership.cpp

@@ -452,7 +452,7 @@ OperandOwnership OperandOwnershipClassifier::visitBranchInst(BranchInst *bi) {
 OperandOwnership
 OperandOwnershipClassifier::visitStoreBorrowInst(StoreBorrowInst *i) {
   if (getValue() == i->getSrc()) {
-    return OperandOwnership::InteriorPointer;
+    return OperandOwnership::Borrow;
   }
   return OperandOwnership::TrivialUse;
 }

lib/SIL/Utils/OwnershipUtils.cpp

@@ -600,6 +600,9 @@ void BorrowingOperandKind::print(llvm::raw_ostream &os) const {
   case Kind::BeginBorrow:
     os << "BeginBorrow";
     return;
+  case Kind::StoreBorrow:
+    os << "StoreBorrow";
+    return;
   case Kind::BeginApply:
     os << "BeginApply";
     return;
@@ -649,6 +652,7 @@ bool BorrowingOperand::hasEmptyRequiredEndingUses() const {
   case BorrowingOperandKind::Invalid:
     llvm_unreachable("Using invalid case");
   case BorrowingOperandKind::BeginBorrow:
+  case BorrowingOperandKind::StoreBorrow:
   case BorrowingOperandKind::BeginApply:
   case BorrowingOperandKind::BeginAsyncLet:
   case BorrowingOperandKind::PartialApplyStack: {
@@ -687,6 +691,20 @@ bool BorrowingOperand::visitScopeEndingUses(
     // false.
     return !deadBorrow;
   }
+  case BorrowingOperandKind::StoreBorrow: {
+    bool deadBorrow = true;
+    for (auto *use : cast<StoreBorrowInst>(op->getUser())->getUses()) {
+      if (isa<EndBorrowInst>(use->getUser())) {
+        deadBorrow = false;
+        if (!func(use))
+          return false;
+      }
+    }
+    // FIXME: special case for dead borrows. This is dangerous because clients
+    // only expect visitScopeEndingUses to return false if the visitor returned
+    // false.
+    return !deadBorrow;
+  }
   case BorrowingOperandKind::BeginApply: {
     bool deadApply = true;
     auto *user = cast<BeginApplyInst>(op->getUser());
@@ -763,6 +781,7 @@ BorrowedValue BorrowingOperand::getBorrowIntroducingUserResult() const {
   case BorrowingOperandKind::Yield:
   case BorrowingOperandKind::PartialApplyStack:
   case BorrowingOperandKind::BeginAsyncLet:
+  case BorrowingOperandKind::StoreBorrow:
     return BorrowedValue();
 
   case BorrowingOperandKind::BeginBorrow: {
@@ -792,6 +811,7 @@ SILValue BorrowingOperand::getScopeIntroducingUserResult() {
   case BorrowingOperandKind::BeginAsyncLet:
   case BorrowingOperandKind::PartialApplyStack:
   case BorrowingOperandKind::BeginBorrow:
+  case BorrowingOperandKind::StoreBorrow:
     return cast<SingleValueInstruction>(op->getUser());
 
   case BorrowingOperandKind::BeginApply:

lib/SILGen/SILGenApply.cpp

@@ -6517,9 +6517,9 @@ ArgumentSource AccessorBaseArgPreparer::prepareAccessorObjectBaseArg() {
   if (selfParam.isConsumed() && !base.hasCleanup()) {
     base = base.copyUnmanaged(SGF, loc);
   }
-
-  // If the parameter is indirect, we need to drop the value into
-  // temporary memory.
+  // If the parameter is indirect, we'll need to drop the value into
+  // temporary memory. Make a copy scoped to the current formal access that
+  // we can materialize later.
   if (SGF.silConv.isSILIndirect(selfParam)) {
     // It's a really bad idea to materialize when we're about to
     // pass a value to an inout argument, because it's a really easy
@@ -6529,14 +6529,17 @@ ArgumentSource AccessorBaseArgPreparer::prepareAccessorObjectBaseArg() {
     // itself).
     assert(!selfParam.isIndirectMutating() &&
            "passing unmaterialized r-value as inout argument");
-
-    base = base.formallyMaterialize(SGF, loc);
-    auto shouldTake = IsTake_t(base.hasCleanup());
-    base = SGF.emitFormalAccessLoad(loc, base.forward(SGF),
-                                    SGF.getTypeLowering(baseLoweredType),
-                                    SGFContext(), shouldTake);
+           
+    // Avoid copying the base if it's move-only. It should be OK to do in this
+    // case since the move-only base will be accessed in a formal access scope
+    // as well. This isn't always true for copyable bases so be less aggressive
+    // in that case.
+    if (base.getType().isMoveOnly()) {
+      base = base.formalAccessBorrow(SGF, loc);
+    } else {
+      base = base.formalAccessCopy(SGF, loc);
+    }
   }
-
   return ArgumentSource(loc, RValue(SGF, loc, baseFormalType, base));
 }
 

lib/SILGen/SILGenLValue.cpp

@@ -2935,10 +2935,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
   SILGenBorrowedBaseVisitor(SILGenLValue &SGL, SILGenFunction &SGF)
       : SGL(SGL), SGF(SGF) {}
 
-  /// Returns the subexpr
   static bool isNonCopyableBaseBorrow(SILGenFunction &SGF, Expr *e) {
-    if (auto *le = dyn_cast<LoadExpr>(e))
-      return le->getType()->isPureMoveOnly();
     if (auto *m = dyn_cast<MemberRefExpr>(e)) {
       // If our m is a pure noncopyable type or our base is, we need to perform
       // a noncopyable base borrow.
@@ -2949,8 +2946,39 @@ class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
       return m->getType()->isPureMoveOnly() ||
              m->getBase()->getType()->isPureMoveOnly();
     }
-    if (auto *d = dyn_cast<DeclRefExpr>(e))
-      return e->getType()->isPureMoveOnly();
+
+    if (auto *le = dyn_cast<LoadExpr>(e)) {
+      // Noncopyable type is obviously noncopyable.
+      if (le->getType()->isPureMoveOnly()) {
+        return true;
+      }
+      // Otherwise, check if the thing we're loading from is a noncopyable
+      // param decl.
+      e = le->getSubExpr();
+      // fall through...
+    }
+    
+    if (auto *de = dyn_cast<DeclRefExpr>(e)) {
+      // Noncopyable type is obviously noncopyable.
+      if (de->getType()->isPureMoveOnly()) {
+        return true;
+      }
+      // If the decl ref refers to a parameter with an explicit ownership
+      // modifier, it is not implicitly copyable.
+      if (auto pd = dyn_cast<ParamDecl>(de->getDecl())) {
+        switch (pd->getSpecifier()) {
+        case ParamSpecifier::Borrowing:
+        case ParamSpecifier::Consuming:
+          return true;
+        case ParamSpecifier::Default:
+        case ParamSpecifier::InOut:
+        case ParamSpecifier::LegacyShared:
+        case ParamSpecifier::LegacyOwned:
+          return false;
+        }
+        llvm_unreachable("unhandled switch case");
+      }
+    }
     return false;
   }
 

lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp

@@ -725,6 +725,9 @@ static SILValue tryRewriteToPartialApplyStack(
 
   SmallSetVector<SILValue, 4> borrowedOriginals;
 
+  unsigned appliedArgStartIdx =
+        newPA->getOrigCalleeType()->getNumParameters() - newPA->getNumArguments();
+
   for (unsigned i : indices(newPA->getArgumentOperands())) {
     auto &arg = newPA->getArgumentOperands()[i];
     SILValue copy = arg.get();
@@ -747,11 +750,12 @@ static SILValue tryRewriteToPartialApplyStack(
     }
 
     // Is the capture a borrow?
-    auto paramIndex = newPA
-      ->getArgumentIndexForOperandIndex(i + newPA->getArgumentOperandNumber())
-      .value();
-    if (!newPA->getOrigCalleeType()->getParameters()[paramIndex]
-          .isIndirectInGuaranteed()) {
+
+    auto paramIndex = i + appliedArgStartIdx;
+    auto param = newPA->getOrigCalleeType()->getParameters()[paramIndex];
+    LLVM_DEBUG(param.print(llvm::dbgs());
+               llvm::dbgs() << '\n');
+    if (!param.isIndirectInGuaranteed()) {
       LLVM_DEBUG(llvm::dbgs() << "-- not an in_guaranteed parameter\n";
                  newPA->getOrigCalleeType()->getParameters()[paramIndex]
                    .print(llvm::dbgs());

lib/SILOptimizer/Mandatory/DiagnoseLifetimeIssues.cpp

@@ -189,6 +189,8 @@ visitUses(SILValue def, bool updateLivenessAndWeakStores, int callDepth) {
         // Try to get information from the called function.
         switch (getArgumentState(ai, use, callDepth)) {
         case DoesNotEscape:
+          if (updateLivenessAndWeakStores)
+            liveness->updateForUse(user, /*lifetimeEnding*/ false);
           break;
         case CanEscape:
           return CanEscape;