Skip to content

Commit 66d186e

Browse files
rooproop
committed
[Property wrappers] Fix l-value computation when composing wrappers
This fixes l-value computation in buildStorageReference when composing property wrappers. The l-value-ness required for each instance in a property wrapper chain is computed through a request and returned as an instance of PropertyWrapperLValueness.
1 parent 6c23146 commit 66d186e

File tree

8 files changed

+337
-37
lines changed

8 files changed

+337
-37
lines changed

‎include/swift/AST/ASTTypeIDZone.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ SWIFT_TYPEID_NAMED(NamedPattern *, NamedPattern)
4343
SWIFT_TYPEID_NAMED(NominalTypeDecl *, NominalTypeDecl)
4444
SWIFT_TYPEID_NAMED(OpaqueTypeDecl *, OpaqueTypeDecl)
4545
SWIFT_TYPEID_NAMED(OperatorDecl *, OperatorDecl)
46+
SWIFT_TYPEID_NAMED(Optional<PropertyWrapperLValueness>,
47+
PropertyWrapperLValueness)
4648
SWIFT_TYPEID_NAMED(Optional<PropertyWrapperMutability>,
4749
PropertyWrapperMutability)
4850
SWIFT_TYPEID_NAMED(ParamDecl *, ParamDecl)

‎include/swift/AST/ASTTypeIDs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class PrecedenceGroupDecl;
4646
struct PropertyWrapperBackingPropertyInfo;
4747
struct PropertyWrapperTypeInfo;
4848
enum class CtorInitializerKind;
49+
struct PropertyWrapperLValueness;
4950
struct PropertyWrapperMutability;
5051
class ProtocolDecl;
5152
class Requirement;

