silgen: allow borrow of subscript for noncopyables

If a subscript uses a read accessor to yield a noncopyable value,
we'd emit an `end_apply` that's too tightly scoped to allow for
a subsequent borrowing access on the yielded value.

resolves rdar://159079818

Author: kavon

lib/SILGen/FormalEvaluation.cpp

@@ -195,6 +195,20 @@ void FormalEvaluationScope::verify() const {
   }
 }
 
+void FormalEvaluationScope::deferPop() && {
+  ASSERT(previous && "no previous scope to defer the pop for!");
+  auto &context = SGF.FormalEvalContext;
+
+  // Remove ourselves as the innermost scope of the chain.
+  ASSERT(context.innermostScope == this &&
+         "popping formal-evaluation scopes out of order");
+  context.innermostScope = previous;
+
+  // Clear-out our saved depth to deactivate our pop-on-deinit.
+  savedDepth.reset();
+}
+
+
 //===----------------------------------------------------------------------===//
 //                         Formal Evaluation Context
 //===----------------------------------------------------------------------===//

lib/SILGen/FormalEvaluation.h

@@ -224,6 +224,14 @@ class FormalEvaluationScope {
     savedDepth.reset();
   }
 
+  /// Defers the emission of clean-ups in this scope until the
+  /// immediate outer evaluation scope is popped.
+  ///
+  /// This is useful for getting-around tightly-nested formal
+  /// access scopes, when a borrow of a value needs to be
+  /// returned and copying the value is not an option.
+  void deferPop() &&;
+
   FormalEvaluationScope(const FormalEvaluationScope &) = delete;
   FormalEvaluationScope &operator=(const FormalEvaluationScope &) = delete;
 

lib/SILGen/SILGenExpr.cpp

@@ -2792,6 +2792,20 @@ RValue RValueEmitter::visitSubscriptExpr(SubscriptExpr *E, SGFContext C) {
   // Any writebacks for this access are tightly scoped.
   FormalEvaluationScope scope(SGF);
 
+  // For a noncopyable resulting expression, try to borrow instead.
+  if (E->getType()->isNoncopyable()) {
+    LValue lv = SGF.emitLValue(E, SGFAccessKind::BorrowedObjectRead);
+    ManagedValue mv = SGF.emitRawProjectedLValue(E, std::move(lv));
+
+    // If we got back a borrow (aka +0) because of a coroutine read
+    // accessor, defer the end_apply to the outer evaluation scope.
+    // Otherwise, we can end any other formal accesses now.
+    if (mv.isPlusZero())
+      std::move(scope).deferPop();
+
+    return RValue(SGF, E, mv);
+  }
+
   LValue lv = SGF.emitLValue(E, SGFAccessKind::OwnedObjectRead);
   // We can't load at +0 without further analysis, since the formal access into
   // the lvalue will end immediately.

lib/SILGen/SILGenFunction.h

@@ -2207,6 +2207,12 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
                                   TSanKind tsanKind = TSanKind::None);
   ManagedValue emitConsumedLValue(SILLocation loc, LValue &&src,
                                   TSanKind tsanKind = TSanKind::None);
