[CoroutineAccessors] If available, provide old ABI

If the feature is enabled, base the requirement for the underscored
accessors on the availability of the non-underscored accessors.  If the
(non-underscored) accessor's was available earlier than the feature,
interpret that to mean that the underscored version was available in
that earlier version, and require the underscored version.  The goal is
to ensure that the ABI is preserved, so long as the simplest migration
is done (namely, deleting the underscores from the old accessors).

For modify2, cache the required-ness in the same way that it is cached
for modify.

Author: nate-chandler

include/swift/AST/Decl.h

@@ -5765,6 +5765,8 @@ class AbstractStorageDecl : public ValueDecl {
     unsigned RequiresOpaqueAccessors : 1;
     unsigned RequiresOpaqueModifyCoroutineComputed : 1;
     unsigned RequiresOpaqueModifyCoroutine : 1;
+    unsigned RequiresOpaqueModify2CoroutineComputed : 1;
+    unsigned RequiresOpaqueModify2Coroutine : 1;
   } LazySemanticInfo = { };
 
   /// The implementation info for the accessors.
@@ -6046,6 +6048,11 @@ class AbstractStorageDecl : public ValueDecl {
   /// set?
   bool requiresOpaqueModify2Coroutine() const;
 
+  /// Given that CoroutineAccessors is enabled, is _read/_modify required for
+  /// ABI stability?
+  bool
+  requiresCorrespondingUnderscoredCoroutineAccessor(AccessorKind kind) const;
+
   /// Does this storage have any explicit observers (willSet or didSet) attached
   /// to it?
   bool hasObservers() const {

include/swift/AST/FeatureAvailability.def

@@ -80,6 +80,8 @@ FEATURE(ClearSensitive,                                 FUTURE)
 FEATURE(UpdatePureObjCClassMetadata,                    FUTURE)
 FEATURE(ValueGenericType,                               FUTURE)
 FEATURE(InitRawStructMetadata2,                         FUTURE)
+// TODO: CoroutineAccessors: Change to correct version number.
+FEATURE(CoroutineAccessors,                             FUTURE)
 
 #undef FEATURE
 #undef FUTURE

include/swift/AST/TypeCheckRequests.h

@@ -1937,19 +1937,19 @@ class RequiresOpaqueAccessorsRequest :
   void cacheResult(bool value) const;
 };
 
-class RequiresOpaqueModifyCoroutineRequest :
-    public SimpleRequest<RequiresOpaqueModifyCoroutineRequest,
-                         bool(AbstractStorageDecl *),
-                         RequestFlags::SeparatelyCached> {
+class RequiresOpaqueModifyCoroutineRequest
+    : public SimpleRequest<RequiresOpaqueModifyCoroutineRequest,
+                           bool(AbstractStorageDecl *, bool isUnderscored),
+                           RequestFlags::SeparatelyCached> {
 public:
   using SimpleRequest::SimpleRequest;
 
 private:
   friend SimpleRequest;
 
   // Evaluation.
-  bool
-  evaluate(Evaluator &evaluator, AbstractStorageDecl *decl) const;
+  bool evaluate(Evaluator &evaluator, AbstractStorageDecl *decl,
+                bool isUnderscored) const;
 
 public:
   // Separate caching.

lib/AST/Decl.cpp

@@ -3018,6 +3018,10 @@ bool AbstractStorageDecl::requiresOpaqueAccessor(AccessorKind kind) const {
 }
 
 bool AbstractStorageDecl::requiresOpaqueReadCoroutine() const {
+  ASTContext &ctx = getASTContext();
+  if (ctx.LangOpts.hasFeature(Feature::CoroutineAccessors))
+    return requiresCorrespondingUnderscoredCoroutineAccessor(
+        AccessorKind::Read2);
   return getOpaqueReadOwnership() != OpaqueReadOwnership::Owned;
 }
 
@@ -3030,19 +3034,20 @@ bool AbstractStorageDecl::requiresOpaqueRead2Coroutine() const {
 
 bool AbstractStorageDecl::requiresOpaqueModifyCoroutine() const {
   ASTContext &ctx = getASTContext();
-  return evaluateOrDefault(ctx.evaluator,
-    RequiresOpaqueModifyCoroutineRequest{const_cast<AbstractStorageDecl *>(this)},
-    false);
+  return evaluateOrDefault(
+      ctx.evaluator,
+      RequiresOpaqueModifyCoroutineRequest{
+          const_cast<AbstractStorageDecl *>(this), /*isUnderscored=*/true},
+      false);
 }
 
 bool AbstractStorageDecl::requiresOpaqueModify2Coroutine() const {
   ASTContext &ctx = getASTContext();
-  if (!ctx.LangOpts.hasFeature(Feature::CoroutineAccessors))
-    return false;
-  return evaluateOrDefault(ctx.evaluator,
-                           RequiresOpaqueModifyCoroutineRequest{
-                               const_cast<AbstractStorageDecl *>(this)},
-                           false);
+  return evaluateOrDefault(
+      ctx.evaluator,
+      RequiresOpaqueModifyCoroutineRequest{
+          const_cast<AbstractStorageDecl *>(this), /*isUnderscored=*/false},
+      false);
 }
 
 AccessorDecl *AbstractStorageDecl::getSynthesizedAccessor(AccessorKind kind) const {

lib/AST/TypeCheckRequests.cpp

@@ -795,15 +795,29 @@ void RequiresOpaqueAccessorsRequest::cacheResult(bool value) const {
 std::optional<bool>
 RequiresOpaqueModifyCoroutineRequest::getCachedResult() const {
   auto *storage = std::get<0>(getStorage());
-  if (storage->LazySemanticInfo.RequiresOpaqueModifyCoroutineComputed)
-    return static_cast<bool>(storage->LazySemanticInfo.RequiresOpaqueModifyCoroutine);
+  auto isUnderscored = std::get<1>(getStorage());
+  if (isUnderscored) {
+    if (storage->LazySemanticInfo.RequiresOpaqueModifyCoroutineComputed)
+      return static_cast<bool>(
+          storage->LazySemanticInfo.RequiresOpaqueModifyCoroutine);
+  } else {
+    if (storage->LazySemanticInfo.RequiresOpaqueModify2CoroutineComputed)
+      return static_cast<bool>(
+          storage->LazySemanticInfo.RequiresOpaqueModify2Coroutine);
+  }
   return std::nullopt;
 }
 
 void RequiresOpaqueModifyCoroutineRequest::cacheResult(bool value) const {
   auto *storage = std::get<0>(getStorage());
-  storage->LazySemanticInfo.RequiresOpaqueModifyCoroutineComputed = 1;
-  storage->LazySemanticInfo.RequiresOpaqueModifyCoroutine = value;
+  auto isUnderscored = std::get<1>(getStorage());
+  if (isUnderscored) {
+    storage->LazySemanticInfo.RequiresOpaqueModifyCoroutineComputed = 1;
+    storage->LazySemanticInfo.RequiresOpaqueModifyCoroutine = value;
+  } else {
+    storage->LazySemanticInfo.RequiresOpaqueModify2CoroutineComputed = 1;
+    storage->LazySemanticInfo.RequiresOpaqueModify2Coroutine = value;
+  }
 }
 
 //----------------------------------------------------------------------------//

lib/Sema/TypeCheckStorage.cpp

@@ -2710,9 +2710,67 @@ RequiresOpaqueAccessorsRequest::evaluate(Evaluator &evaluator,
   return true;
 }
 
-bool
-RequiresOpaqueModifyCoroutineRequest::evaluate(Evaluator &evaluator,
-                                               AbstractStorageDecl *storage) const {
+/// When the CoroutineAccessors feature is available, the coroutine accessor
+/// _could_ be required.  That non-underscored accessor would be preferred to
+/// its underscored counterpart accessor.
+///
+/// The underscored accessor could, however, still be required for ABI
+/// stability.
+bool AbstractStorageDecl::requiresCorrespondingUnderscoredCoroutineAccessor(
+    AccessorKind kind) const {
+  auto &ctx = getASTContext();
+  assert(ctx.LangOpts.hasFeature(Feature::CoroutineAccessors));
+  assert(kind == AccessorKind::Modify2 || kind == AccessorKind::Read2);
+
+  // Non-stable modules have no ABI to keep stable.
+  if (getModuleContext()->getResilienceStrategy() !=
+      ResilienceStrategy::Resilient)
+    return false;
+
+  // Non-exported storage has no ABI to keep stable.
+  if (!isExported(this))
+    return false;
+
+  // The the non-underscored accessor is not present, the underscored accessor
+  // won't be either.
+  // TODO: CoroutineAccessors: What if only the underscored is written out?
+  auto *accessor = getOpaqueAccessor(kind);
+  if (!accessor)
+    return false;
+
+  // Availability checks are only relevant on targets which support versioned
+  // availability.  Otherwise, since we're building with library evolution,
+  // conservatively assume that the binary must keep ABI compatibility with its
+  // prior versions, and emit the underscored variant.
+  if (!ctx.supportsVersionedAvailability())
+    return true;
+
+  auto modifyAvailability = TypeChecker::availabilityAtLocation({}, accessor);
+  auto featureAvailability = ctx.getCoroutineAccessorsRuntimeAvailability();
+  // If accessor was introduced only after the feature was, there's no old ABI
+  // to maintain.
+  if (modifyAvailability.getPlatformRange().isContainedIn(featureAvailability))
+    return false;
+
+  // The underscored accessor is required for ABI stability.
+  return true;
+}
+
+bool RequiresOpaqueModifyCoroutineRequest::evaluate(
+    Evaluator &evaluator, AbstractStorageDecl *storage,
+    bool isUnderscored) const {
+  auto &ctx = storage->getASTContext();
+  bool hasModifyFeature = ctx.LangOpts.hasFeature(Feature::CoroutineAccessors);
+
+  // No `modify` accessor without the feature.
+  if (!hasModifyFeature && !isUnderscored)
+    return false;
+
+  if (hasModifyFeature && isUnderscored) {
+    return storage->requiresCorrespondingUnderscoredCoroutineAccessor(
+        AccessorKind::Modify2);
+  }
+
   // Only for mutable storage.
   if (!storage->supportsMutation())
     return false;