Replace materializeForSet with the modify coroutine.

Most of this patch is just removing special cases for materializeForSet
or other fairly mechanical replacements.  Unfortunately, the rest is
still a fairly big change, and not one that can be easily split apart
because of the quite reasonable reliance on metaprogramming throughout
the compiler.  And, of course, there are a bunch of test updates that
have to be sync'ed with the actual change to code-generation.

This is SR-7134.

Author: rjmccall

include/swift/ABI/MetadataValues.h

@@ -322,7 +322,8 @@ class MethodDescriptorFlags {
     Init,
     Getter,
     Setter,
-    MaterializeForSet,
+    ModifyCoroutine,
+    ReadCoroutine,
   };
 
 private:
@@ -542,7 +543,8 @@ class ProtocolRequirementFlags {
     Init,
     Getter,
     Setter,
-    MaterializeForSet,
+    ReadCoroutine,
+    ModifyCoroutine,
     AssociatedTypeAccessFunction,
     AssociatedConformanceAccessFunction,
   };

include/swift/AST/AccessorKinds.def

@@ -68,9 +68,9 @@
 /// COROUTINE_ACCESSOR(ID, KEYWORD)
 ///   The given accessor is a coroutine accessor, i.e. a reader or modifier.
 ///
-///   Defaults to SINGLETON_ACCESSOR(ID, KEYWORD).
+///   Defaults to OPAQUE_ACCESSOR(ID, KEYWORD).
 #ifndef COROUTINE_ACCESSOR
-#define COROUTINE_ACCESSOR(ID, KEYWORD) SINGLETON_ACCESSOR(ID, KEYWORD)
+#define COROUTINE_ACCESSOR(ID, KEYWORD) OPAQUE_ACCESSOR(ID, KEYWORD)
 #endif
 
 /// ANY_ADDRESSOR(ACCESSOR_ID, ADDRESSOR_ID, KEYWORD)
@@ -127,18 +127,21 @@ OBJC_ACCESSOR(Get, get)
 /// always be synthesized if the storage is mutable at all.
 OBJC_ACCESSOR(Set, set)
 
-/// This is a materializeForSet accessor: a function which is called
-/// when a combined read-modify operation is performed on the storage,
-/// such as passing it as an inout argument (which includes calling
-/// a mutating function on it).  It is essentially a SILGen-lowered
-/// coroutine that yields a pointer to a mutable value of the storage
-/// type.
-///
-/// We do not allow users to provide materializeForSet.  It can always
-/// be synthesized if the storage is mutable at all.
-#if !(SUPPRESS_ARTIFICIAL_ACCESSORS)
-OPAQUE_ACCESSOR(MaterializeForSet, materializeForSet)
-#endif
+/// This is a read accessor: a yield-once coroutine which is called when a
+/// value is loaded from the storage, like a getter, but which works
+/// by yielding a borrowed value of the storage type.
+///
+/// If the storage is not implemented with a read accessor then
+/// one can always be synthesized (even if the storage type is move-only).
+COROUTINE_ACCESSOR(Read, _read)
+
+/// This is a modify accessor: a yield-once coroutine which is called when a
+/// the storage is modified which works by yielding an inout value
+/// of the storage type.
+///
+/// If the storage is not implemented with a modify accessor then
+/// one can be synthesized if the storage is mutable at all.
+COROUTINE_ACCESSOR(Modify, _modify)
 
 /// This is a willSet observer: a function which "decorates" an
 /// underlying assignment operation by being called prior to the
@@ -156,24 +159,6 @@ OBSERVING_ACCESSOR(WillSet, willSet)
 /// setter idiom.
 OBSERVING_ACCESSOR(DidSet, didSet)
 
-/// This is a read accessor: a coroutine which is called when a
-/// value is loaded from the storage, like a getter, but which works
-/// by yielding a borrowed value of the storage type.
-///
-/// If the storage is not implemented with a read accessor then
-/// one can always be synthesized (even if the storage type is move-only).
-COROUTINE_ACCESSOR(Read, _read)
-
-/// This is a modify accessor: a coroutine which is called when a
-/// the storage is assigned to (like a setter) or read-modified
-/// (like materializeForSet), but which works by yielding an inout
-/// value of the storage type.
-///
-/// If the storage is not implemented with a modify accessor then
-/// one can be synthesized if the storage is mutable at all.
-/// Modify accessors are intended to eventually replace materializeForSet.
-COROUTINE_ACCESSOR(Modify, _modify)
-
 /// This is an address-family accessor: a function that is called when
 /// a value is loaded from the storage, like a getter, but which works
 /// by returning a pointer to an immutable value of the storage type.
