[CoroutineAccessors] Unwind based on feature.

Now that coroutine kind (and consequently ABI) for the accessors is
keyed off a SIL option, it's no longer possible to read whether a given
SILFunction arose from a read/modify coroutine just by checking its
coroutine kind.  Regardless of ABI, read/modify coroutines may only
unwind (i.e. are only permitted not to "run to completion") if the
relevant experimental (soon to be deleted) feature is enabled.

Author: nate-chandler

lib/SILGen/SILGenApply.cpp

@@ -4999,14 +4999,16 @@ class CallEmission {
   FormalEvaluationScope initialWritebackScope;
   std::optional<ActorIsolation> implicitActorHopTarget;
   bool implicitlyThrows;
+  bool canUnwind;
 
 public:
   /// Create an emission for a call of the given callee.
   CallEmission(SILGenFunction &SGF, Callee &&callee,
                FormalEvaluationScope &&writebackScope)
       : SGF(SGF), callee(std::move(callee)),
         initialWritebackScope(std::move(writebackScope)),
-        implicitActorHopTarget(std::nullopt), implicitlyThrows(false) {}
+        implicitActorHopTarget(std::nullopt), implicitlyThrows(false),
+        canUnwind(false) {}
 
   /// A factory method for decomposing the apply expr \p e into a call
   /// emission.
@@ -5057,6 +5059,8 @@ class CallEmission {
   /// function can throw or not.
   void setImplicitlyThrows(bool flag) { implicitlyThrows = flag; }
 
+  void setCanUnwind(bool flag) { canUnwind = flag; }
+
   CleanupHandle applyCoroutine(SmallVectorImpl<ManagedValue> &yields);
 
   RValue apply(SGFContext C = SGFContext()) {
@@ -5114,9 +5118,11 @@ namespace {
 /// Cleanup to end a coroutine application.
 class EndCoroutineApply : public Cleanup {
   SILValue ApplyToken;
+  bool CanUnwind;
   std::vector<BeginBorrowInst *> BorrowedMoveOnlyValues;
 public:
-  EndCoroutineApply(SILValue applyToken) : ApplyToken(applyToken) {}
+  EndCoroutineApply(SILValue applyToken, bool CanUnwind)
+      : ApplyToken(applyToken), CanUnwind(CanUnwind) {}
 
   void setBorrowedMoveOnlyValues(ArrayRef<BeginBorrowInst *> values) {
     BorrowedMoveOnlyValues.insert(BorrowedMoveOnlyValues.end(),
@@ -5129,14 +5135,7 @@ class EndCoroutineApply : public Cleanup {
       SGF.B.createEndBorrow(l, *i);
       SGF.B.createDestroyValue(l, (*i)->getOperand());
     }
-    auto *beginApply =
-        cast<BeginApplyInst>(ApplyToken->getDefiningInstruction());
-    auto isCalleeAllocated = beginApply->isCalleeAllocated();
-    auto unwindOnCallerError =
-        !isCalleeAllocated ||
-        SGF.SGM.getASTContext().LangOpts.hasFeature(
-            Feature::CoroutineAccessorsUnwindOnCallerError);
-    if (forUnwind && unwindOnCallerError) {
+    if (forUnwind && CanUnwind) {
       SGF.B.createAbortApply(l, ApplyToken);
     } else {
       SGF.B.createEndApply(l, ApplyToken,
@@ -5179,18 +5178,15 @@ CallEmission::applyCoroutine(SmallVectorImpl<ManagedValue> &yields) {
 
   auto fnValue = callee.getFnValue(SGF, borrowedSelf);
 
-  return SGF.emitBeginApply(uncurriedLoc.value(), fnValue,
+  return SGF.emitBeginApply(uncurriedLoc.value(), fnValue, canUnwind,
                             callee.getSubstitutions(), uncurriedArgs,
                             calleeTypeInfo.substFnType, options, yields);
 }
 
-CleanupHandle
-SILGenFunction::emitBeginApply(SILLocation loc, ManagedValue fn,
-                               SubstitutionMap subs,
-                               ArrayRef<ManagedValue> args,
-                               CanSILFunctionType substFnType,
-                               ApplyOptions options,
-                               SmallVectorImpl<ManagedValue> &yields) {
+CleanupHandle SILGenFunction::emitBeginApply(
+    SILLocation loc, ManagedValue fn, bool canUnwind, SubstitutionMap subs,
+    ArrayRef<ManagedValue> args, CanSILFunctionType substFnType,
+    ApplyOptions options, SmallVectorImpl<ManagedValue> &yields) {
   // Emit the call.
   SmallVector<SILValue, 4> rawResults;
   emitRawApply(*this, loc, fn, subs, args, substFnType, options,
@@ -5206,7 +5202,7 @@ SILGenFunction::emitBeginApply(SILLocation loc, ManagedValue fn,
 
   // Push a cleanup to end the application.
   // TODO: destroy all the arguments at exactly this point?
-  Cleanups.pushCleanup<EndCoroutineApply>(token);
+  Cleanups.pushCleanup<EndCoroutineApply>(token, canUnwind);
   auto endApplyHandle = getTopCleanup();
 
   // Manage all the yielded values.
@@ -6183,7 +6179,7 @@ SILValue SILGenFunction::emitApplyWithRethrow(SILLocation loc, SILValue fn,
 std::tuple<MultipleValueInstructionResult *, CleanupHandle, SILValue,
            CleanupHandle>
 SILGenFunction::emitBeginApplyWithRethrow(SILLocation loc, SILValue fn,
-                                          SILType substFnType,
+                                          SILType substFnType, bool canUnwind,
                                           SubstitutionMap subs,
                                           ArrayRef<SILValue> args,
                                           SmallVectorImpl<SILValue> &yields) {
@@ -6208,7 +6204,7 @@ SILGenFunction::emitBeginApplyWithRethrow(SILLocation loc, SILValue fn,
     deallocCleanup = enterDeallocStackCleanup(allocation);
   }
 
-  Cleanups.pushCleanup<EndCoroutineApply>(token);
+  Cleanups.pushCleanup<EndCoroutineApply>(token, canUnwind);
   auto abortCleanup = Cleanups.getTopCleanup();
 
   return {token, abortCleanup, allocation, deallocCleanup};
@@ -7561,6 +7557,21 @@ ManagedValue SILGenFunction::emitAddressorAccessor(
   return ManagedValue::forLValue(address);
 }
 
+bool SILGenFunction::canUnwindAccessorDeclRef(SILDeclRef accessorRef) {
+  auto *accessor =
+      dyn_cast_or_null<AccessorDecl>(accessorRef.getAbstractFunctionDecl());
+  ASSERT(accessor && "only accessors can unwind");
+  auto kind = accessor->getAccessorKind();
+  ASSERT(isYieldingAccessor(kind) && "only yielding accessors can unwind");
+  if (!requiresFeatureCoroutineAccessors(kind)) {
+    // _read and _modify can unwind
+    return true;
+  }
+  // Coroutine accessors can only unwind with the experimental feature.
+  return getASTContext().LangOpts.hasFeature(
+      Feature::CoroutineAccessorsUnwindOnCallerError);
+}
+
 CleanupHandle
 SILGenFunction::emitCoroutineAccessor(SILLocation loc, SILDeclRef accessor,
                                       SubstitutionMap substitutions,
@@ -7596,6 +7607,8 @@ SILGenFunction::emitCoroutineAccessor(SILLocation loc, SILDeclRef accessor,
 
   emission.addCallSite(loc, std::move(subscriptIndices));
 
+  emission.setCanUnwind(canUnwindAccessorDeclRef(accessor));
+
   auto endApplyHandle = emission.applyCoroutine(yields);
 
   return endApplyHandle;

lib/SILGen/SILGenFunction.h

@@ -2034,6 +2034,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
       PreparedArguments &&optionalSubscripts,
       SILType addressType, bool isOnSelfParameter);
 
+  bool canUnwindAccessorDeclRef(SILDeclRef accessorRef);
   CleanupHandle emitCoroutineAccessor(SILLocation loc, SILDeclRef accessor,
                                       SubstitutionMap substitutions,
                                       ArgumentSource &&optionalSelfValue,
@@ -2301,8 +2302,9 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
                                         PreparedArguments &&args, Type overriddenSelfType,
                                         SGFContext ctx);
 
-  CleanupHandle emitBeginApply(SILLocation loc, ManagedValue fn,
-                               SubstitutionMap subs, ArrayRef<ManagedValue> args,
+  CleanupHandle emitBeginApply(SILLocation loc, ManagedValue fn, bool canUnwind,
+                               SubstitutionMap subs,
+                               ArrayRef<ManagedValue> args,
                                CanSILFunctionType substFnType,
                                ApplyOptions options,
                                SmallVectorImpl<ManagedValue> &yields);
@@ -2315,7 +2317,8 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   std::tuple<MultipleValueInstructionResult *, CleanupHandle, SILValue,
              CleanupHandle>
   emitBeginApplyWithRethrow(SILLocation loc, SILValue fn, SILType substFnType,
-                            SubstitutionMap subs, ArrayRef<SILValue> args,
+                            bool canUnwind, SubstitutionMap subs,
+                            ArrayRef<SILValue> args,
                             SmallVectorImpl<SILValue> &yields);
   void emitEndApplyWithRethrow(SILLocation loc,
                                MultipleValueInstructionResult *token,

lib/SILGen/SILGenLValue.cpp

@@ -2518,9 +2518,9 @@ namespace {
 
       // Perform the begin_apply.
       SmallVector<ManagedValue, 1> yields;
-      auto cleanup =
-        SGF.emitBeginApply(loc, projectFnRef, subs, { base, keyPathValue },
-                           substFnType, ApplyOptions(), yields);
+      auto cleanup = SGF.emitBeginApply(loc, projectFnRef, /*canUnwind=*/true,
+                                        subs, {base, keyPathValue}, substFnType,
+                                        ApplyOptions(), yields);
 
       // Push an operation to do the end_apply.
       pushEndApplyWriteback(SGF, loc, cleanup, getTypeData());

lib/SILGen/SILGenPoly.cpp

@@ -7046,8 +7046,8 @@ SILGenFunction::emitVTableThunk(SILDeclRef base,
   case SILCoroutineKind::YieldOnce2: {
     SmallVector<SILValue, 4> derivedYields;
     auto tokenAndCleanups = emitBeginApplyWithRethrow(
-        loc, derivedRef, SILType::getPrimitiveObjectType(derivedFTy), subs,
-        args, derivedYields);
+        loc, derivedRef, SILType::getPrimitiveObjectType(derivedFTy),
+        canUnwindAccessorDeclRef(base), subs, args, derivedYields);
     auto token = std::get<0>(tokenAndCleanups);
     auto abortCleanup = std::get<1>(tokenAndCleanups);
     auto allocation = std::get<2>(tokenAndCleanups);
@@ -7441,7 +7441,8 @@ void SILGenFunction::emitProtocolWitness(
   case SILCoroutineKind::YieldOnce2: {
     SmallVector<SILValue, 4> witnessYields;
     auto tokenAndCleanups = emitBeginApplyWithRethrow(
-        loc, witnessFnRef, witnessSILTy, witnessSubs, args, witnessYields);
+        loc, witnessFnRef, witnessSILTy, canUnwindAccessorDeclRef(requirement),
+        witnessSubs, args, witnessYields);
     auto token = std::get<0>(tokenAndCleanups);
     auto abortCleanup = std::get<1>(tokenAndCleanups);
     auto allocation = std::get<2>(tokenAndCleanups);

test/Interpreter/coroutine_accessors_old_abi_nounwind.swift

@@ -0,0 +1,41 @@
+// RUN: %target-run-simple-swift(-Xfrontend -disable-callee-allocated-coro-abi -enable-experimental-feature CoroutineAccessors) | %FileCheck %s
+
+// REQUIRES: swift_feature_CoroutineAccessors
+// REQUIRES: executable_test
+
+struct AirOr : Error {
+}
+
+struct Thrower {
+  mutating func doit() throws {
+    throw AirOr()
+  }
+}
+
+struct S {
+  var _thrower = Thrower()
+  var thrower: Thrower {
+    read {
+      yield _thrower
+    }
+    modify {
+      // CHECK: first
+      print("first")
+      yield &_thrower
+      // CHECK: also ran
+      print("also ran")
+    }
+  }
+}
+
+func doit() {
+  do {
+    var s = S()
+    try s.thrower.doit()
+  } catch let error {
+    // CHECK: AirOr
+    print(error)
+  }
+}
+
+doit()

test/SILGen/coroutine_accessors_new_abi.swift

@@ -0,0 +1,22 @@
+// RUN: %target-swift-emit-silgen -Xllvm -sil-print-types   \
+// RUN:     %s                                              \
+// RUN:     -enable-callee-allocated-coro-abi               \
+// RUN:     -enable-library-evolution                       \
+// RUN:     -enable-experimental-feature CoroutineAccessors \
+// RUN: | %FileCheck %s --check-prefixes=CHECK,CHECK-NORMAL
+
+// REQUIRES: swift_feature_CoroutineAccessors
+
+// CHECK: yield_once_2
+
+struct S {
+
+var one: Int = 1
+var i: Int {
+  read {
+    yield one
+  }
+}
+
+}
+

test/SILGen/coroutine_accessors_old_abi.swift

@@ -0,0 +1,22 @@
+// RUN: %target-swift-emit-silgen -Xllvm -sil-print-types   \
+// RUN:     %s                                              \
+// RUN:     -disable-callee-allocated-coro-abi              \
+// RUN:     -enable-library-evolution                       \
+// RUN:     -enable-experimental-feature CoroutineAccessors \
+// RUN: | %FileCheck %s --check-prefixes=CHECK,CHECK-NORMAL
+
+// REQUIRES: swift_feature_CoroutineAccessors
+
+// CHECK: yield_once
+// CHECK-NOT: yield_once_2
+
+struct S {
+
+var zero: Int = 0
+var i: Int {
+  read {
+    yield zero
+  }
+}
+
+}