Sema: Add request to synthesize opaque accessors

For now, this just serves to clean up some code; we still eagerly trigger
synthesis in the same places as before.

Author: slavapestov

include/swift/AST/Decl.h

@@ -4397,6 +4397,7 @@ class AbstractStorageDecl : public ValueDecl {
   friend class StorageImplInfoRequest;
   friend class RequiresOpaqueAccessorsRequest;
   friend class RequiresOpaqueModifyCoroutineRequest;
+  friend class SynthesizeAccessorRequest;
 
 public:
   static const size_t MaxNumAccessors = 255;
@@ -4469,6 +4470,9 @@ class AbstractStorageDecl : public ValueDecl {
   /// The implementation info for the accessors.
   StorageImplInfo ImplInfo;
 
+  /// Add a synthesized accessor.
+  void setSynthesizedAccessor(AccessorKind kind, AccessorDecl *getter);
+
 protected:
   AbstractStorageDecl(DeclKind Kind, bool IsStatic, DeclContext *DC,
                       DeclName Name, SourceLoc NameLoc,
@@ -4582,6 +4586,12 @@ class AbstractStorageDecl : public ValueDecl {
     return {};
   }
 
+  /// Return an accessor that this storage is expected to have, synthesizing
+  /// one if necessary. Note that will always synthesize one, even if the
+  /// accessor is not part of the expected opaque set for the storage, so use
+  /// with caution.
+  AccessorDecl *getSynthesizedAccessor(AccessorKind kind) const;
+
   /// Visit all the opaque accessors that this storage is expected to have.
   void visitExpectedOpaqueAccessors(
                             llvm::function_ref<void (AccessorKind)>) const;
@@ -4597,18 +4607,6 @@ class AbstractStorageDecl : public ValueDecl {
   /// This should only be used by the ClangImporter.
   void setComputedSetter(AccessorDecl *Set);
 
-  /// Add a synthesized getter.
-  void setSynthesizedGetter(AccessorDecl *getter);
-
-  /// Add a synthesized setter.
-  void setSynthesizedSetter(AccessorDecl *setter);
-
-  /// Add a synthesized read coroutine.
-  void setSynthesizedReadCoroutine(AccessorDecl *read);
-
-  /// Add a synthesized modify coroutine.
-  void setSynthesizedModifyCoroutine(AccessorDecl *modify);
-
   /// Does this storage require opaque accessors of any kind?
   bool requiresOpaqueAccessors() const;
 

include/swift/AST/TypeCheckRequests.h

@@ -964,6 +964,29 @@ class IsAccessorTransparentRequest :
   void cacheResult(bool value) const;
 };
 
+class SynthesizeAccessorRequest :
+    public SimpleRequest<SynthesizeAccessorRequest,
+                         AccessorDecl *(AbstractStorageDecl *,
+                                        AccessorKind),
+                         CacheKind::SeparatelyCached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  llvm::Expected<AccessorDecl *>
+  evaluate(Evaluator &evaluator, AbstractStorageDecl *decl,
+           AccessorKind kind) const;
+
+public:
+  // Separate caching.
+  bool isCached() const { return true; }
+  Optional<AccessorDecl *> getCachedResult() const;
+  void cacheResult(AccessorDecl *value) const;
+};
+
 // Allow AnyValue to compare two Type values, even though Type doesn't
 // support ==.
 template<>

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -52,3 +52,4 @@ SWIFT_TYPEID(StorageImplInfoRequest)
 SWIFT_TYPEID(RequiresOpaqueAccessorsRequest)
 SWIFT_TYPEID(RequiresOpaqueModifyCoroutineRequest)
 SWIFT_TYPEID(IsAccessorTransparentRequest)
+SWIFT_TYPEID(SynthesizeAccessorRequest)

lib/AST/Decl.cpp

@@ -1960,6 +1960,16 @@ bool AbstractStorageDecl::requiresOpaqueModifyCoroutine() const {
     false);
 }
 
+AccessorDecl *AbstractStorageDecl::getSynthesizedAccessor(AccessorKind kind) const {
+  if (auto *accessor = getAccessor(kind))
+    return accessor;
+
+  ASTContext &ctx = getASTContext();
+  return evaluateOrDefault(ctx.evaluator,
+    SynthesizeAccessorRequest{const_cast<AbstractStorageDecl *>(this), kind},
+    nullptr);
+}
+
 void AbstractStorageDecl::visitExpectedOpaqueAccessors(
                         llvm::function_ref<void (AccessorKind)> visit) const {
   if (!requiresOpaqueAccessors())
@@ -4789,32 +4799,24 @@ AbstractStorageDecl::getSetterFormalAccess() const {
         AccessLevel::Private);
 }
 
-#ifndef NDEBUG
-static bool isAccessor(AccessorDecl *accessor, AccessorKind kind,
-                       AbstractStorageDecl *storage) {
-  // TODO: this should check that the accessor belongs to this storage, but
-  // the Clang importer currently likes to violate that condition.
-  return (accessor && accessor->getAccessorKind() == kind);
-}
-#endif
-
 void AbstractStorageDecl::setComputedSetter(AccessorDecl *setter) {
   assert(getImplInfo().getReadImpl() == ReadImplKind::Get);
   assert(!getImplInfo().supportsMutation());
-  assert(getGetter() && "sanity check: missing getter");
+  assert(getGetter() && "invariant check: missing getter");
   assert(!getSetter() && "already has a setter");
   assert(hasClangNode() && "should only be used for ObjC properties");
-  assert(isAccessor(setter, AccessorKind::Set, this));
   assert(setter && "should not be called for readonly properties");
+  assert(setter->getAccessorKind() == AccessorKind::Set);
 
   setImplInfo(StorageImplInfo::getMutableComputed());
   Accessors.getPointer()->addOpaqueAccessor(setter);
 }
 
 void
-AbstractStorageDecl::setSynthesizedGetter(AccessorDecl *accessor) {
-  assert(!getGetter() && "declaration doesn't already have getter!");
-  assert(isAccessor(accessor, AccessorKind::Get, this));
+AbstractStorageDecl::setSynthesizedAccessor(AccessorKind kind,
+                                            AccessorDecl *accessor) {
+  assert(!getAccessor(kind) && "accessor already exists");
+  assert(accessor->getAccessorKind() == kind);
 
   auto accessors = Accessors.getPointer();
   if (!accessors) {
@@ -4825,42 +4827,6 @@ AbstractStorageDecl::setSynthesizedGetter(AccessorDecl *accessor) {
   accessors->addOpaqueAccessor(accessor);
 }
 
-void
-AbstractStorageDecl::setSynthesizedReadCoroutine(AccessorDecl *accessor) {
-  assert(!getReadCoroutine() && "already has a read accessor");
-  assert(isAccessor(accessor, AccessorKind::Read, this));
-
-  auto accessors = Accessors.getPointer();
-  if (!accessors) {
-    accessors = AccessorRecord::create(getASTContext(), SourceRange(), {});
-    Accessors.setPointer(accessors);
-  }
-
-  accessors->addOpaqueAccessor(accessor);
-}
-
-void
-AbstractStorageDecl::setSynthesizedSetter(AccessorDecl *accessor) {
-  assert((getGetter() || getReadCoroutine()) &&
-         "declaration doesn't already have getter!");
-  assert(supportsMutation() && "adding setter to immutable storage");
-  assert(isAccessor(accessor, AccessorKind::Set, this));
-
-  Accessors.getPointer()->addOpaqueAccessor(accessor);
-}
-
-void
-AbstractStorageDecl::setSynthesizedModifyCoroutine(AccessorDecl *accessor) {
-  assert((getGetter() || getReadCoroutine()) &&
-         "declaration doesn't already have getter!");
-  assert(getSetter() && "declaration doesn't already have setter!");
-  assert(supportsMutation() && "adding modify to immutable storage");
-  assert(!getModifyCoroutine() && "already has a modify accessor");
-  assert(isAccessor(accessor, AccessorKind::Modify, this));
-
-  Accessors.getPointer()->addOpaqueAccessor(accessor);
-}
-
 static Optional<ObjCSelector>
 getNameFromObjcAttribute(const ObjCAttr *attr, DeclName preferredName) {
   if (!attr)

lib/AST/TypeCheckRequests.cpp

@@ -764,3 +764,24 @@ void IsAccessorTransparentRequest::cacheResult(bool value) const {
     }
   }
 }
+
+//----------------------------------------------------------------------------//
+// SynthesizeAccessorRequest computation.
+//----------------------------------------------------------------------------//
+
+Optional<AccessorDecl *>
+SynthesizeAccessorRequest::getCachedResult() const {
+  auto *storage = std::get<0>(getStorage());
+  auto kind = std::get<1>(getStorage());
+  auto *accessor = storage->getAccessor(kind);
+  if (accessor)
+    return accessor;
+  return None;
+}
+
+void SynthesizeAccessorRequest::cacheResult(AccessorDecl *accessor) const {
+  auto *storage = std::get<0>(getStorage());
+  auto kind = std::get<1>(getStorage());
+
+  storage->setSynthesizedAccessor(kind, accessor);
+}

lib/Sema/CodeSynthesis.cpp

@@ -1266,54 +1266,24 @@ synthesizeModifyCoroutineBody(AccessorDecl *modify, ASTContext &ctx) {
   return synthesizeCoroutineAccessorBody(modify, ctx);
 }
 
-static void addGetterToStorage(AbstractStorageDecl *storage,
-                               ASTContext &ctx) {
-  auto getter = createGetterPrototype(storage, ctx);
-
-  // Install the prototype.
-  storage->setSynthesizedGetter(getter);
-}
-
-static void addSetterToStorage(AbstractStorageDecl *storage,
-                               ASTContext &ctx) {
-  auto setter = createSetterPrototype(storage, ctx);
-
-  // Install the prototype.
-  storage->setSynthesizedSetter(setter);
-}
-
-static void addReadCoroutineToStorage(AbstractStorageDecl *storage,
-                                      ASTContext &ctx) {
-  auto read = createReadCoroutinePrototype(storage, ctx);
-
-  // Install the prototype.
-  storage->setSynthesizedReadCoroutine(read);
-}
-
-static void addModifyCoroutineToStorage(AbstractStorageDecl *storage,
-                                        ASTContext &ctx) {
-  auto modify = createModifyCoroutinePrototype(storage, ctx);
-
-  // Install the prototype.
-  storage->setSynthesizedModifyCoroutine(modify);
-}
-
+llvm::Expected<AccessorDecl *>
+SynthesizeAccessorRequest::evaluate(Evaluator &evaluator,
+                                    AbstractStorageDecl *storage,
+                                    AccessorKind kind) const {
+  auto &ctx = storage->getASTContext();
 
-static void addOpaqueAccessorToStorage(AbstractStorageDecl *storage,
-                                       AccessorKind kind,
-                                       ASTContext &ctx) {
   switch (kind) {
   case AccessorKind::Get:
-    return addGetterToStorage(storage, ctx);
+    return createGetterPrototype(storage, ctx);
 
   case AccessorKind::Set:
-    return addSetterToStorage(storage, ctx);
+    return createSetterPrototype(storage, ctx);
 
   case AccessorKind::Read:
-    return addReadCoroutineToStorage(storage, ctx);
+    return createReadCoroutinePrototype(storage, ctx);
 
   case AccessorKind::Modify:
-    return addModifyCoroutineToStorage(storage, ctx);
+    return createModifyCoroutinePrototype(storage, ctx);
 
 #define OPAQUE_ACCESSOR(ID, KEYWORD)
 #define ACCESSOR(ID) \
@@ -1324,14 +1294,9 @@ static void addOpaqueAccessorToStorage(AbstractStorageDecl *storage,
 }
 
 void swift::addExpectedOpaqueAccessorsToStorage(AbstractStorageDecl *storage) {
-  auto &ctx = storage->getASTContext();
-
   storage->visitExpectedOpaqueAccessors([&](AccessorKind kind) {
-    // If the accessor is already present, there's nothing to do.
-    if (storage->getAccessor(kind))
-      return;
-
-    addOpaqueAccessorToStorage(storage, kind, ctx);
+    // Force synthesis if necessary.
+    (void) storage->getSynthesizedAccessor(kind);
   });
 }
 
@@ -1366,12 +1331,8 @@ void TypeChecker::synthesizeWitnessAccessorsForStorage(
   DeclsToFinalize.insert(requirement);
 
   requirement->visitExpectedOpaqueAccessors([&](AccessorKind kind) {
-    // If the accessor already exists, we have nothing to do.
-    if (storage->getAccessor(kind))
-      return;
-
-    // Otherwise, synthesize it.
-    addOpaqueAccessorToStorage(storage, kind, Context);
+    // Force synthesis if necessary.
+    (void) storage->getSynthesizedAccessor(kind);
   });
 }