Merge pull request #26326 from jckarter/property-wrapper-composition-mutatiness

Sema: Correct composition of property wrappers.

Author: jckarter

include/swift/AST/ASTTypeIDZone.def

@@ -22,5 +22,6 @@ SWIFT_TYPEID_NAMED(Decl *, Decl)
 SWIFT_TYPEID(Type)
 SWIFT_TYPEID(PropertyWrapperBackingPropertyInfo)
 SWIFT_TYPEID(PropertyWrapperTypeInfo)
+SWIFT_TYPEID_NAMED(Optional<PropertyWrapperMutability>, PropertyWrapperMutability)
 SWIFT_TYPEID_NAMED(CustomAttr *, CustomAttr)
 SWIFT_TYPEID_NAMED(TypeAliasDecl *, TypeAliasDecl)

include/swift/AST/ASTTypeIDs.h

@@ -24,6 +24,7 @@ class CustomAttr;
 class NominalTypeDecl;
 struct PropertyWrapperBackingPropertyInfo;
 struct PropertyWrapperTypeInfo;
+struct PropertyWrapperMutability;
 class Type;
 class VarDecl;
 class TypeAliasDecl;

include/swift/AST/Decl.h

@@ -79,6 +79,7 @@ namespace swift {
   struct PrintOptions;
   struct PropertyWrapperBackingPropertyInfo;
   struct PropertyWrapperTypeInfo;
+  struct PropertyWrapperMutability;
   class ProtocolDecl;
   class ProtocolType;
   struct RawComment;
@@ -5065,6 +5066,11 @@ class VarDecl : public AbstractStorageDecl {
   PropertyWrapperBackingPropertyInfo
       getPropertyWrapperBackingPropertyInfo() const;
 
+  /// Retrieve information about the mutability of the composed
+  /// property wrappers.
+  Optional<PropertyWrapperMutability>
+      getPropertyWrapperMutability() const;
+
   /// Retrieve the backing storage property for a property that has an
   /// attached property wrapper.
   ///

include/swift/AST/DiagnosticsSema.def

@@ -4481,8 +4481,9 @@ ERROR(property_wrapper_attribute_not_on_property, none,
 NOTE(property_wrapper_declared_here,none,
      "property wrapper type %0 declared here", (DeclName))
 
-ERROR(property_wrapper_composition_not_implemented, none,
-      "multiple property wrappers are not supported", ())
+ERROR(property_wrapper_mutating_get_composed_to_get_only,none,
+      "property wrapper %0 with a mutating getter cannot be composed inside "
+      "get-only property wrapper %1", (Type, Type))
 
 ERROR(property_wrapper_local,none,
       "property wrappers are not yet supported on local properties", ())

include/swift/AST/PropertyWrappers.h

@@ -77,6 +77,47 @@ struct PropertyWrapperTypeInfo {
   }
 };
 
+/// Describes the mutability of the operations on a property wrapper or composition.
+struct PropertyWrapperMutability {
+  enum Value: uint8_t {
+    Nonmutating = 0,
+    Mutating = 1,
+    DoesntExist = 2,
+  };
+  
+  Value Getter, Setter;
+  
+  /// Get the mutability of a composed access chained after accessing a wrapper with `this`
+  /// getter and setter mutability.
+  Value composeWith(Value x) {
+    switch (x) {
+    case DoesntExist:
+      return DoesntExist;
+    
+    // If an operation is nonmutating, then its input relies only on the
+    // mutating-ness of the outer wrapper's get operation.
+    case Nonmutating:
+      return Getter;
+        
+    // If it's mutating, then it relies
+    // on a) the outer wrapper having a setter to exist at all, and b) the
+    // mutating-ness of either the getter or setter, since we need both to
+    // perform a writeback cycle.
+    case Mutating:
+      if (Setter == DoesntExist) {
+        return DoesntExist;
+      }
+      return std::max(Getter, Setter);
+    }
+  }
+  
+  bool operator==(PropertyWrapperMutability other) const {
+    return Getter == other.Getter && Setter == other.Setter;
+  }
+};
+
+void simple_display(llvm::raw_ostream &os, PropertyWrapperMutability m);
+
 /// Describes the backing property of a property that has an attached wrapper.
 struct PropertyWrapperBackingPropertyInfo {
   /// The backing property.

include/swift/AST/TypeCheckRequests.h

@@ -32,6 +32,7 @@ namespace swift {
 class AbstractStorageDecl;
 class GenericParamList;
 struct PropertyWrapperBackingPropertyInfo;
+struct PropertyWrapperMutability;
 class RequirementRepr;
 class SpecializeAttr;
 class TypeAliasDecl;
@@ -579,6 +580,26 @@ class PropertyWrapperBackingPropertyTypeRequest :
   bool isCached() const;
 };
 
+/// Request information about the mutability of composed property wrappers.
+class PropertyWrapperMutabilityRequest :
+    public SimpleRequest<PropertyWrapperMutabilityRequest,
+                         Optional<PropertyWrapperMutability> (VarDecl *),
+                         CacheKind::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  llvm::Expected<Optional<PropertyWrapperMutability>>
+  evaluate(Evaluator &evaluator, VarDecl *var) const;
+
+public:
+  // Caching
+  bool isCached() const;
+};
+
 /// Request information about the backing property for properties that have
 /// attached property wrappers.
 class PropertyWrapperBackingPropertyInfoRequest :

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -32,6 +32,7 @@ SWIFT_TYPEID(StructuralTypeRequest)
 SWIFT_TYPEID(DefaultTypeRequest)
 SWIFT_TYPEID(MangleLocalTypeDeclRequest)
 SWIFT_TYPEID(PropertyWrapperTypeInfoRequest)
+SWIFT_TYPEID(PropertyWrapperMutabilityRequest)
 SWIFT_TYPEID(AttachedPropertyWrappersRequest)
 SWIFT_TYPEID(AttachedPropertyWrapperTypeRequest)
 SWIFT_TYPEID(PropertyWrapperBackingPropertyTypeRequest)
@@ -46,4 +47,4 @@ SWIFT_TYPEID(LazyStoragePropertyRequest)
 SWIFT_TYPEID(TypeCheckFunctionBodyUntilRequest)
 SWIFT_TYPEID(StoredPropertiesRequest)
 SWIFT_TYPEID(StoredPropertiesAndMissingMembersRequest)
-SWIFT_TYPEID(StorageImplInfoRequest)
+SWIFT_TYPEID(StorageImplInfoRequest)

include/swift/Basic/AnyValue.h

@@ -165,6 +165,14 @@ namespace llvm {
   bool operator!=(const TinyPtrVector<T> &lhs, const TinyPtrVector<T> &rhs) {
     return !(lhs == rhs);
   }
+
+  template<typename T>
+  void simple_display(raw_ostream &out, const Optional<T> &opt) {
+    if (opt) {
+      simple_display(out, *opt);
+    }
+    out << "None";
+  }
 } // end namespace llvm
 
 #endif //

lib/AST/ASTContext.cpp

@@ -3611,10 +3611,11 @@ OpaqueTypeArchetypeType::get(OpaqueTypeDecl *Decl,
     }
   }
 #else
-  // Assert that there are no same type constraints on the underlying type.
+  // Assert that there are no same type constraints on the underlying type
   // or its associated types.
   //
-  // This should not be possible until we add where clause support.
+  // This should not be possible until we add where clause support, with the
+  // exception of generic base class constraints (handled below).
 # ifndef NDEBUG
   for (auto reqt :
                 Decl->getOpaqueInterfaceGenericSignature()->getRequirements()) {

lib/AST/Decl.cpp

@@ -5488,6 +5488,16 @@ VarDecl::getPropertyWrapperBackingPropertyInfo() const {
       PropertyWrapperBackingPropertyInfo());
 }
 
+Optional<PropertyWrapperMutability>
+VarDecl::getPropertyWrapperMutability() const {
+  auto &ctx = getASTContext();
+  auto mutableThis = const_cast<VarDecl *>(this);
+  return evaluateOrDefault(
+      ctx.evaluator,
+      PropertyWrapperMutabilityRequest{mutableThis},
+      None);
+}
+
 VarDecl *VarDecl::getPropertyWrapperBackingProperty() const {
   return getPropertyWrapperBackingPropertyInfo().backingVar;
 }

lib/AST/TypeCheckRequests.cpp

@@ -563,6 +563,11 @@ bool PropertyWrapperBackingPropertyInfoRequest::isCached() const {
   return !var->getAttrs().isEmpty();
 }
 
+bool PropertyWrapperMutabilityRequest::isCached() const {
+  auto var = std::get<0>(getStorage());
+  return !var->getAttrs().isEmpty();
+}
+
 void swift::simple_display(
     llvm::raw_ostream &out, const PropertyWrapperTypeInfo &propertyWrapper) {
   out << "{ ";
@@ -587,6 +592,14 @@ void swift::simple_display(
   out << " }";
 }
 
+void swift::simple_display(llvm::raw_ostream &os, PropertyWrapperMutability m) {
+  static const char *names[] =
+    {"is nonmutating", "is mutating", "doesn't exist"};
+  
+  os << "getter " << names[m.Getter] << ", setter " << names[m.Setter];
+}
+
+
 //----------------------------------------------------------------------------//
 // FunctionBuilder-related requests.
 //----------------------------------------------------------------------------//

lib/Sema/CodeSynthesis.cpp

@@ -1835,6 +1835,76 @@ static void typeCheckSynthesizedWrapperInitializer(
   tc.checkPropertyWrapperErrorHandling(pbd, initializer);
 }
 
+static PropertyWrapperMutability::Value
+getGetterMutatingness(VarDecl *var) {
+  return var->isGetterMutating()
+    ? PropertyWrapperMutability::Mutating
+    : PropertyWrapperMutability::Nonmutating;
+}
+
+static PropertyWrapperMutability::Value
+getSetterMutatingness(VarDecl *var, DeclContext *dc) {
+  if (!var->isSettable(nullptr) ||
+      !var->isSetterAccessibleFrom(dc))
+    return PropertyWrapperMutability::DoesntExist;
+  
+  return var->isSetterMutating()
+    ? PropertyWrapperMutability::Mutating
+    : PropertyWrapperMutability::Nonmutating;
+}
+
+llvm::Expected<Optional<PropertyWrapperMutability>>
+PropertyWrapperMutabilityRequest::evaluate(Evaluator &,
+                                           VarDecl *var) const {
+  unsigned numWrappers = var->getAttachedPropertyWrappers().size();
+  if (numWrappers < 1)
+    return None;
+  if (var->getGetter() && !var->getGetter()->isImplicit())
+    return None;
+  if (var->getSetter() && !var->getSetter()->isImplicit())
+    return None;
+
+  // Start with the traits from the outermost wrapper.
+  auto firstWrapper = var->getAttachedPropertyWrapperTypeInfo(0);
+  if (!firstWrapper.valueVar)
+    return None;
+  
+  PropertyWrapperMutability result;
+  
+  result.Getter = getGetterMutatingness(firstWrapper.valueVar);
+  result.Setter = getSetterMutatingness(firstWrapper.valueVar,
+                                        var->getInnermostDeclContext());
+  
+  // Compose the traits of the following wrappers.
+  for (unsigned i = 1; i < numWrappers; ++i) {
+    auto wrapper = var->getAttachedPropertyWrapperTypeInfo(i);
+    if (!wrapper.valueVar)
+      return None;
+    
+    PropertyWrapperMutability nextResult;
+    nextResult.Getter =
+                    result.composeWith(getGetterMutatingness(wrapper.valueVar));
+    // A property must have a getter, so we can't compose a wrapper that
+    // exposes a mutating getter wrapped inside a get-only wrapper.
+    if (nextResult.Getter == PropertyWrapperMutability::DoesntExist) {
+      auto &ctx = var->getASTContext();
+      ctx.Diags.diagnose(var->getAttachedPropertyWrappers()[i]->getLocation(),
+               diag::property_wrapper_mutating_get_composed_to_get_only,
+               var->getAttachedPropertyWrappers()[i]->getTypeLoc().getType(),
+               var->getAttachedPropertyWrappers()[i-1]->getTypeLoc().getType());
+
+      return None;
+    }
+    nextResult.Setter =
+              result.composeWith(getSetterMutatingness(wrapper.valueVar,
+                                               var->getInnermostDeclContext()));
+    result = nextResult;
+  }
+  assert(result.Getter != PropertyWrapperMutability::DoesntExist
+         && "getter must exist");
+  return result;
+}
+
 llvm::Expected<PropertyWrapperBackingPropertyInfo>
 PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
                                                      VarDecl *var) const {
@@ -1954,7 +2024,7 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
     storageVar = synthesizePropertyWrapperStorageWrapperProperty(
         ctx, var, storageInterfaceType, wrapperInfo.projectedValueVar);
   }
-
+  
   // Get the property wrapper information.
   if (!var->allAttachedPropertyWrappersHaveInitialValueInit() &&
       !originalInitialValue) {
@@ -1972,7 +2042,7 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
       /*ignoreAttributeArgs=*/!originalInitialValue);
   typeCheckSynthesizedWrapperInitializer(
       pbd, backingVar, parentPBD, initializer);
-
+  
   return PropertyWrapperBackingPropertyInfo(
       backingVar, storageVar, originalInitialValue, initializer, origValue);
 }