+  /// Simply projects the LValue as a ManagedValue, allowing the caller
+  /// to check invariants and make adjustments manually.
+  ///
+  /// When in doubt, reach for emit[Borrowed|Consumed|etc]LValue.
+  ManagedValue emitRawProjectedLValue(SILLocation loc, LValue &&src,
+                                  TSanKind tsanKind = TSanKind::None);
   LValue emitOpenExistentialLValue(SILLocation loc,
                                    LValue &&existentialLV,
                                    CanArchetypeType openedArchetype,

lib/SILGen/SILGenLValue.cpp

@@ -5587,11 +5587,8 @@ ManagedValue SILGenFunction::emitAddressOfLValue(SILLocation loc,
          src.getAccessKind() == SGFAccessKind::OwnedAddressConsume ||
          src.getAccessKind() == SGFAccessKind::ReadWrite);
 
-  ManagedValue addr;
-  PathComponent &&component =
-    drillToLastComponent(loc, std::move(src), addr, tsanKind);
+  ManagedValue addr = emitRawProjectedLValue(loc, std::move(src), tsanKind);
 
-  addr = drillIntoComponent(*this, loc, std::move(component), addr, tsanKind);
   assert(addr.getType().isAddress() &&
          "resolving lvalue did not give an address");
   return ManagedValue::forLValue(addr.getValue());
@@ -5605,12 +5602,7 @@ ManagedValue SILGenFunction::emitBorrowedLValue(SILLocation loc,
          src.getAccessKind() == SGFAccessKind::BorrowedObjectRead);
   bool isIgnored = src.getAccessKind() == SGFAccessKind::IgnoredRead;
 
-  ManagedValue base;
-  PathComponent &&component =
-    drillToLastComponent(loc, std::move(src), base, tsanKind);
-
-  auto value =
-    drillIntoComponent(*this, loc, std::move(component), base, tsanKind);
+  auto value = emitRawProjectedLValue(loc, std::move(src), tsanKind);
 
   // If project() returned an owned value, and the caller cares, borrow it.
   if (value.hasCleanup() && !isIgnored)
@@ -5621,15 +5613,20 @@ ManagedValue SILGenFunction::emitBorrowedLValue(SILLocation loc,
 ManagedValue SILGenFunction::emitConsumedLValue(SILLocation loc, LValue &&src,
                                                 TSanKind tsanKind) {
   assert(isConsumeAccess(src.getAccessKind()));
+  return emitRawProjectedLValue(loc, std::move(src), tsanKind);
+}
 
+ManagedValue SILGenFunction::emitRawProjectedLValue(SILLocation loc,
+                                                LValue &&src,
+                                                TSanKind tsanKind) {
   ManagedValue base;
   PathComponent &&component =
-      drillToLastComponent(loc, std::move(src), base, tsanKind);
+    drillToLastComponent(loc, std::move(src), base, tsanKind);
 
-  auto value =
-      drillIntoComponent(*this, loc, std::move(component), base, tsanKind);
+  ManagedValue result =
+    drillIntoComponent(*this, loc, std::move(component), base, tsanKind);
 
-  return value;
+  return result;
 }
 
 LValue

test/SILGen/moveonly.swift

@@ -982,9 +982,13 @@ public struct LoadableSubscriptGetOnlyTester : ~Copyable {
 // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
 // CHECK: [[MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[ACCESS]]
 // CHECK: [[LOAD:%.*]] = load_borrow [unchecked] [[MARK]]
-// CHECK: apply {{%.*}}([[M2_PROJECT]], {{%.*}}, [[LOAD]])
+// CHECK: [[STACK_ALLOC:%.*]] = alloc_stack $AddressOnlyProtocol
+// CHECK: [[M_STACK_ALLOC:%.*]] = mark_unresolved_non_copyable_value [consumable_and_assignable] [[STACK_ALLOC]]
+// CHECK: apply {{%.*}}([[M_STACK_ALLOC]], {{%.*}}, [[LOAD]])
 // CHECK: end_borrow [[LOAD]]
 // CHECK: end_access [[ACCESS]]
+// CHECK: copy_addr [take] [[M_STACK_ALLOC]] to [init] [[M2_PROJECT]]
+// CHECK: dealloc_stack [[STACK_ALLOC]]
 // CHECK: [[M2_MARK:%.*]] = mark_unresolved_non_copyable_value [no_consume_or_assign] [[M2_PROJECT]]
 // CHECK: end_borrow [[M2_BORROW]]
 // CHECK: destroy_value [[M2_BOX]]

test/SILOptimizer/moveonly_read_coroutine.swift

@@ -6,6 +6,16 @@
 class X {}
 
 struct NC: ~Copyable {
+  var field: Int = 0
+  var consumingGetter: Int {
+    consuming get { return 0 }
+  }
+  var readCoroutine: Int {
+    _read { yield field }
+  }
+  consuming func consumingFunc() -> Int { return 0 }
+  borrowing func borrowingFunc() -> Int { return 0 }
+
 #if EMPTY
 #elseif TRIVIAL
   var x: Int = 0
@@ -23,6 +33,10 @@ struct S {
   var data: NC {
     _read { yield NC() }
   }
+
+  subscript(index: Int) -> NC {
+    _read { yield NC() }
+  }
 }
 
 struct SNC: ~Copyable {
@@ -31,6 +45,10 @@ struct SNC: ~Copyable {
   var data: NC {
     _read { yield _data }
   }
+
+  subscript(index: Int) -> NC {
+    _read { yield _data }
+  }
 }
 
 class C {
@@ -39,35 +57,84 @@ class C {
   var data: NC {
     _read { yield _data }
   }
+
+  subscript(index: Int) -> NC {
+    _read { yield _data }
+  }
 }
 
 protocol P {
   @_borrowed
   var data: NC { get }
+
+  @_borrowed
+  subscript(index: Int) -> NC { get }
 }
 
 func borrow(_ nc: borrowing NC) {}
 func take(_ nc: consuming NC) {}
 
+func assume(_ b: Bool) { precondition(b, "uh oh") }
+
 func test(c: C) {
   borrow(c.data)
   take(c.data) // expected-error{{'c.data' is borrowed and cannot be consumed}} expected-note{{consumed here}}
+
+  borrow(c[0])
+  assume(c[0].field == 0)
+  assume(c[0].borrowingFunc() == 0)
+  assume(c[0].readCoroutine == 0)
+  // take(c[0]) // FIXME: the move-checker's diagnostics produced for this code are non-deterministic?
+  assume(c[0].consumingGetter == 0) // expected-error {{'c.subscript' is borrowed and cannot be consumed}} expected-note {{consumed here}}
+  // assume(c[0].consumingFunc() == 0) // FIXME: the move-checker's diagnostics produced for this code are non-deterministic?
 }
 func test(s: S) {
   borrow(s.data)
   take(s.data) // expected-error{{'s.data' is borrowed and cannot be consumed}} expected-note{{consumed here}}
+
+  borrow(s[0])
+  assume(s[0].field == 0)
+  assume(s[0].borrowingFunc() == 0)
+  assume(s[0].readCoroutine == 0)
+  // take(s[0]) // FIXME: the move-checker's diagnostics produced for this code are non-deterministic?
+  assume(s[0].consumingGetter == 0) // expected-error {{'s.subscript' is borrowed and cannot be consumed}} expected-note {{consumed here}}
+  // assume(s[0].consumingFunc() == 0) // FIXME: the move-checker's diagnostics produced for this code are non-deterministic?
 }
 func test(snc: borrowing SNC) {
   borrow(snc.data)
   take(snc.data) // expected-error{{'snc.data' is borrowed and cannot be consumed}} expected-note{{consumed here}}
+
+  borrow(snc[0])
+  assume(snc[0].field == 0)
+  assume(snc[0].borrowingFunc() == 0)
+  assume(snc[0].readCoroutine == 0)
+  // take(snc[0]) // FIXME: the move-checker's diagnostics produced for this code are non-deterministic?
+  assume(snc[0].consumingGetter == 0) // expected-error {{'snc.subscript' is borrowed and cannot be consumed}} expected-note {{consumed here}}
+  // assume(snc[0].consumingFunc() == 0) // FIXME: the move-checker's diagnostics produced for this code are non-deterministic?
 }
 
 func test<T: P>(t: T) {
   borrow(t.data)
   take(t.data) // expected-error{{'t.data' is borrowed and cannot be consumed}} expected-note{{consumed here}}
+
+  borrow(t[0])
+  assume(t[0].field == 0)
+  assume(t[0].borrowingFunc() == 0)
+  assume(t[0].readCoroutine == 0)
+  // take(t[0]) // FIXME: the move-checker's diagnostics produced for this code are non-deterministic?
+  assume(t[0].consumingGetter == 0) // expected-error {{'t.subscript' is borrowed and cannot be consumed}} expected-note {{consumed here}}
+  // assume(t[0].consumingFunc() == 0) // FIXME: the move-checker's diagnostics produced for this code are non-deterministic?
 }
 func test(p: P) {
   borrow(p.data)
   take(p.data) // expected-error{{'p.data' is borrowed and cannot be consumed}} expected-note{{consumed here}}
+
+  borrow(p[0])
+  assume(p[0].field == 0)
+  assume(p[0].borrowingFunc() == 0)
+  assume(p[0].readCoroutine == 0)
+  // take(p[0]) // FIXME: the move-checker's diagnostics produced for this code are non-deterministic?
+  assume(p[0].consumingGetter == 0) // expected-error {{'p.subscript' is borrowed and cannot be consumed}} expected-note {{consumed here}}
+  // assume(p[0].consumingFunc() == 0) // FIXME: the move-checker's diagnostics produced for this code are non-deterministic?
 }