[SILOptimizer]: fix some missing use after consume diagnostics

[jamieQ]

Updates ConsumeOperatorCopyableValuesChecker to identify store_borrow instructions as a liveness-affecting use so that patterns that would previously slip through undiagnosed are correctly identified. e.g.

func use<V>(_ v: borrowing V) {}

func f() {
  let a = A()
  _ = consume a
  use(a) // previously would not be diagnosed
}

Resolves #83277

[jamieQ]

@swift-ci please smoke test

[jamieQ]

i copied the logic for the lexical borrow above, but it's not entirely clear to me if this is a conceptually correct solution (tests seem to pass, but they also passed before this change so 🤷 – well, the existing ones did, not the added cases). feedback appreciated if/when you have time – cc @nate-chandler @eeckstein

[nate-chandler]

The comment we are going to check its uses separately and emit diagnostics for it doesn't apply here, we need to add the whole range in which the value is borrowed to liveness as in the non-lexical case above:

if (liveness->updateForBorrowingOperand(use) !=
                InnerBorrowKind::Contained) {
               return false;
             }

[jamieQ]

@nate-chandler slightly tangential, but do you happen to know if the implementation of updateForBorrowingOperand() is robust enough that the handling of this case could (or should) be refactored to only special case the lexical begin_borrows, and have everything else take the alternate path? i.e. could the 'FIXME' below be resolved with such an approach? or should this just special case store_borrow?

[nate-chandler]

I suggest landing the store_borrow fix first and then investigating more involved changes separately, ideally together with Swift source test cases that aren't handled properly.

At that point, yes, this would be a good starting point:

      case OperandOwnership::Borrow: {
        auto *bbi = dyn_cast<BeginBorrowInst>(user);
        if (bbi && bbi->isFromVarDecl()) {
          // If we have a begin_borrow [var_decl], we are going to check its
          // uses separately and emit diagnostics for it. So we just need to
          // the begin_borrow to liveness.
          //
          // NOTE: We know that semantically the use VarDecl must have a
          // separate lifetime from the base VarDecl that we are processing. We
          // do not want to include those uses as transitive uses of our base
          // VarDecl lifetime. We just want to treat the formation of the new
          // variable as a use. Thus we only include the begin_borrow itself as
          // the use.
          liveness->updateForUse(bbi, false /*lifetime ending*/);
          break;
        }
        // Otherwise, try to update liveness for a borrowing operand
        // use. This will make it so that we add the end_borrows of the
        // liveness use. If we have a reborrow here, we will bail.
        if (liveness->updateForBorrowingOperand(use) !=
            InnerBorrowKind::Contained) {
          return false;
        }
        break;
      }

Note in particular this changes to [var_decl] which is the flag that's actually looked for in the begin_borrow instructions added to the worklist.

[jamieQ]

@swift-ci please smoke test

[nate-chandler]

Looks good--thanks!

[xwu]

@nate-chandler Worthwhile cherrypicking for 6.2?

[nate-chandler]

@jamieQ Please raise a PR to cherry-pick this to 6.2 to give the branch managers the option of taking this patch .