Fix BorrowingOperand more.

Don't allow an owned call argument to be considered a valid BorrowingOperand.

More generally, make sure there is a perfect equivalence between valid
BorrowingOperand and the corresponding OperandOwnership kind.

Author: atrick

include/swift/SIL/OwnershipUtils.h

@@ -255,6 +255,9 @@ struct BorrowedValue;
 /// borrowed and thus the incoming value must implicitly be borrowed until the
 /// user's corresponding end scope instruction.
 ///
+/// Invariant: For a given operand, BorrowingOperand is valid iff
+/// it has OperandOwnership::Borrow or OperandOwnership::Reborrow.
+///
 /// NOTE: We do not require that the guaranteed scope be represented by a
 /// guaranteed value in the same function: see begin_apply. In such cases, we
 /// require instead an end_* instruction to mark the end of the scope's region.
@@ -264,10 +267,14 @@ struct BorrowingOperand {
 
   BorrowingOperand(Operand *op)
       : op(op), kind(BorrowingOperandKind::get(op->getUser()->getKind())) {
-    if (kind == BorrowingOperandKind::Branch
-        && op->get().getOwnershipKind() != OwnershipKind::Guaranteed) {
+    auto ownership = op->getOperandOwnership();
+    if (ownership != OperandOwnership::Borrow
+        && ownership != OperandOwnership::Reborrow) {
+      // consuming applies and branch arguments are not borrowing operands.
       kind = BorrowingOperandKind::Invalid;
+      return;
     }
+    assert(kind != BorrowingOperandKind::Invalid && "missing case");
   }
   BorrowingOperand(const BorrowingOperand &other)
       : op(other.op), kind(other.kind) {}
@@ -284,28 +291,7 @@ struct BorrowingOperand {
   const Operand *operator->() const { return op; }
   Operand *operator->() { return op; }
 
-  operator bool() const { return kind != BorrowingOperandKind::Invalid && op; }
-
-  /// If \p op is a borrow introducing operand return it after doing some
-  /// checks.
-  static BorrowingOperand get(Operand *op) {
-    auto *user = op->getUser();
-    auto kind = BorrowingOperandKind::get(user->getKind());
-    if (!kind)
-      return {nullptr, kind};
-
-    if (kind == BorrowingOperandKind::Branch
-        && op->get().getOwnershipKind() != OwnershipKind::Guaranteed) {
-      return {nullptr, BorrowingOperandKind::Invalid};
-    }
-    return {op, kind};
-  }
-
-  /// If \p op is a borrow introducing operand return it after doing some
-  /// checks.
-  static BorrowingOperand get(const Operand *op) {
-    return get(const_cast<Operand *>(op));
-  }
+  operator bool() const { return kind != BorrowingOperandKind::Invalid; }
 
   /// If this borrowing operand results in the underlying value being borrowed
   /// over a region of code instead of just for a single instruction, visit
@@ -347,8 +333,14 @@ struct BorrowingOperand {
     llvm_unreachable("Covered switch isn't covered?!");
   }
 
-  /// Return true if the user instruction introduces a borrow scope? This is
-  /// true for both reborrows and nested borrows.
+  /// Return true if the user instruction defines a borrowed value that
+  /// introduces a borrow scope and therefore may be reborrowed. This is true
+  /// for both reborrows and nested borrows.
+  ///
+  /// Note that begin_apply does create a borrow scope, and may define
+  /// guaranteed value within that scope. The difference is that those yielded
+  /// values do not themselves introduce a borrow scope. In other words, they
+  /// cannot be reborrowed.
   ///
   /// If true, the visitBorrowIntroducingUserResults() can be called to acquire
   /// each BorrowedValue that introduces a new borrow scopes.
@@ -599,7 +591,7 @@ struct BorrowedValue {
                            &foundReborrows) const {
     bool foundAnyReborrows = false;
     for (auto *op : value->getUses()) {
-      if (auto borrowingOperand = BorrowingOperand::get(op)) {
+      if (auto borrowingOperand = BorrowingOperand(op)) {
         if (borrowingOperand.isReborrow()) {
           foundReborrows.push_back(
               {value->getParentBlock(), op->getOperandNumber()});

lib/SIL/IR/OperandOwnership.cpp

@@ -24,7 +24,7 @@ bool swift::checkOperandOwnershipInvariants(const Operand *operand) {
   OperandOwnership opOwnership = operand->getOperandOwnership();
   if (opOwnership == OperandOwnership::Borrow) {
     // Must be a valid BorrowingOperand.
-    return bool(BorrowingOperand::get(operand));
+    return bool(BorrowingOperand(const_cast<Operand *>(operand)));
   }
   return true;
 }

lib/SIL/Verifier/LinearLifetimeChecker.cpp

@@ -314,7 +314,7 @@ void State::checkForSameBlockUseAfterFree(Operand *consumingUse,
                        }) == userBlock->end()) {
         continue;
       }
-    } else if (auto borrowingOperand = BorrowingOperand::get(nonConsumingUse)) {
+    } else if (auto borrowingOperand = BorrowingOperand(nonConsumingUse)) {
       assert(borrowingOperand.isReborrow());
       // a reborrow is expected to be consumed by the same phi.
       continue;

lib/SIL/Verifier/SILOwnershipVerifier.cpp

@@ -244,7 +244,7 @@ bool SILValueOwnershipChecker::gatherNonGuaranteedUsers(
     // regular users so we can ensure that the borrow scope operand's scope is
     // completely within the owned value's scope. If we do not have a borrow
     // scope operand, just continue, we are done.
-    auto initialScopedOperand = BorrowingOperand::get(op);
+    auto initialScopedOperand = BorrowingOperand(op);
     if (!initialScopedOperand) {
       continue;
     }
@@ -348,7 +348,7 @@ bool SILValueOwnershipChecker::gatherUsers(
       // Ok, our operand does not consume guaranteed values. Check if it is a
       // BorrowScopeOperand and if so, add its end scope instructions as
       // implicit regular users of our value.
-      if (auto scopedOperand = BorrowingOperand::get(op)) {
+      if (auto scopedOperand = BorrowingOperand(op)) {
         assert(!scopedOperand.isReborrow());
 
         std::function<void(Operand *)> onError = [&](Operand *op) {

lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp

@@ -473,7 +473,7 @@ static bool tryJoinIfDestroyConsumingUseInSameBlock(
     // we need to only find scopes that end within the region in between the
     // singleConsumingUse (the original forwarded use) and the destroy_value. In
     // such a case, we must bail!
-    if (auto operand = BorrowingOperand::get(use))
+    if (auto operand = BorrowingOperand(use))
       if (!operand.visitScopeEndingUses([&](Operand *endScopeUse) {
             // Return false if we did see the relevant end scope instruction
             // in the block. That means that we are going to exit early and

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

@@ -508,7 +508,7 @@ static void eliminateReborrowsOfRecursiveBorrows(
       // Otherwise, we have a reborrow. For now our reborrows must be
       // phis. Add our owned value as a new argument of that phi along our
       // edge and undef along all other edges.
-      auto borrowingOp = BorrowingOperand::get(use);
+      auto borrowingOp = BorrowingOperand(use);
       auto *brInst = cast<BranchInst>(borrowingOp.op->getUser());
       auto *newBorrowedPhi = brInst->getArgForOperand(*borrowingOp);
       auto *newBasePhi =
@@ -572,7 +572,7 @@ rewriteReborrows(SILValue newBorrowedValue,
       // Otherwise, we have a reborrow. For now our reborrows must be
       // phis. Add our owned value as a new argument of that phi along our
       // edge and undef along all other edges.
-      auto borrowingOp = BorrowingOperand::get(use);
+      auto borrowingOp = BorrowingOperand(use);
       auto *brInst = cast<BranchInst>(borrowingOp.op->getUser());
       auto *newBorrowedPhi = brInst->getArgForOperand(*borrowingOp);
       auto *newBasePhi =