Merge pull request #67677 from gottesmm/borrowed-base-silgenlvalue

[move-only] Ensure that we properly nest accesses to base values if the base is noncopyable or the accessor result is noncopyable.

Author: gottesmm

include/swift/SIL/SILInstruction.h

@@ -4421,8 +4421,18 @@ class StoreBorrowInst
 
   ArrayRef<Operand> getAllOperands() const { return Operands.asArray(); }
   MutableArrayRef<Operand> getAllOperands() { return Operands.asArray(); }
+
+  using EndBorrowRange =
+      decltype(std::declval<ValueBase>().getUsersOfType<EndBorrowInst>());
+
+  /// Return a range over all EndBorrow instructions for this BeginBorrow.
+  EndBorrowRange getEndBorrows() const;
 };
 
+inline auto StoreBorrowInst::getEndBorrows() const -> EndBorrowRange {
+  return getUsersOfType<EndBorrowInst>();
+}
+
 /// Represents the end of a borrow scope of a value %val from a
 /// value or address %src.
 ///

lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp

@@ -129,6 +129,11 @@ SubElementOffset::computeForAddress(SILValue projectionDerivedFromRoot,
       continue;
     }
 
+    if (auto *sbi = dyn_cast<StoreBorrowInst>(projectionDerivedFromRoot)) {
+      projectionDerivedFromRoot = sbi->getDest();
+      continue;
+    }
+
     if (auto *m = dyn_cast<MoveOnlyWrapperToCopyableAddrInst>(
             projectionDerivedFromRoot)) {
       projectionDerivedFromRoot = m->getOperand();

lib/SIL/Verifier/SILVerifier.cpp

@@ -2595,7 +2595,12 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     require(SI->getDest()->getType().isAddress(),
             "Must store to an address dest");
     // Note: This is the current implementation and the design is not final.
-    require(isa<AllocStackInst>(SI->getDest()),
+    auto isLegal = [](SILValue value) {
+      if (auto *mmci = dyn_cast<MarkMustCheckInst>(value))
+        value = mmci->getOperand();
+      return isa<AllocStackInst>(value);
+    };
+    require(isLegal(SI->getDest()),
             "store_borrow destination can only be an alloc_stack");
     requireSameType(SI->getDest()->getType().getObjectType(),
                     SI->getSrc()->getType(),

lib/SILGen/LValue.h

@@ -530,7 +530,7 @@ class LValue {
                                  SGFAccessKind selfAccess,
                                  SGFAccessKind otherAccess);
 
-  void dump() const;
+  SWIFT_DEBUG_DUMP;
   void dump(raw_ostream &os, unsigned indent = 0) const;
 };
 

lib/SILGen/SILGenApply.cpp

@@ -3124,6 +3124,21 @@ Expr *ArgumentSource::findStorageReferenceExprForMoveOnly(
   if (kind == StorageReferenceOperationKind::Consume && !sawLoad)
     return nullptr;
 
+  // If we did not see a load or a subscript expr and our argExpr is a
+  // declref_expr, return nullptr. We have an object not something that will be
+  // in memory. This can happen with classes or with values captured by a
+  // closure.
+  //
+  // NOTE: If we see a member_ref_expr from a decl_ref_expr, we still process it
+  // since the declref_expr could be from a class.
+  if (!sawLoad && !subscriptExpr) {
+    if (auto *declRef = dyn_cast<DeclRefExpr>(argExpr)) {
+      assert(!declRef->getType()->is<LValueType>() &&
+             "Shouldn't ever have an lvalue type here!");
+      return nullptr;
+    }
+  }
+
   auto result = ::findStorageReferenceExprForBorrow(argExpr);
 
   if (!result)
@@ -3143,31 +3158,18 @@ Expr *ArgumentSource::findStorageReferenceExprForMoveOnly(
   }
 
   if (!storage)
-      return nullptr;
+    return nullptr;
   assert(type);
 
   SILType ty =
       SGF.getLoweredType(type->getWithoutSpecifierType()->getCanonicalType());
   bool isMoveOnly = ty.isPureMoveOnly();
   if (auto *pd = dyn_cast<ParamDecl>(storage)) {
-      isMoveOnly |= pd->getSpecifier() == ParamSpecifier::Borrowing;
-      isMoveOnly |= pd->getSpecifier() == ParamSpecifier::Consuming;
+    isMoveOnly |= pd->getSpecifier() == ParamSpecifier::Borrowing;
+    isMoveOnly |= pd->getSpecifier() == ParamSpecifier::Consuming;
   }
   if (!isMoveOnly)
-      return nullptr;
-
-  // It makes sense to borrow any kind of storage we refer to at this stage,
-  // but SILGenLValue does not currently handle some kinds of references well.
-  //
-  // When rejecting to do the LValue-style borrow here, it'll end up going thru
-  // the RValue-style emission, after which the extra copy will get eliminated.
-  //
-  // If we did not see a LoadExpr around the argument expression, then only
-  // do the borrow if the storage is non-local.
-  // FIXME: I don't have a principled reason for why this matters and hope that
-  // we can fix the AST we're working with.
-  if (!sawLoad && storage->getDeclContext()->isLocalContext())
-      return nullptr;
+    return nullptr;
 
   // Claim the value of this argument since we found a storage reference that
   // has a move only base.

lib/SILGen/SILGenLValue.cpp

@@ -2156,12 +2156,25 @@ namespace {
         if (value.getType().isAddress() ||
             !isReadAccessResultAddress(getAccessKind()))
           return value;
+
+        // If we have a guaranteed object and our read access result requires an
+        // address, store it using a store_borrow.
+        if (value.getType().isObject() &&
+            value.getOwnershipKind() == OwnershipKind::Guaranteed) {
+          SILValue alloc = SGF.emitTemporaryAllocation(loc, getTypeOfRValue());
+          if (alloc->getType().isMoveOnly())
+            alloc = SGF.B.createMarkMustCheckInst(
+                loc, alloc, MarkMustCheckInst::CheckKind::NoConsumeOrAssign);
+          return SGF.B.createFormalAccessStoreBorrow(loc, value, alloc);
+        }
       }
 
       // Otherwise, we need to make a temporary.
+      // TODO: This needs to be changed to use actual store_borrows. Noncopyable
+      // types do not support tuples today, so we can avoid this for now.
       // TODO: build a scalar tuple if possible.
-      auto temporary =
-        SGF.emitTemporary(loc, SGF.getTypeLowering(getTypeOfRValue()));
+      auto temporary = SGF.emitFormalAccessTemporary(
+          loc, SGF.getTypeLowering(getTypeOfRValue()));
       auto yieldsAsArray = llvm::makeArrayRef(yields);
       copyBorrowedYieldsIntoTemporary(SGF, loc, yieldsAsArray,
                                       getOrigFormalType(), getSubstFormalType(),
@@ -2882,6 +2895,118 @@ static ManagedValue visitRecNonInOutBase(SILGenLValue &SGL, Expr *e,
                                                       value);
 }
 
+static CanType getBaseFormalType(Expr *baseExpr) {
+  return baseExpr->getType()->getWithoutSpecifierType()->getCanonicalType();
+}
+
+class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
+    : public Lowering::ExprVisitor<SILGenBorrowedBaseVisitor, LValue,
+                                   SGFAccessKind, LValueOptions> {
+public:
+  SILGenLValue &SGL;
+  SILGenFunction &SGF;
+
+  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.
+      //
+      // DISCUSSION: We can have a noncopyable member_ref_expr with a copyable
+      // base if the noncopyable member_ref_expr is from a computed method. In
+      // such a case, we want to ensure that we wrap things the right way.
+      return m->getType()->isPureMoveOnly() ||
+             m->getBase()->getType()->isPureMoveOnly();
+    }
+    if (auto *d = dyn_cast<DeclRefExpr>(e))
+      return e->getType()->isPureMoveOnly();
+    return false;
+  }
+
+  LValue visitExpr(Expr *e, SGFAccessKind accessKind, LValueOptions options) {
+    e->dump(llvm::errs());
+    llvm::report_fatal_error("Unimplemented node!");
+  }
+
+  LValue visitMemberRefExpr(MemberRefExpr *e, SGFAccessKind accessKind,
+                            LValueOptions options) {
+    // If we have a member_ref_expr, we create a component that will when we
+    // evaluate the lvalue,
+    VarDecl *var = cast<VarDecl>(e->getMember().getDecl());
+
+    assert(!e->getType()->is<LValueType>());
+
+    auto accessSemantics = e->getAccessSemantics();
+    AccessStrategy strategy = var->getAccessStrategy(
+        accessSemantics, getFormalAccessKind(accessKind),
+        SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion());
+
+    auto baseFormalType = getBaseFormalType(e->getBase());
+    LValue lv = visit(
+        e->getBase(),
+        getBaseAccessKind(SGF.SGM, var, accessKind, strategy, baseFormalType),
+        getBaseOptions(options, strategy));
+    llvm::Optional<ActorIsolation> actorIso;
+    if (e->isImplicitlyAsync())
+      actorIso = getActorIsolation(var);
+    lv.addMemberVarComponent(SGF, e, var, e->getMember().getSubstitutions(),
+                             options, e->isSuper(), accessKind, strategy,
+                             getSubstFormalRValueType(e),
+                             false /*is on self parameter*/, actorIso);
+    return lv;
+  }
+
+  ManagedValue emitImmediateBaseValue(Expr *e) {
+    // We are going to immediately use this base value, so we want to borrow it.
+    ManagedValue mv =
+        SGF.emitRValueAsSingleValue(e, SGFContext::AllowImmediatePlusZero);
+    if (mv.isPlusZeroRValueOrTrivial())
+      return mv;
+
+    // Any temporaries needed to materialize the lvalue must be destroyed when
+    // at the end of the lvalue's formal evaluation scope.
+    // e.g. for foo(self.bar)
+    //   %self = load [copy] %ptr_self
+    //   %rvalue = barGetter(%self)
+    //   destroy_value %self // self must be released before calling foo.
+    //   foo(%rvalue)
+    SILValue value = mv.forward(SGF);
+    return SGF.emitFormalAccessManagedRValueWithCleanup(CleanupLocation(e),
+                                                        value);
+  }
+
+  LValue visitDeclRefExpr(DeclRefExpr *e, SGFAccessKind accessKind,
+                          LValueOptions options) {
+    if (accessKind == SGFAccessKind::BorrowedObjectRead) {
+      auto rv = emitImmediateBaseValue(e);
+      CanType formalType = getSubstFormalRValueType(e);
+      auto typeData = getValueTypeData(accessKind, formalType, rv.getValue());
+      LValue lv;
+      lv.add<ValueComponent>(rv, llvm::None, typeData, /*isRValue=*/true);
+      return lv;
+    }
+
+    return SGL.visitDeclRefExpr(e, accessKind, options);
+  }
+
+  LValue visitLoadExpr(LoadExpr *e, SGFAccessKind accessKind,
+                       LValueOptions options) {
+    // TODO: orig abstraction pattern.
+    LValue lv = SGL.visitRec(e->getSubExpr(),
+                             SGFAccessKind::BorrowedAddressRead, options);
+    CanType formalType = getSubstFormalRValueType(e);
+    LValueTypeData typeData{accessKind, AbstractionPattern(formalType),
+                            formalType, lv.getTypeOfRValue().getASTType()};
+    lv.add<BorrowValueComponent>(typeData);
+    return lv;
+  }
+};
+
 LValue SILGenLValue::visitRec(Expr *e, SGFAccessKind accessKind,
                               LValueOptions options, AbstractionPattern orig) {
   // First see if we have an lvalue type. If we do, then quickly handle that and
@@ -2894,19 +3019,14 @@ LValue SILGenLValue::visitRec(Expr *e, SGFAccessKind accessKind,
   // a `borrow x` operator, the operator is used on the base here), we want to
   // apply the lvalue within a formal access to the original value instead of
   // an actual loaded copy.
-  
-  if (e->getType()->isPureMoveOnly()) {
-    if (auto load = dyn_cast<LoadExpr>(e)) {
-      LValue lv = visitRec(load->getSubExpr(), SGFAccessKind::BorrowedAddressRead,
-                               options, orig);
-      CanType formalType = getSubstFormalRValueType(e);
-      LValueTypeData typeData{accessKind, AbstractionPattern(formalType),
-                              formalType, lv.getTypeOfRValue().getASTType()};
-      lv.add<BorrowValueComponent>(typeData);
-      return lv;
-    }
+  if (SILGenBorrowedBaseVisitor::isNonCopyableBaseBorrow(SGF, e)) {
+    SILGenBorrowedBaseVisitor visitor(*this, SGF);
+    auto accessKind = SGFAccessKind::BorrowedObjectRead;
+    if (e->getType()->is<LValueType>())
+      accessKind = SGFAccessKind::BorrowedAddressRead;
+    return visitor.visit(e, accessKind, options);
   }
-  
+
   // Otherwise we have a non-lvalue type (references, values, metatypes,
   // etc). These act as the root of a logical lvalue. Compute the root value,
   // wrap it in a ValueComponent, and return it for our caller.
@@ -3559,10 +3679,6 @@ static SGFAccessKind getBaseAccessKind(SILGenModule &SGM,
   llvm_unreachable("bad access strategy");
 }
 
-static CanType getBaseFormalType(Expr *baseExpr) {
-  return baseExpr->getType()->getWithoutSpecifierType()->getCanonicalType();
-}
-
 bool isCallToReplacedInDynamicReplacement(SILGenFunction &SGF,
                                           AbstractFunctionDecl *afd,
                                           bool &isObjCReplacementSelfCall);