@@ -189,9 +174,8 @@ IMMUTABLE_ADDRESSOR(Owning, addressWithOwner)
 IMMUTABLE_ADDRESSOR(NativeOwning, addressWithNativeOwner)
 
 /// This is a mutableAddress-family accessor: a function that is
-/// called when the storage is assigned to (like a setter) or
-/// read-modified (like materializeForSet), but which works by
-/// returning a pointer to a mutable value of the storage type.
+/// called when the storage is modified and which works by returning
+/// a pointer to a mutable value of the storage type.
 /// This kind of accessor also has an addressor kind.
 ///
 /// Addressors are a way of proving more efficient access to storage

include/swift/AST/AnyFunctionRef.h

@@ -190,7 +190,8 @@ class AnyFunctionRef {
     if (auto *AFD = TheFunction.dyn_cast<AbstractFunctionDecl *>()) {
       if (auto *AD = dyn_cast<AccessorDecl>(AFD)) {
         if (AD->isCoroutine()) {
-          auto valueTy = AD->getStorage()->getValueInterfaceType();
+          auto valueTy = AD->getStorage()->getValueInterfaceType()
+                                         ->getReferenceStorageReferent();
           if (mapIntoContext)
             valueTy = AD->mapTypeIntoContext(valueTy);
           YieldTypeFlags flags(AD->getAccessorKind() == AccessorKind::Modify

include/swift/AST/Decl.h

@@ -147,7 +147,6 @@ enum class DescriptiveDeclKind : uint8_t {
   ClassMethod,
   Getter,
   Setter,
-  MaterializeForSet,
   Addressor,
   MutableAddressor,
   ReadAccessor,
@@ -325,7 +324,7 @@ class alignas(1 << DeclAlignInBits) Decl {
     IsUserAccessible : 1
   );
 
-  SWIFT_INLINE_BITFIELD(AbstractStorageDecl, ValueDecl, 1+1+1+1,
+  SWIFT_INLINE_BITFIELD(AbstractStorageDecl, ValueDecl, 1+1+1+1+2,
     /// Whether the getter is mutating.
     IsGetterMutating : 1,
 
@@ -336,7 +335,10 @@ class alignas(1 << DeclAlignInBits) Decl {
     HasStorage : 1,
 
     /// Whether this storage supports semantic mutation in some way.
-    SupportsMutation : 1
+    SupportsMutation : 1,
+
+    /// Whether an opaque read of this storage produces an owned value.
+    OpaqueReadOwnership : 2
   );
 
   SWIFT_INLINE_BITFIELD(VarDecl, AbstractStorageDecl, 1+4+1+1+1+1,
@@ -4142,6 +4144,8 @@ class AbstractStorageDecl : public ValueDecl {
     Bits.AbstractStorageDecl.SupportsMutation = supportsMutation;
     Bits.AbstractStorageDecl.IsGetterMutating = false;
     Bits.AbstractStorageDecl.IsSetterMutating = true;
+    Bits.AbstractStorageDecl.OpaqueReadOwnership =
+      unsigned(OpaqueReadOwnership::Owned);
   }
 
   void setSupportsMutationIfStillStored(StorageIsMutable_t supportsMutation) {
@@ -4211,7 +4215,15 @@ class AbstractStorageDecl : public ValueDecl {
   bool hasAnyAccessors() const {
     return !getAllAccessors().empty();
   }
-  
+
+  /// \brief Return the ownership of values opaquely read from this storage.
+  OpaqueReadOwnership getOpaqueReadOwnership() const {
+    return OpaqueReadOwnership(Bits.AbstractStorageDecl.OpaqueReadOwnership);
+  }
+  void setOpaqueReadOwnership(OpaqueReadOwnership ownership) {
+    Bits.AbstractStorageDecl.OpaqueReadOwnership = unsigned(ownership);
+  }
+
   /// \brief Return true if reading this storage requires the ability to
   /// modify the base value.
   bool isGetterMutating() const {
@@ -4267,11 +4279,30 @@ class AbstractStorageDecl : public ValueDecl {
   /// \brief Add a synthesized setter.
   void setSynthesizedSetter(AccessorDecl *setter);
 
-  /// \brief Add a synthesized materializeForSet accessor.
-  void setSynthesizedMaterializeForSet(AccessorDecl *materializeForSet);
+  /// \brief Add a synthesized read coroutine.
+  void setSynthesizedReadCoroutine(AccessorDecl *read);
+
+  /// \brief Add a synthesized modify coroutine.
+  void setSynthesizedModifyCoroutine(AccessorDecl *modify);
+
+  /// Does this storage require an opaque accessor of the given kind?
+  bool requiresOpaqueAccessor(AccessorKind kind) const;
 
-  /// Does this storage require a materializeForSet accessor?
-  bool requiresMaterializeForSet() const;
+  /// Does this storage require a 'get' accessor in its opaque-accessors set?
+  bool requiresOpaqueGetter() const {
+    return getOpaqueReadOwnership() != OpaqueReadOwnership::Borrowed;
+  }
+
+  /// Does this storage require a 'read' accessor in its opaque-accessors set?
+  bool requiresOpaqueReadCoroutine() const {
+    return getOpaqueReadOwnership() != OpaqueReadOwnership::Owned;
+  }
+
+  /// Does this storage require a 'set' accessor in its opaque-accessors set?
+  bool requiresOpaqueSetter() const { return supportsMutation(); }
+
+  /// Does this storage require a 'modify' accessor in its opaque-accessors set?
+  bool requiresOpaqueModifyCoroutine() const;
 
   SourceRange getBracesRange() const {
     if (auto info = Accessors.getPointer())
@@ -4298,12 +4329,6 @@ class AbstractStorageDecl : public ValueDecl {
 
   void overwriteSetterAccess(AccessLevel accessLevel);
 
-  /// \brief Retrieve the materializeForSet function, if this
-  /// declaration has one.
-  AccessorDecl *getMaterializeForSetFunc() const {
-    return getAccessor(AccessorKind::MaterializeForSet);
-  }
-
   /// \brief Return the decl for the immutable addressor if it exists.
   AccessorDecl *getAddressor() const {
     return getAccessor(AccessorKind::Address);
@@ -5571,9 +5596,6 @@ class AccessorDecl final : public FuncDecl {
 
   bool isGetter() const { return getAccessorKind() == AccessorKind::Get; }
   bool isSetter() const { return getAccessorKind() == AccessorKind::Set; }
-  bool isMaterializeForSet() const {
-    return getAccessorKind() == AccessorKind::MaterializeForSet;
-  }
   bool isAnyAddressor() const {
     auto kind = getAccessorKind();
     return kind == AccessorKind::Address
@@ -6471,8 +6493,6 @@ AbstractStorageDecl::overwriteSetterAccess(AccessLevel accessLevel) {
   Accessors.setInt(accessLevel);
   if (auto setter = getSetter())
     setter->overwriteAccess(accessLevel);
-  if (auto materializeForSet = getMaterializeForSetFunc())
-    materializeForSet->overwriteAccess(accessLevel);
   if (auto modify = getModifyCoroutine())
     modify->overwriteAccess(accessLevel);
   if (auto mutableAddressor = getMutableAddressor())

include/swift/AST/DiagnosticsParse.def

@@ -262,8 +262,8 @@ ERROR(conflicting_accessor,none,
       (unsigned, StringRef, StringRef))
 NOTE(previous_accessor,none,
      "%select{|previous definition of }1%0 %select{defined |}1here", (StringRef, bool))
-ERROR(expected_accessor_name,none,
-      "expected %select{%error|setter|%error|willSet|didSet}0 parameter name",
+ERROR(expected_accessor_parameter_name,none,
+      "expected %select{setter|willSet|didSet}0 parameter name",
       (unsigned))
 ERROR(expected_rparen_set_name,none,
       "expected ')' after setter parameter name",())

include/swift/AST/DiagnosticsSIL.def

@@ -89,6 +89,10 @@ ERROR(inout_argument_alias,none,
 NOTE(previous_inout_alias,none,
       "previous aliasing argument", ())
 
+ERROR(unimplemented_generator_witnesses,none,
+      "protocol conformance emission for generator coroutines is unimplemented",
+      ())
+
 WARNING(exclusivity_access_required_warn,none,
         "overlapping accesses to %0, but "
         "%select{initialization|read|modification|deinitialization}1 requires "

include/swift/AST/Stmt.h

@@ -237,7 +237,8 @@ class YieldStmt final
 
 public:
   static YieldStmt *create(const ASTContext &ctx, SourceLoc yieldLoc,
-                           SourceLoc lp, ArrayRef<Expr*> yields, SourceLoc rp);
+                           SourceLoc lp, ArrayRef<Expr*> yields, SourceLoc rp,
+                           Optional<bool> implicit = None);
 
   SourceLoc getYieldLoc() const { return YieldLoc; }
   SourceLoc getLParenLoc() const { return LPLoc; }

include/swift/AST/StorageImpl.h

@@ -27,6 +27,18 @@ enum StorageIsMutable_t : bool {
   StorageIsMutable = true
 };
 
+enum class OpaqueReadOwnership : uint8_t {
+  /// An opaque read produces an owned value.
+  Owned,
+
+  /// An opaque read produces a borrowed value.
+  Borrowed,
+
+  /// An opaque read can be either owned or borrowed, depending on the
+  /// preference of the caller.
+  OwnedOrBorrowed
+};
+
 // Note that the values of these enums line up with %select values in
 // diagnostics.
 enum class AccessorKind {
@@ -220,10 +232,6 @@ enum class ReadWriteImplKind {
   /// There's storage.
   Stored,
 
-  /// There's a materializeForSet.  (This is currently only used for opaque
-  /// declarations.)
-  MaterializeForSet,
-
   /// There's a mutable addressor.
   MutableAddress,
 
@@ -289,7 +297,6 @@ class StorageImplInfo {
              readImpl == ReadImplKind::Address ||
              readImpl == ReadImplKind::Read);
       assert(readWriteImpl == ReadWriteImplKind::MaterializeToTemporary ||
-             readWriteImpl == ReadWriteImplKind::MaterializeForSet ||
              readWriteImpl == ReadWriteImplKind::Modify);
       return;
 
@@ -319,20 +326,21 @@ class StorageImplInfo {
                        : ReadWriteImplKind::Immutable };
   }
 
-  static StorageImplInfo getOpaque(StorageIsMutable_t isMutable) {
-    return (isMutable ? getMutableOpaque()
-                      : getImmutableOpaque());
+  static StorageImplInfo getOpaque(StorageIsMutable_t isMutable,
+                                   OpaqueReadOwnership ownership) {
+    return (isMutable ? getMutableOpaque(ownership)
+                      : getImmutableOpaque(ownership));
   }
 
   /// Describe the implementation of a immutable property implemented opaquely.
-  static StorageImplInfo getImmutableOpaque() {
-    return { ReadImplKind::Get };
+  static StorageImplInfo getImmutableOpaque(OpaqueReadOwnership ownership) {
+    return { getOpaqueReadImpl(ownership) };
   }
 
   /// Describe the implementation of a mutable property implemented opaquely.
-  static StorageImplInfo getMutableOpaque() {
-    return { ReadImplKind::Get, WriteImplKind::Set,
-             ReadWriteImplKind::MaterializeForSet };
+  static StorageImplInfo getMutableOpaque(OpaqueReadOwnership ownership) {
+    return { getOpaqueReadImpl(ownership), WriteImplKind::Set,
+             ReadWriteImplKind::Modify };
   }
 
   static StorageImplInfo getComputed(StorageIsMutable_t isMutable) {
@@ -379,6 +387,18 @@ class StorageImplInfo {
   ReadWriteImplKind getReadWriteImpl() const {
     return ReadWriteImplKind(ReadWrite);
   }
+
+private:
+  static ReadImplKind getOpaqueReadImpl(OpaqueReadOwnership ownership) {
+    switch (ownership) {
+    case OpaqueReadOwnership::Owned:
+      return ReadImplKind::Get;
+    case OpaqueReadOwnership::OwnedOrBorrowed:
+    case OpaqueReadOwnership::Borrowed:
+      return ReadImplKind::Read;
+    }
+    llvm_unreachable("bad read-ownership kind");
+  }
 };
 
 } // end namespace swift

include/swift/SIL/SILType.h

@@ -563,7 +563,10 @@ NON_SIL_TYPE(LValue)
 
 CanSILFunctionType getNativeSILFunctionType(
     SILModule &M, Lowering::AbstractionPattern origType,
-    CanAnyFunctionType substType, Optional<SILDeclRef> constant = None,
+    CanAnyFunctionType substType,
+    Optional<SILDeclRef> origConstant = None,
+    Optional<SILDeclRef> constant = None,
+    Optional<SubstitutionMap> reqtSubs = None,
     Optional<ProtocolConformanceRef> witnessMethodConformance = None);
 
 inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, SILType T) {

include/swift/SIL/TypeLowering.h

@@ -807,15 +807,6 @@ class TypeConverter {
 
   SILType getLoweredTypeOfGlobal(VarDecl *var);
 
-  /// The return type of a materializeForSet contains a callback
-  /// whose type cannot be represented in the AST because it is
-  /// a polymorphic function value. This function returns the
-  /// unsubstituted lowered type of this callback.
-  CanSILFunctionType getMaterializeForSetCallbackType(
-      AbstractStorageDecl *storage, CanGenericSignature genericSig,
-      Type selfType, SILFunctionTypeRepresentation rep,
-      Optional<ProtocolConformanceRef> witnessMethodConformance);
-
   /// Return the SILFunctionType for a native function value of the
   /// given type.
   CanSILFunctionType getSILFunctionType(AbstractionPattern origType,

include/swift/Serialization/ModuleFile.h

@@ -574,6 +574,7 @@ class ModuleFile
 
   /// Sets the accessors for \p storage based on \p rawStorageKind.
   void configureStorage(AbstractStorageDecl *storage,
+                        uint8_t rawOpaqueReadOwnership,
                         uint8_t rawReadImpl,
                         uint8_t rawWriteImpl,
                         uint8_t rawReadWriteImpl,