@@ -2115,25 +2185,6 @@ static void finishLazyVariableImplInfo(VarDecl *var,
     info = StorageImplInfo::getMutableComputed();
 }
 
-/// Determine whether all of the wrapped-value setters for the property
-/// wrappers attached to this variable are available and accessible.
-static bool allPropertyWrapperValueSettersAreAccessible(VarDecl *var) {
-  auto wrapperAttrs = var->getAttachedPropertyWrappers();
-  auto innermostDC = var->getInnermostDeclContext();
-  for (unsigned i : indices(wrapperAttrs)) {
-    auto wrapperInfo = var->getAttachedPropertyWrapperTypeInfo(i);
-    auto valueVar = wrapperInfo.valueVar;
-    // Only nullptr with invalid code.
-    if (!valueVar)
-      return false;
-    if (!valueVar->isSettable(nullptr) ||
-        !valueVar->isSetterAccessibleFrom(innermostDC))
-      return false;
-  }
-  
-  return true;
-}
-
 static void finishPropertyWrapperImplInfo(VarDecl *var,
                                           StorageImplInfo &info) {
   auto parentSF = var->getDeclContext()->getParentSourceFile();
@@ -2149,12 +2200,18 @@ static void finishPropertyWrapperImplInfo(VarDecl *var,
     return;
   }
 
-  bool wrapperSetterIsUsable =
-    var->getSetter() ||
-    (parentSF &&
-     parentSF->Kind != SourceFileKind::Interface &&
-     !var->isLet() &&
-     allPropertyWrapperValueSettersAreAccessible(var));
+  bool wrapperSetterIsUsable = false;
+  if (var->getSetter()) {
+    wrapperSetterIsUsable = true;
+  } else if (parentSF && parentSF->Kind != SourceFileKind::Interface
+             && !var->isLet()) {
+    if (auto comp = var->getPropertyWrapperMutability()) {
+      wrapperSetterIsUsable =
+        comp->Setter != PropertyWrapperMutability::DoesntExist;
+    } else {
+      wrapperSetterIsUsable = true;
+    }
+  }
 
   if (wrapperSetterIsUsable)
     info = StorageImplInfo::getMutableComputed();