From 1446b608011e3387384d6fb9c77d9ed2030d190f Mon Sep 17 00:00:00 2001 From: Joe Groff Date: Thu, 25 Jul 2019 12:11:17 -0700 Subject: [PATCH 1/2] Reenable property wrapper composition. This reverts commit 5e08f7da3274585825b081adbdb400d6cf433af3. --- include/swift/AST/DiagnosticsSema.def | 3 --- lib/Sema/TypeCheckPropertyWrapper.cpp | 7 ------- test/SILGen/property_wrappers.swift | 28 +++++++++++++-------------- test/decl/var/property_wrappers.swift | 12 ++++++------ 4 files changed, 19 insertions(+), 31 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 9c0a7bb7a8d2a..dad6cb9b69f4a 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4478,9 +4478,6 @@ 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_local,none, "property wrappers are not yet supported on local properties", ()) ERROR(property_wrapper_top_level,none, diff --git a/lib/Sema/TypeCheckPropertyWrapper.cpp b/lib/Sema/TypeCheckPropertyWrapper.cpp index d4d0b3f5eed5f..2fc302108d55f 100644 --- a/lib/Sema/TypeCheckPropertyWrapper.cpp +++ b/lib/Sema/TypeCheckPropertyWrapper.cpp @@ -460,13 +460,6 @@ AttachedPropertyWrappersRequest::evaluate(Evaluator &evaluator, result.push_back(mutableAttr); } - // TODO: Property wrapper compositions do not yet compose correctly - // in all situtaions. - if (result.size() > 1) { - ctx.Diags.diagnose(result[result.size() - 2]->getLocation(), - diag::property_wrapper_composition_not_implemented); - } - // Attributes are stored in reverse order in the AST, but we want them in // source order so that the outermost property wrapper comes first. std::reverse(result.begin(), result.end()); diff --git a/test/SILGen/property_wrappers.swift b/test/SILGen/property_wrappers.swift index efc419a8b2193..b3e8b60761107 100644 --- a/test/SILGen/property_wrappers.swift +++ b/test/SILGen/property_wrappers.swift @@ -308,33 +308,31 @@ struct WrapperC { var wrappedValue: Value? } -/* TODO: Reenable composed property wrappers struct CompositionMembers { // CompositionMembers.p1.getter - // C/HECK-LABEL: sil hidden [ossa] @$s17property_wrappers18CompositionMembersV2p1SiSgvg : $@convention(method) (@guaranteed CompositionMembers) -> Optional - // C/HECK: bb0([[SELF:%.*]] : @guaranteed $CompositionMembers): - // C/HECK: [[P1:%.*]] = struct_extract [[SELF]] : $CompositionMembers, #CompositionMembers._p1 - // C/HECK: [[P1_VALUE:%.*]] = struct_extract [[P1]] : $WrapperA>>, #WrapperA.wrappedValue - // C/HECK: [[P1_VALUE2:%.*]] = struct_extract [[P1_VALUE]] : $WrapperB>, #WrapperB.wrappedValue - // C/HECK: [[P1_VALUE3:%.*]] = struct_extract [[P1_VALUE2]] : $WrapperC, #WrapperC.wrappedValue - // C/HECK: return [[P1_VALUE3]] : $Optional + // CHECK-LABEL: sil hidden [ossa] @$s17property_wrappers18CompositionMembersV2p1SiSgvg : $@convention(method) (@guaranteed CompositionMembers) -> Optional + // CHECK: bb0([[SELF:%.*]] : @guaranteed $CompositionMembers): + // CHECK: [[P1:%.*]] = struct_extract [[SELF]] : $CompositionMembers, #CompositionMembers._p1 + // CHECK: [[P1_VALUE:%.*]] = struct_extract [[P1]] : $WrapperA>>, #WrapperA.wrappedValue + // CHECK: [[P1_VALUE2:%.*]] = struct_extract [[P1_VALUE]] : $WrapperB>, #WrapperB.wrappedValue + // CHECK: [[P1_VALUE3:%.*]] = struct_extract [[P1_VALUE2]] : $WrapperC, #WrapperC.wrappedValue + // CHECK: return [[P1_VALUE3]] : $Optional @WrapperA @WrapperB @WrapperC var p1: Int? @WrapperA @WrapperB @WrapperC var p2 = "Hello" // variable initialization expression of CompositionMembers.$p2 - // C/HECK-LABEL: sil hidden [transparent] [ossa] @$s17property_wrappers18CompositionMembersV3_p233_{{.*}}8WrapperAVyAA0N1BVyAA0N1CVySSGGGvpfi : $@convention(thin) () -> @owned Optional { - // C/HECK: %0 = string_literal utf8 "Hello" + // CHECK-LABEL: sil hidden [transparent] [ossa] @$s17property_wrappers18CompositionMembersV3_p233_{{.*}}8WrapperAVyAA0N1BVyAA0N1CVySSGGGvpfi : $@convention(thin) () -> @owned Optional { + // CHECK: %0 = string_literal utf8 "Hello" - // C/HECK-LABEL: sil hidden [ossa] @$s17property_wrappers18CompositionMembersV2p12p2ACSiSg_SSSgtcfC : $@convention(method) (Optional, @owned Optional, @thin CompositionMembers.Type) -> @owned CompositionMembers - // C/HECK: function_ref @$s17property_wrappers8WrapperCV12wrappedValueACyxGxSg_tcfC - // C/HECK: function_ref @$s17property_wrappers8WrapperBV12wrappedValueACyxGx_tcfC - // C/HECK: function_ref @$s17property_wrappers8WrapperAV12wrappedValueACyxGx_tcfC + // CHECK-LABEL: sil hidden [ossa] @$s17property_wrappers18CompositionMembersV2p12p2ACSiSg_SSSgtcfC : $@convention(method) (Optional, @owned Optional, @thin CompositionMembers.Type) -> @owned CompositionMembers + // CHECK: function_ref @$s17property_wrappers8WrapperCV12wrappedValueACyxGxSg_tcfC + // CHECK: function_ref @$s17property_wrappers8WrapperBV12wrappedValueACyxGx_tcfC + // CHECK: function_ref @$s17property_wrappers8WrapperAV12wrappedValueACyxGx_tcfC } func testComposition() { _ = CompositionMembers(p1: nil) } -*/ // Observers with non-default mutatingness. @propertyWrapper diff --git a/test/decl/var/property_wrappers.swift b/test/decl/var/property_wrappers.swift index 5cc2aa1a704f3..1dfbd31c912ad 100644 --- a/test/decl/var/property_wrappers.swift +++ b/test/decl/var/property_wrappers.swift @@ -217,7 +217,7 @@ struct BadCombinations { struct MultipleWrappers { @Wrapper(stored: 17) - @WrapperWithInitialValue // expected-error{{extra argument 'wrappedValue' in call}} expected-error{{multiple property wrappers are not supported}} + @WrapperWithInitialValue // expected-error{{extra argument 'wrappedValue' in call}} var x: Int = 17 @WrapperWithInitialValue // expected-error 2{{property wrapper can only apply to a single variable}} @@ -959,11 +959,11 @@ struct WrapperE { } struct TestComposition { - @WrapperA @WrapperB @WrapperC var p1: Int? // expected-error{{multiple property wrappers are not supported}} - @WrapperA @WrapperB @WrapperC var p2 = "Hello" // expected-error{{multiple property wrappers are not supported}} - @WrapperD @WrapperE var p3: Int? // expected-error{{multiple property wrappers are not supported}} - @WrapperD @WrapperC var p4: Int? // expected-error{{multiple property wrappers are not supported}} - @WrapperD @WrapperE var p5: Int // expected-error{{property type 'Int' does not match that of the 'wrappedValue' property of its wrapper type 'WrapperD'}} // expected-error{{multiple property wrappers are not supported}} + @WrapperA @WrapperB @WrapperC var p1: Int? + @WrapperA @WrapperB @WrapperC var p2 = "Hello" + @WrapperD @WrapperE var p3: Int? + @WrapperD @WrapperC var p4: Int? + @WrapperD @WrapperE var p5: Int // expected-error{{property type 'Int' does not match that of the 'wrappedValue' property of its wrapper type 'WrapperD'}} func triggerErrors(d: Double) { p1 = d // expected-error{{cannot assign value of type 'Double' to type 'Int?'}} From fa4dd15612c7fcbd50e9c2ed286fad7913a13914 Mon Sep 17 00:00:00 2001 From: Joe Groff Date: Mon, 22 Jul 2019 18:40:54 -0700 Subject: [PATCH 2/2] Sema: Correct composition of property wrappers. Fixes rdar://problem/53407949 | SR-11138. Previously, we'd only look at the outermost property wrapper to decide whether the wrapped property's getter and setter were `mutating` (or exist at all). In reality, this requires considering the semantics of the composed accesses of each wrapper layer's `wrappedValue` property. Fixing this systematically addresses a number of issues: - As SR-11138 reported, composing a nonmutating-get-set wrapper ought to produce a composed wrapper that's nonmutating. - We would previously allow a property wrapper with a mutating getter to be nested inside one with only a getter, even though the resulting implementation was unsound (because there's no mutable context for the inner wrapper to execute its get on.) - Similarly, we would construct unsound setters in cases where the setter can't exist, such as when the nested wrapper isn't settable but the outer wrapper is. --- include/swift/AST/ASTTypeIDZone.def | 1 + include/swift/AST/ASTTypeIDs.h | 1 + include/swift/AST/Decl.h | 6 + include/swift/AST/DiagnosticsSema.def | 4 + include/swift/AST/PropertyWrappers.h | 41 +++ include/swift/AST/TypeCheckRequests.h | 21 ++ include/swift/AST/TypeCheckerTypeIDZone.def | 3 +- include/swift/Basic/AnyValue.h | 8 + lib/AST/ASTContext.cpp | 5 +- lib/AST/Decl.cpp | 10 + lib/AST/TypeCheckRequests.cpp | 13 + lib/Sema/CodeSynthesis.cpp | 111 ++++-- lib/Sema/TypeCheckDecl.cpp | 26 +- test/decl/var/property_wrappers.swift | 385 ++++++++++++++++++++ 14 files changed, 589 insertions(+), 46 deletions(-) diff --git a/include/swift/AST/ASTTypeIDZone.def b/include/swift/AST/ASTTypeIDZone.def index 7bde46956e2f3..ef189fd9d9485 100644 --- a/include/swift/AST/ASTTypeIDZone.def +++ b/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) SWIFT_TYPEID_NAMED(CustomAttr *, CustomAttr) SWIFT_TYPEID_NAMED(TypeAliasDecl *, TypeAliasDecl) diff --git a/include/swift/AST/ASTTypeIDs.h b/include/swift/AST/ASTTypeIDs.h index 7bac65236a851..45d58d1de2627 100644 --- a/include/swift/AST/ASTTypeIDs.h +++ b/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; diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 2b6f1db24ebb7..5c8ac91680438 100644 --- a/include/swift/AST/Decl.h +++ b/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 + getPropertyWrapperMutability() const; + /// Retrieve the backing storage property for a property that has an /// attached property wrapper. /// diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index dad6cb9b69f4a..41b58b9233f95 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4478,6 +4478,10 @@ ERROR(property_wrapper_attribute_not_on_property, none, NOTE(property_wrapper_declared_here,none, "property wrapper type %0 declared here", (DeclName)) +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", ()) ERROR(property_wrapper_top_level,none, diff --git a/include/swift/AST/PropertyWrappers.h b/include/swift/AST/PropertyWrappers.h index d8792187affd4..b01958a369c20 100644 --- a/include/swift/AST/PropertyWrappers.h +++ b/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. diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index 50811dfb42227..0971b90119d78 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/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 (VarDecl *), + CacheKind::Cached> { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + // Evaluation. + llvm::Expected> + 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 : diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def index 038570179ee29..6154966c87b34 100644 --- a/include/swift/AST/TypeCheckerTypeIDZone.def +++ b/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) \ No newline at end of file +SWIFT_TYPEID(StorageImplInfoRequest) diff --git a/include/swift/Basic/AnyValue.h b/include/swift/Basic/AnyValue.h index 26d1cb12c8b34..6d76e9ba43b75 100644 --- a/include/swift/Basic/AnyValue.h +++ b/include/swift/Basic/AnyValue.h @@ -165,6 +165,14 @@ namespace llvm { bool operator!=(const TinyPtrVector &lhs, const TinyPtrVector &rhs) { return !(lhs == rhs); } + + template + void simple_display(raw_ostream &out, const Optional &opt) { + if (opt) { + simple_display(out, *opt); + } + out << "None"; + } } // end namespace llvm #endif // diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 4294b6760c183..b6e31489ffd5c 100644 --- a/lib/AST/ASTContext.cpp +++ b/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()) { diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 83206ef88b2d3..297451909a5e8 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -5488,6 +5488,16 @@ VarDecl::getPropertyWrapperBackingPropertyInfo() const { PropertyWrapperBackingPropertyInfo()); } +Optional +VarDecl::getPropertyWrapperMutability() const { + auto &ctx = getASTContext(); + auto mutableThis = const_cast(this); + return evaluateOrDefault( + ctx.evaluator, + PropertyWrapperMutabilityRequest{mutableThis}, + None); +} + VarDecl *VarDecl::getPropertyWrapperBackingProperty() const { return getPropertyWrapperBackingPropertyInfo().backingVar; } diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp index 366d60b6a1915..e35ecda5fbebd 100644 --- a/lib/AST/TypeCheckRequests.cpp +++ b/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. //----------------------------------------------------------------------------// diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index ec0bc400a888e..9837f0a4f1fe8 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/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> +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 PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator, VarDecl *var) const { @@ -1942,7 +2012,7 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator, storageVar = synthesizePropertyWrapperStorageWrapperProperty( ctx, var, storageInterfaceType, wrapperInfo.projectedValueVar); } - + // Get the property wrapper information. if (!var->allAttachedPropertyWrappersHaveInitialValueInit() && !originalInitialValue) { @@ -1960,7 +2030,7 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator, /*ignoreAttributeArgs=*/!originalInitialValue); typeCheckSynthesizedWrapperInitializer( pbd, backingVar, parentPBD, initializer); - + return PropertyWrapperBackingPropertyInfo( backingVar, storageVar, originalInitialValue, initializer, origValue); } @@ -2103,25 +2173,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(); @@ -2137,12 +2188,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(); diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 12bc04cc3ace5..ac1e415197f1c 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -2204,15 +2204,12 @@ IsGetterMutatingRequest::evaluate(Evaluator &evaluator, return result; } - // If we have an attached property wrapper, the getter is mutating if - // the "value" property of the outermost wrapper type is mutating and we're - // in a context that has value semantics. + // If we have an attached property wrapper, the getter's mutating-ness + // depends on the composition of the wrappers. if (auto var = dyn_cast(storage)) { - if (auto wrapperInfo = var->getAttachedPropertyWrapperTypeInfo(0)) { - if (wrapperInfo.valueVar && - (!storage->getGetter() || storage->getGetter()->isImplicit())) { - return wrapperInfo.valueVar->isGetterMutating() && result; - } + if (auto mut = var->getPropertyWrapperMutability()) { + return mut->Getter == PropertyWrapperMutability::Mutating + && result; } } @@ -2254,15 +2251,12 @@ IsSetterMutatingRequest::evaluate(Evaluator &evaluator, bool result = (!storage->isStatic() && doesContextHaveValueSemantics(storage->getDeclContext())); - // If we have an attached property wrapper, the setter is mutating if - // the "value" property of the outermost wrapper type is mutating and we're - // in a context that has value semantics. + // If we have an attached property wrapper, the setter is mutating + // or not based on the composition of the wrappers. if (auto var = dyn_cast(storage)) { - if (auto wrapperInfo = var->getAttachedPropertyWrapperTypeInfo(0)) { - if (wrapperInfo.valueVar && - (!storage->getSetter() || storage->getSetter()->isImplicit())) { - return wrapperInfo.valueVar->isSetterMutating() && result; - } + if (auto mut = var->getPropertyWrapperMutability()) { + return mut->Setter == PropertyWrapperMutability::Mutating + && result; } } diff --git a/test/decl/var/property_wrappers.swift b/test/decl/var/property_wrappers.swift index 1dfbd31c912ad..396ebddebf2ad 100644 --- a/test/decl/var/property_wrappers.swift +++ b/test/decl/var/property_wrappers.swift @@ -1119,3 +1119,388 @@ struct SR_11060_Wrapper { } } +// SR-11138 +// Check that all possible compositions of nonmutating/mutating accessors +// on wrappers produce wrapped properties with the correct settability and +// mutatiness in all compositions. + +@propertyWrapper +struct NonmutatingGetWrapper { + var wrappedValue: T { + nonmutating get { fatalError() } + } +} + +@propertyWrapper +struct MutatingGetWrapper { + var wrappedValue: T { + mutating get { fatalError() } + } +} + +@propertyWrapper +struct NonmutatingGetNonmutatingSetWrapper { + var wrappedValue: T { + nonmutating get { fatalError() } + nonmutating set { fatalError() } + } +} + +@propertyWrapper +struct MutatingGetNonmutatingSetWrapper { + var wrappedValue: T { + mutating get { fatalError() } + nonmutating set { fatalError() } + } +} + +@propertyWrapper +struct NonmutatingGetMutatingSetWrapper { + var wrappedValue: T { + nonmutating get { fatalError() } + mutating set { fatalError() } + } +} + +@propertyWrapper +struct MutatingGetMutatingSetWrapper { + var wrappedValue: T { + mutating get { fatalError() } + mutating set { fatalError() } + } +} + +struct AllCompositionsStruct { + // Should have nonmutating getter, undefined setter + @NonmutatingGetWrapper @NonmutatingGetWrapper + var ngxs_ngxs: Int + + // Should be disallowed + @NonmutatingGetWrapper @MutatingGetWrapper // expected-error{{cannot be composed}} + var ngxs_mgxs: Int + + // Should have nonmutating getter, nonmutating setter + @NonmutatingGetWrapper @NonmutatingGetNonmutatingSetWrapper + var ngxs_ngns: Int + + // Should be disallowed + @NonmutatingGetWrapper @MutatingGetNonmutatingSetWrapper // expected-error{{cannot be composed}} + var ngxs_mgns: Int + + // Should have nonmutating getter, undefined setter + @NonmutatingGetWrapper @NonmutatingGetMutatingSetWrapper + var ngxs_ngms: Int + + // Should be disallowed + @NonmutatingGetWrapper @MutatingGetMutatingSetWrapper // expected-error{{cannot be composed}} + var ngxs_mgms: Int + + //// + + // Should have mutating getter, undefined setter + @MutatingGetWrapper @NonmutatingGetWrapper + var mgxs_ngxs: Int + + // Should be disallowed + @MutatingGetWrapper @MutatingGetWrapper // expected-error{{cannot be composed}} + var mgxs_mgxs: Int + + // Should have mutating getter, mutating setter + @MutatingGetWrapper @NonmutatingGetNonmutatingSetWrapper + var mgxs_ngns: Int + + // Should be disallowed + @MutatingGetWrapper @MutatingGetNonmutatingSetWrapper // expected-error{{cannot be composed}} + var mgxs_mgns: Int + + // Should have mutating getter, undefined setter + @MutatingGetWrapper @NonmutatingGetMutatingSetWrapper + var mgxs_ngms: Int + + // Should be disallowed + @MutatingGetWrapper @MutatingGetMutatingSetWrapper // expected-error{{cannot be composed}} + var mgxs_mgms: Int + + //// + + // Should have nonmutating getter, undefined setter + @NonmutatingGetNonmutatingSetWrapper @NonmutatingGetWrapper + var ngns_ngxs: Int + + // Should have nonmutating getter, undefined setter + @NonmutatingGetNonmutatingSetWrapper @MutatingGetWrapper + var ngns_mgxs: Int + + // Should have nonmutating getter, nonmutating setter + @NonmutatingGetNonmutatingSetWrapper @NonmutatingGetNonmutatingSetWrapper + var ngns_ngns: Int + + // Should have nonmutating getter, nonmutating setter + @NonmutatingGetNonmutatingSetWrapper @MutatingGetNonmutatingSetWrapper + var ngns_mgns: Int + + // Should have nonmutating getter, nonmutating setter + @NonmutatingGetNonmutatingSetWrapper @NonmutatingGetMutatingSetWrapper + var ngns_ngms: Int + + // Should have nonmutating getter, nonmutating setter + @NonmutatingGetNonmutatingSetWrapper @MutatingGetMutatingSetWrapper + var ngns_mgms: Int + + //// + + // Should have mutating getter, undefined setter + @MutatingGetNonmutatingSetWrapper @NonmutatingGetWrapper + var mgns_ngxs: Int + + // Should have mutating getter, undefined setter + @MutatingGetNonmutatingSetWrapper @MutatingGetWrapper + var mgns_mgxs: Int + + // Should have mutating getter, mutating setter + @MutatingGetNonmutatingSetWrapper @NonmutatingGetNonmutatingSetWrapper + var mgns_ngns: Int + + // Should have mutating getter, mutating setter + @MutatingGetNonmutatingSetWrapper @MutatingGetNonmutatingSetWrapper + var mgns_mgns: Int + + // Should have mutating getter, mutating setter + @MutatingGetNonmutatingSetWrapper @NonmutatingGetMutatingSetWrapper + var mgns_ngms: Int + + // Should have mutating getter, mutating setter + @MutatingGetNonmutatingSetWrapper @MutatingGetMutatingSetWrapper + var mgns_mgms: Int + + //// + + // Should have nonmutating getter, undefined setter + @NonmutatingGetMutatingSetWrapper @NonmutatingGetWrapper + var ngms_ngxs: Int + + // Should have mutating getter, undefined setter + @NonmutatingGetMutatingSetWrapper @MutatingGetWrapper + var ngms_mgxs: Int + + // Should have nonmutating getter, nonmutating setter + @NonmutatingGetMutatingSetWrapper @NonmutatingGetNonmutatingSetWrapper + var ngms_ngns: Int + + // Should have mutating getter, nonmutating setter + @NonmutatingGetMutatingSetWrapper @MutatingGetNonmutatingSetWrapper + var ngms_mgns: Int + + // Should have nonmutating getter, mutating setter + @NonmutatingGetMutatingSetWrapper @NonmutatingGetMutatingSetWrapper + var ngms_ngms: Int + + // Should have mutating getter, mutating setter + @NonmutatingGetMutatingSetWrapper @MutatingGetMutatingSetWrapper + var ngms_mgms: Int + + //// + + // Should have mutating getter, undefined setter + @MutatingGetMutatingSetWrapper @NonmutatingGetWrapper + var mgms_ngxs: Int + + // Should have mutating getter, undefined setter + @MutatingGetMutatingSetWrapper @MutatingGetWrapper + var mgms_mgxs: Int + + // Should have mutating getter, mutating setter + @MutatingGetMutatingSetWrapper @NonmutatingGetNonmutatingSetWrapper + var mgms_ngns: Int + + // Should have mutating getter, mutating setter + @MutatingGetMutatingSetWrapper @MutatingGetNonmutatingSetWrapper + var mgms_mgns: Int + + // Should have mutating getter, mutating setter + @MutatingGetMutatingSetWrapper @NonmutatingGetMutatingSetWrapper + var mgms_ngms: Int + + // Should have mutating getter, mutating setter + @MutatingGetMutatingSetWrapper @MutatingGetMutatingSetWrapper + var mgms_mgms: Int + + func readonlyContext(x: Int) { // expected-note *{{}} + _ = ngxs_ngxs + // _ = ngxs_mgxs + _ = ngxs_ngns + // _ = ngxs_mgns + _ = ngxs_ngms + // _ = ngxs_mgms + + _ = mgxs_ngxs // expected-error{{}} + // _ = mgxs_mgxs + _ = mgxs_ngns // expected-error{{}} + // _ = mgxs_mgns + _ = mgxs_ngms // expected-error{{}} + // _ = mgxs_mgms + + _ = ngns_ngxs + _ = ngns_mgxs + _ = ngns_ngns + _ = ngns_mgns + _ = ngns_ngms + _ = ngns_mgms + + _ = mgns_ngxs // expected-error{{}} + _ = mgns_mgxs // expected-error{{}} + _ = mgns_ngns // expected-error{{}} + _ = mgns_mgns // expected-error{{}} + _ = mgns_ngms // expected-error{{}} + _ = mgns_mgms // expected-error{{}} + + _ = ngms_ngxs + _ = ngms_mgxs // expected-error{{}} + _ = ngms_ngns + _ = ngms_mgns // expected-error{{}} + _ = ngms_ngms + _ = ngms_mgms // expected-error{{}} + + _ = mgms_ngxs // expected-error{{}} + _ = mgms_mgxs // expected-error{{}} + _ = mgms_ngns // expected-error{{}} + _ = mgms_mgns // expected-error{{}} + _ = mgms_ngms // expected-error{{}} + _ = mgms_mgms // expected-error{{}} + + //// + + ngxs_ngxs = x // expected-error{{}} + // ngxs_mgxs = x + ngxs_ngns = x + // ngxs_mgns = x + ngxs_ngms = x // expected-error{{}} + // ngxs_mgms = x + + mgxs_ngxs = x // expected-error{{}} + // mgxs_mgxs = x + mgxs_ngns = x // expected-error{{}} + // mgxs_mgns = x + mgxs_ngms = x // expected-error{{}} + // mgxs_mgms = x + + ngns_ngxs = x // expected-error{{}} + ngns_mgxs = x // expected-error{{}} + ngns_ngns = x + ngns_mgns = x + ngns_ngms = x + ngns_mgms = x + + mgns_ngxs = x // expected-error{{}} + mgns_mgxs = x // expected-error{{}} + mgns_ngns = x // expected-error{{}} + mgns_mgns = x // expected-error{{}} + mgns_ngms = x // expected-error{{}} + mgns_mgms = x // expected-error{{}} + + ngms_ngxs = x // expected-error{{}} + ngms_mgxs = x // expected-error{{}} + ngms_ngns = x + // FIXME: This ought to be allowed because it's a pure set, so the mutating + // get should not come into play. + ngms_mgns = x // expected-error{{cannot use mutating getter}} + ngms_ngms = x // expected-error{{}} + ngms_mgms = x // expected-error{{}} + + mgms_ngxs = x // expected-error{{}} + mgms_mgxs = x // expected-error{{}} + mgms_ngns = x // expected-error{{}} + mgms_mgns = x // expected-error{{}} + mgms_ngms = x // expected-error{{}} + mgms_mgms = x // expected-error{{}} + } + + mutating func mutatingContext(x: Int) { + _ = ngxs_ngxs + // _ = ngxs_mgxs + _ = ngxs_ngns + // _ = ngxs_mgns + _ = ngxs_ngms + // _ = ngxs_mgms + + _ = mgxs_ngxs + // _ = mgxs_mgxs + _ = mgxs_ngns + // _ = mgxs_mgns + _ = mgxs_ngms + // _ = mgxs_mgms + + _ = ngns_ngxs + _ = ngns_mgxs + _ = ngns_ngns + _ = ngns_mgns + _ = ngns_ngms + _ = ngns_mgms + + _ = mgns_ngxs + _ = mgns_mgxs + _ = mgns_ngns + _ = mgns_mgns + _ = mgns_ngms + _ = mgns_mgms + + _ = ngms_ngxs + _ = ngms_mgxs + _ = ngms_ngns + _ = ngms_mgns + _ = ngms_ngms + _ = ngms_mgms + + _ = mgms_ngxs + _ = mgms_mgxs + _ = mgms_ngns + _ = mgms_mgns + _ = mgms_ngms + _ = mgms_mgms + + //// + + ngxs_ngxs = x // expected-error{{}} + // ngxs_mgxs = x + ngxs_ngns = x + // ngxs_mgns = x + ngxs_ngms = x // expected-error{{}} + // ngxs_mgms = x + + mgxs_ngxs = x // expected-error{{}} + // mgxs_mgxs = x + mgxs_ngns = x + // mgxs_mgns = x + mgxs_ngms = x // expected-error{{}} + // mgxs_mgms = x + + ngns_ngxs = x // expected-error{{}} + ngns_mgxs = x // expected-error{{}} + ngns_ngns = x + ngns_mgns = x + ngns_ngms = x + ngns_mgms = x + + mgns_ngxs = x // expected-error{{}} + mgns_mgxs = x // expected-error{{}} + mgns_ngns = x + mgns_mgns = x + mgns_ngms = x + mgns_mgms = x + + ngms_ngxs = x // expected-error{{}} + ngms_mgxs = x // expected-error{{}} + ngms_ngns = x + ngms_mgns = x + ngms_ngms = x + ngms_mgms = x + + mgms_ngxs = x // expected-error{{}} + mgms_mgxs = x // expected-error{{}} + mgms_ngns = x + mgms_mgns = x + mgms_ngms = x + mgms_mgms = x + } +} +