Skip to content

Sema: Correct composition of property wrappers. #26326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/swift/AST/ASTTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -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)
1 change: 1 addition & 0 deletions include/swift/AST/ASTTypeIDs.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class CustomAttr;
class NominalTypeDecl;
struct PropertyWrapperBackingPropertyInfo;
struct PropertyWrapperTypeInfo;
struct PropertyWrapperMutability;
class Type;
class VarDecl;
class TypeAliasDecl;
Expand Down
6 changes: 6 additions & 0 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ namespace swift {
struct PrintOptions;
struct PropertyWrapperBackingPropertyInfo;
struct PropertyWrapperTypeInfo;
struct PropertyWrapperMutability;
class ProtocolDecl;
class ProtocolType;
struct RawComment;
Expand Down Expand Up @@ -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.
///
Expand Down
5 changes: 3 additions & 2 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -4478,8 +4478,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", ())
Expand Down
41 changes: 41 additions & 0 deletions include/swift/AST/PropertyWrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need a special case? If the setter is DoesntExist, std::max() will always end up returning DoesntExist, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose not. It seems worth calling this out as a specific rule, though.

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.
Expand Down
21 changes: 21 additions & 0 deletions include/swift/AST/TypeCheckRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ namespace swift {
class AbstractStorageDecl;
class GenericParamList;
struct PropertyWrapperBackingPropertyInfo;
struct PropertyWrapperMutability;
class RequirementRepr;
class SpecializeAttr;
class TypeAliasDecl;
Expand Down Expand Up @@ -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 :
Expand Down
3 changes: 2 additions & 1 deletion include/swift/AST/TypeCheckerTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -46,4 +47,4 @@ SWIFT_TYPEID(LazyStoragePropertyRequest)
SWIFT_TYPEID(TypeCheckFunctionBodyUntilRequest)
SWIFT_TYPEID(StoredPropertiesRequest)
SWIFT_TYPEID(StoredPropertiesAndMissingMembersRequest)
SWIFT_TYPEID(StorageImplInfoRequest)
SWIFT_TYPEID(StorageImplInfoRequest)
8 changes: 8 additions & 0 deletions include/swift/Basic/AnyValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 //
Expand Down
5 changes: 3 additions & 2 deletions lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
10 changes: 10 additions & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
13 changes: 13 additions & 0 deletions lib/AST/TypeCheckRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 << "{ ";
Expand All @@ -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.
//----------------------------------------------------------------------------//
Expand Down
111 changes: 84 additions & 27 deletions lib/Sema/CodeSynthesis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -1942,7 +2012,7 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
storageVar = synthesizePropertyWrapperStorageWrapperProperty(
ctx, var, storageInterfaceType, wrapperInfo.projectedValueVar);
}

// Get the property wrapper information.
if (!var->allAttachedPropertyWrappersHaveInitialValueInit() &&
!originalInitialValue) {
Expand All @@ -1960,7 +2030,7 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
/*ignoreAttributeArgs=*/!originalInitialValue);
typeCheckSynthesizedWrapperInitializer(
pbd, backingVar, parentPBD, initializer);

return PropertyWrapperBackingPropertyInfo(
backingVar, storageVar, originalInitialValue, initializer, origValue);
}
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down
Loading