‎include/swift/AST/PropertyWrappers.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,20 @@ struct PropertyWrapperMutability {
123123

124124
void simple_display(llvm::raw_ostream &os, PropertyWrapperMutability m);
125125

126+
/// Describes whether the reference to a property wrapper instance used for
127+
/// accessing a wrapped property should be an l-value or not.
128+
struct PropertyWrapperLValueness {
129+
llvm::SmallVector<bool, 1> isLValueForGetAccess;
130+
llvm::SmallVector<bool, 1> isLValueForSetAccess;
131+
132+
bool operator==(PropertyWrapperLValueness other) const {
133+
return (isLValueForGetAccess == other.isLValueForGetAccess &&
134+
isLValueForSetAccess == other.isLValueForSetAccess);
135+
}
136+
};
137+
138+
void simple_display(llvm::raw_ostream &os, PropertyWrapperLValueness l);
139+
126140
/// Describes the backing property of a property that has an attached wrapper.
127141
struct PropertyWrapperBackingPropertyInfo {
128142
/// The backing property.

‎include/swift/AST/TypeCheckRequests.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class DefaultArgumentExpr;
3939
class GenericParamList;
4040
class PrecedenceGroupDecl;
4141
struct PropertyWrapperBackingPropertyInfo;
42+
struct PropertyWrapperLValueness;
4243
struct PropertyWrapperMutability;
4344
class RequirementRepr;
4445
class SpecializeAttr;
@@ -619,6 +620,26 @@ class PropertyWrapperMutabilityRequest :
619620
bool isCached() const;
620621
};
621622

623+
/// Request information about the l-valueness of composed property wrappers.
624+
class PropertyWrapperLValuenessRequest :
625+
public SimpleRequest<PropertyWrapperLValuenessRequest,
626+
Optional<PropertyWrapperLValueness> (VarDecl *),
627+
CacheKind::Cached> {
628+
public:
629+
using SimpleRequest::SimpleRequest;
630+
631+
private:
632+
friend SimpleRequest;
633+
634+
// Evaluation.
635+
llvm::Expected<Optional<PropertyWrapperLValueness>>
636+
evaluate(Evaluator &evaluator, VarDecl *var) const;
637+
638+
public:
639+
// Caching
640+
bool isCached() const;
641+
};
642+
622643
/// Request information about the backing property for properties that have
623644
/// attached property wrappers.
624645
class PropertyWrapperBackingPropertyInfoRequest :

‎include/swift/AST/TypeCheckerTypeIDZone.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ SWIFT_REQUEST(TypeChecker, PropertyWrapperBackingPropertyInfoRequest,
136136
NoLocationInfo)
137137
SWIFT_REQUEST(TypeChecker, PropertyWrapperBackingPropertyTypeRequest,
138138
Type(VarDecl *), Cached, NoLocationInfo)
139+
SWIFT_REQUEST(TypeChecker, PropertyWrapperLValuenessRequest,
140+
Optional<PropertyWrapperLValueness>(VarDecl *), Cached,
141+
NoLocationInfo)
139142
SWIFT_REQUEST(TypeChecker, PropertyWrapperMutabilityRequest,
140143
Optional<PropertyWrapperMutability>(VarDecl *), Cached,
141144
NoLocationInfo)

‎lib/AST/TypeCheckRequests.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,11 @@ bool PropertyWrapperMutabilityRequest::isCached() const {
547547
return !var->getAttrs().isEmpty();
548548
}
549549

550+
bool PropertyWrapperLValuenessRequest::isCached() const {
551+
auto var = std::get<0>(getStorage());
552+
return !var->getAttrs().isEmpty();
553+
}
554+
550555
void swift::simple_display(
551556
llvm::raw_ostream &out, const PropertyWrapperTypeInfo &propertyWrapper) {
552557
out << "{ ";
@@ -589,6 +594,14 @@ void swift::simple_display(llvm::raw_ostream &os, PropertyWrapperMutability m) {
589594
os << "getter " << names[m.Getter] << ", setter " << names[m.Setter];
590595
}
591596

597+
void swift::simple_display(llvm::raw_ostream &out, PropertyWrapperLValueness l) {
598+
out << "is lvalue for get: {";
599+
simple_display(out, l.isLValueForGetAccess);
600+
out << "}, is lvalue for set: {";
601+
simple_display(out, l.isLValueForSetAccess);
602+
out << "}";
603+
}
604+
592605
void swift::simple_display(llvm::raw_ostream &out,
593606
const ResilienceExpansion &value) {
594607
out << value;

‎lib/Sema/TypeCheckStorage.cpp

Lines changed: 158 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,15 @@ getEnclosingSelfPropertyWrapperAccess(VarDecl *property, bool forProjected) {
606606
return result;
607607
}
608608

609+
static Optional<PropertyWrapperLValueness>
610+
getPropertyWrapperLValueness(VarDecl *var) {
611+
auto &ctx = var->getASTContext();
612+
return evaluateOrDefault(
613+
ctx.evaluator,
614+
PropertyWrapperLValuenessRequest{var},
615+
None);
616+
}
617+
609618
/// Build a reference to the storage of a declaration. Returns nullptr if there
610619
/// was an error. This should only occur if an invalid declaration was type
611620
/// checked; another diagnostic should have been emitted already.
@@ -621,9 +630,11 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
621630
// Local function to "finish" the expression, creating a member reference
622631
// to the given sequence of underlying variables.
623632
Optional<EnclosingSelfPropertyWrapperAccess> enclosingSelfAccess;
624-
llvm::TinyPtrVector<VarDecl *> underlyingVars;
633+
llvm::SmallVector<std::pair<VarDecl *, bool>, 1> underlyingVars;
625634
auto finish = [&](Expr *result) -> Expr * {
626-
for (auto underlyingVar : underlyingVars) {
635+
for (auto underlyingVarPair : underlyingVars) {
636+
auto underlyingVar = underlyingVarPair.first;
637+
auto isWrapperRefLValue = underlyingVarPair.second;
627638
auto subs = result->getType()
628639
->getWithoutSpecifierType()
629640
->getContextSubstitutionMap(
@@ -634,7 +645,7 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
634645
auto *memberRefExpr = new (ctx) MemberRefExpr(
635646
result, SourceLoc(), memberRef, DeclNameLoc(), /*Implicit=*/true);
636647
auto type = underlyingVar->getValueInterfaceType().subst(subs);
637-
if (isLValue)
648+
if (isWrapperRefLValue)
638649
type = LValueType::get(type);
639650
memberRefExpr->setType(type);
640651

@@ -650,6 +661,8 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
650661
SelfAccessorKind selfAccessKind;
651662
Type selfTypeForAccess = (selfDecl ? selfDecl->getType() : Type());
652663

664+
bool isMemberLValue = isLValue;
665+
653666
auto *genericEnv = accessor->getGenericEnvironment();
654667
SubstitutionMap subs;
655668
if (genericEnv)
@@ -714,22 +727,43 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
714727
firstWrapperIdx = 1;
715728

716729
// Perform accesses to the wrappedValues along the composition chain.
717-
for (unsigned i : range(firstWrapperIdx, lastWrapperIdx)) {
718-
auto wrapperInfo = var->getAttachedPropertyWrapperTypeInfo(i);
719-
auto wrappedValue = wrapperInfo.valueVar;
720-
721-
// Check for availability of wrappedValue.
722-
if (accessor->getAccessorKind() == AccessorKind::Get ||
723-
accessor->getAccessorKind() == AccessorKind::Read) {
724-
if (auto *attr = wrappedValue->getAttrs().getUnavailable(ctx)) {
725-
diagnoseExplicitUnavailability(
726-
wrappedValue,
727-
var->getAttachedPropertyWrappers()[i]->getRangeWithAt(),
728-
var->getDeclContext(), nullptr);
730+
if (firstWrapperIdx < lastWrapperIdx) {
731+
if (auto lvalueness = getPropertyWrapperLValueness(var)) {
732+
733+
// Figure out if the outermost wrapper instance should be an l-value
734+
bool isLValueForGet = lvalueness->isLValueForGetAccess[firstWrapperIdx];
735+
bool isLValueForSet = lvalueness->isLValueForSetAccess[firstWrapperIdx];
736+
isMemberLValue = (isLValueForGet && isUsedForGetAccess) ||
737+
(isLValueForSet && isUsedForSetAccess);
738+
739+
for (unsigned i : range(firstWrapperIdx, lastWrapperIdx)) {
740+
auto wrapperInfo = var->getAttachedPropertyWrapperTypeInfo(i);
741+
auto wrappedValue = wrapperInfo.valueVar;
742+
743+
// Figure out if the wrappedValue accesses should be l-values
744+
bool isWrapperRefLValue = isLValue;
745+
if (i < lastWrapperIdx - 1) {
746+
bool isLValueForGet = lvalueness->isLValueForGetAccess[i+1];
747+
bool isLValueForSet = lvalueness->isLValueForSetAccess[i+1];
748+
isWrapperRefLValue = (isLValueForGet && isUsedForGetAccess) ||
749+
(isLValueForSet && isUsedForSetAccess);
750+
}
751+
752+
// Check for availability of wrappedValue.
753+
if (accessor->getAccessorKind() == AccessorKind::Get ||
754+
accessor->getAccessorKind() == AccessorKind::Read) {
755+
if (auto *attr = wrappedValue->getAttrs().getUnavailable(ctx)) {
756+
diagnoseExplicitUnavailability(
757+
wrappedValue,
758+
var->getAttachedPropertyWrappers()[i]->getRangeWithAt(),
759+
var->getDeclContext(), nullptr);
760+
}
761+
}
762+
763+
underlyingVars.push_back({ wrappedValue, isWrapperRefLValue });
764+
729765
}
730766
}
731-
732-
underlyingVars.push_back(wrappedValue);
733767
}
734768
semantics = AccessSemantics::DirectToStorage;
735769
selfAccessKind = SelfAccessorKind::Peer;
@@ -750,8 +784,15 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
750784
enclosingSelfAccess =
751785
getEnclosingSelfPropertyWrapperAccess(var, /*forProjected=*/true);
752786
if (!enclosingSelfAccess) {
787+
auto projectionVar = cast<VarDecl>(accessor->getStorage());
788+
if (auto lvalueness = getPropertyWrapperLValueness(projectionVar)) {
789+
isMemberLValue =
790+
(lvalueness->isLValueForGetAccess[0] && isUsedForGetAccess) ||
791+
(lvalueness->isLValueForSetAccess[0] && isUsedForSetAccess);
792+
}
753793
underlyingVars.push_back(
754-
var->getAttachedPropertyWrapperTypeInfo(0).projectedValueVar);
794+
{ var->getAttachedPropertyWrapperTypeInfo(0).projectedValueVar,
795+
isLValue });
755796
}
756797
semantics = AccessSemantics::DirectToStorage;
757798
selfAccessKind = SelfAccessorKind::Peer;
@@ -806,25 +847,6 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
806847

807848
// Build self.member or equivalent
808849

809-
bool isMemberLValue = isLValue;
810-
if (target == TargetImpl::Wrapper || target == TargetImpl::WrapperStorage) {
811-
// If the outermost property wrapper's getter / setter is mutating,
812-
// then the reference to the backing storage should be an l-value.
813-
auto var = cast<VarDecl>(accessor->getStorage());
814-
auto varMember = (target == TargetImpl::WrapperStorage)
815-
? &PropertyWrapperTypeInfo::projectedValueVar
816-
: &PropertyWrapperTypeInfo::valueVar;
817-
auto wrapper = (target == TargetImpl::WrapperStorage)
818-
? var->getOriginalWrappedProperty(
819-
PropertyWrapperSynthesizedPropertyKind::StorageWrapper)
820-
->getAttachedPropertyWrapperTypeInfo(0)
821-
: var->getAttachedPropertyWrapperTypeInfo(0);
822-
bool isWrapperGetterMutating = (wrapper.*varMember)->isGetterMutating();
823-
bool isWrapperSetterMutating = (wrapper.*varMember)->isSetterMutating();
824-
isMemberLValue = (isWrapperGetterMutating && isUsedForGetAccess) ||
825-
(isWrapperSetterMutating && isUsedForSetAccess);
826-
}
827-
828850
Expr *lookupExpr;
829851
ConcreteDeclRef memberRef(storage, subs);
830852
auto type = storage->getValueInterfaceType().subst(subs);
@@ -2393,6 +2415,105 @@ PropertyWrapperMutabilityRequest::evaluate(Evaluator &,
23932415
return result;
23942416
}
23952417

2418+
llvm::Expected<Optional<PropertyWrapperLValueness>>
2419+
PropertyWrapperLValuenessRequest::evaluate(Evaluator &,
2420+
VarDecl *var) const {
2421+
VarDecl *VD = var;
2422+
unsigned numWrappers = var->getAttachedPropertyWrappers().size();
2423+
bool isProjectedValue = false;
2424+
if (numWrappers < 1) {
2425+
VD = var->getOriginalWrappedProperty(
2426+
PropertyWrapperSynthesizedPropertyKind::StorageWrapper);
2427+
numWrappers = 1; // Can't compose projected values
2428+
isProjectedValue = true;
2429+
}
2430+
2431+
if (!VD)
2432+
return None;
2433+
2434+
auto varMember = isProjectedValue
2435+
? &PropertyWrapperTypeInfo::projectedValueVar
2436+
: &PropertyWrapperTypeInfo::valueVar;
2437+
2438+
// Calling the getter (or setter) on the nth property wrapper in the chain
2439+
// is done as follows:
2440+
// 1. call the getter on the (n-1)th property wrapper instance to get the
2441+
// nth property wrapper instance
2442+
// 2. call the getter (or setter) on the nth property wrapper instance
2443+
// 3. if (2) is a mutating access, call the setter on the (n-1)th property
2444+
// wrapper instance to write back the mutated value
2445+
2446+
// Below, we determine which of these property wrapper instances need to be
2447+
// accessed mutating-ly, and therefore should be l-values.
2448+
2449+
// The variables used are:
2450+
//
2451+
// - prevWrappersMutabilityForGet:
2452+
//
2453+
// The mutability needed for the first (n-1) wrapper instances to be
2454+
// able to call the getter on the (n-1)th instance, for step (1) above
2455+
//
2456+
// - prevWrappersMutabilityForGetAndSet:
2457+
//
2458+
// The mutability needed for the first (n-1) wrapper instances to be
2459+
// able to call both the getter and setter on the (n-1)th instance, for
2460+
// steps (1) and (3) above
2461+
//
2462+
// - mutability:
2463+
//
2464+
// The mutability needed for calling the getter / setter on the
2465+
// nth wrapper instance, for step (2) above.
2466+
2467+
llvm::SmallVector<PropertyWrapperMutability::Value, 1>
2468+
prevWrappersMutabilityForGet, prevWrappersMutabilityForGetAndSet;
2469+
PropertyWrapperMutability mutability;
2470+
2471+
auto firstWrapperInfo = VD->getAttachedPropertyWrapperTypeInfo(0);
2472+
mutability.Getter = getGetterMutatingness(firstWrapperInfo.*varMember);
2473+
mutability.Setter = getSetterMutatingness(firstWrapperInfo.*varMember,
2474+
var->getInnermostDeclContext());
2475+
2476+
for (unsigned i : range(1, numWrappers)) {
2477+
if (mutability.Getter == PropertyWrapperMutability::Mutating) {
2478+
prevWrappersMutabilityForGet = prevWrappersMutabilityForGetAndSet;
2479+
}
2480+
if (mutability.Getter != PropertyWrapperMutability::Mutating &&
2481+
mutability.Setter != PropertyWrapperMutability::Mutating) {
2482+
prevWrappersMutabilityForGetAndSet = prevWrappersMutabilityForGet;
2483+
}
2484+
prevWrappersMutabilityForGet.push_back(mutability.Getter);
2485+
prevWrappersMutabilityForGetAndSet.push_back(
2486+
std::max(mutability.Getter, mutability.Setter));
2487+
auto wrapperInfo = VD->getAttachedPropertyWrapperTypeInfo(i);
2488+
mutability.Getter = getGetterMutatingness(wrapperInfo.*varMember);
2489+
mutability.Setter = getSetterMutatingness(wrapperInfo.*varMember,
2490+
var->getInnermostDeclContext());
2491+
}
2492+
2493+
auto mutabilitySequenceForLastGet =
2494+
(mutability.Getter == PropertyWrapperMutability::Mutating)
2495+
? &prevWrappersMutabilityForGetAndSet
2496+
: &prevWrappersMutabilityForGet;
2497+
auto mutabilitySequenceForLastSet =
2498+
(mutability.Setter == PropertyWrapperMutability::Mutating)
2499+
? &prevWrappersMutabilityForGetAndSet
2500+
: &prevWrappersMutabilityForGet;
2501+
2502+
PropertyWrapperLValueness lvalueness;
2503+
for (unsigned i : range(numWrappers - 1)) {
2504+
lvalueness.isLValueForGetAccess.push_back(
2505+
(*mutabilitySequenceForLastGet)[i] == PropertyWrapperMutability::Mutating);
2506+
lvalueness.isLValueForSetAccess.push_back(
2507+
(*mutabilitySequenceForLastSet)[i] == PropertyWrapperMutability::Mutating);
2508+
}
2509+
lvalueness.isLValueForGetAccess.push_back(
2510+
mutability.Getter == PropertyWrapperMutability::Mutating);
2511+
lvalueness.isLValueForSetAccess.push_back(
2512+
mutability.Setter == PropertyWrapperMutability::Mutating);
2513+
2514+
return lvalueness;
2515+
}
2516+
23962517
llvm::Expected<PropertyWrapperBackingPropertyInfo>
23972518
PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
23982519
VarDecl *var) const {

0 commit comments

Comments
 (0)