Skip to content

Commit 149a1f4

Browse files
roophborla
roop
authored and
hborla
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 abaf79d commit 149a1f4

File tree

8 files changed

+336
-37
lines changed

8 files changed

+336
-37
lines changed

‎include/swift/AST/ASTTypeIDZone.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ SWIFT_TYPEID_NAMED(NamedPattern *, NamedPattern)
4646
SWIFT_TYPEID_NAMED(NominalTypeDecl *, NominalTypeDecl)
4747
SWIFT_TYPEID_NAMED(OpaqueTypeDecl *, OpaqueTypeDecl)
4848
SWIFT_TYPEID_NAMED(OperatorDecl *, OperatorDecl)
49+
SWIFT_TYPEID_NAMED(Optional<PropertyWrapperLValueness>,
50+
PropertyWrapperLValueness)
4951
SWIFT_TYPEID_NAMED(Optional<PropertyWrapperMutability>,
5052
PropertyWrapperMutability)
5153
SWIFT_TYPEID_NAMED(ParamDecl *, ParamDecl)

‎include/swift/AST/ASTTypeIDs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class PrefixOperatorDecl;
4949
struct PropertyWrapperBackingPropertyInfo;
5050
struct PropertyWrapperTypeInfo;
5151
enum class CtorInitializerKind;
52+
struct PropertyWrapperLValueness;
5253
struct PropertyWrapperMutability;
5354
class ProtocolDecl;
5455
class Requirement;

‎include/swift/AST/PropertyWrappers.h

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

128128
void simple_display(llvm::raw_ostream &os, PropertyWrapperMutability m);
129129

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

‎include/swift/AST/TypeCheckRequests.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class ClosureExpr;
4141
class GenericParamList;
4242
class PrecedenceGroupDecl;
4343
struct PropertyWrapperBackingPropertyInfo;
44+
struct PropertyWrapperLValueness;
4445
struct PropertyWrapperMutability;
4546
class RequirementRepr;
4647
class SpecializeAttr;
@@ -632,6 +633,26 @@ class PropertyWrapperMutabilityRequest :
632633
bool isCached() const;
633634
};
634635

636+
/// Request information about the l-valueness of composed property wrappers.
637+
class PropertyWrapperLValuenessRequest :
638+
public SimpleRequest<PropertyWrapperLValuenessRequest,
639+
Optional<PropertyWrapperLValueness> (VarDecl *),
640+
RequestFlags::Cached> {
641+
public:
642+
using SimpleRequest::SimpleRequest;
643+
644+
private:
645+
friend SimpleRequest;
646+
647+
// Evaluation.
648+
Optional<PropertyWrapperLValueness>
649+
evaluate(Evaluator &evaluator, VarDecl *var) const;
650+
651+
public:
652+
// Caching
653+
bool isCached() const;
654+
};
655+
635656
/// Request information about the backing property for properties that have
636657
/// attached property wrappers.
637658
class PropertyWrapperBackingPropertyInfoRequest :

‎include/swift/AST/TypeCheckerTypeIDZone.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,9 @@ SWIFT_REQUEST(TypeChecker, PropertyWrapperBackingPropertyInfoRequest,
150150
NoLocationInfo)
151151
SWIFT_REQUEST(TypeChecker, PropertyWrapperBackingPropertyTypeRequest,
152152
Type(VarDecl *), Cached, NoLocationInfo)
153+
SWIFT_REQUEST(TypeChecker, PropertyWrapperLValuenessRequest,
154+
Optional<PropertyWrapperLValueness>(VarDecl *), Cached,
155+
NoLocationInfo)
153156
SWIFT_REQUEST(TypeChecker, PropertyWrapperMutabilityRequest,
154157
Optional<PropertyWrapperMutability>(VarDecl *), Cached,
155158
NoLocationInfo)

‎lib/AST/TypeCheckRequests.cpp

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

576+
bool PropertyWrapperLValuenessRequest::isCached() const {
577+
auto var = std::get<0>(getStorage());
578+
return !var->getAttrs().isEmpty();
579+
}
580+
576581
void swift::simple_display(
577582
llvm::raw_ostream &out, const PropertyWrapperTypeInfo &propertyWrapper) {
578583
out << "{ ";
@@ -615,6 +620,14 @@ void swift::simple_display(llvm::raw_ostream &os, PropertyWrapperMutability m) {
615620
os << "getter " << names[m.Getter] << ", setter " << names[m.Setter];
616621
}
617622

623+
void swift::simple_display(llvm::raw_ostream &out, PropertyWrapperLValueness l) {
624+
out << "is lvalue for get: {";
625+
simple_display(out, l.isLValueForGetAccess);
626+
out << "}, is lvalue for set: {";
627+
simple_display(out, l.isLValueForSetAccess);
628+
out << "}";
629+
}
630+
618631
void swift::simple_display(llvm::raw_ostream &out,
619632
const ResilienceExpansion &value) {
620633
out << value;

‎lib/Sema/TypeCheckStorage.cpp

Lines changed: 157 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,15 @@ getEnclosingSelfPropertyWrapperAccess(VarDecl *property, bool forProjected) {
592592
return result;
593593
}
594594

595+
static Optional<PropertyWrapperLValueness>
596+
getPropertyWrapperLValueness(VarDecl *var) {
597+
auto &ctx = var->getASTContext();
598+
return evaluateOrDefault(
599+
ctx.evaluator,
600+
PropertyWrapperLValuenessRequest{var},
601+
None);
602+
}
603+
595604
/// Build a reference to the storage of a declaration. Returns nullptr if there
596605
/// was an error. This should only occur if an invalid declaration was type
597606
/// checked; another diagnostic should have been emitted already.
@@ -606,9 +615,11 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
606615
// Local function to "finish" the expression, creating a member reference
607616
// to the given sequence of underlying variables.
608617
Optional<EnclosingSelfPropertyWrapperAccess> enclosingSelfAccess;
609-
llvm::TinyPtrVector<VarDecl *> underlyingVars;
618+
llvm::SmallVector<std::pair<VarDecl *, bool>, 1> underlyingVars;
610619
auto finish = [&](Expr *result) -> Expr * {
611-
for (auto underlyingVar : underlyingVars) {
620+
for (auto underlyingVarPair : underlyingVars) {
621+
auto underlyingVar = underlyingVarPair.first;
622+
auto isWrapperRefLValue = underlyingVarPair.second;
612623
auto subs = result->getType()
613624
->getWithoutSpecifierType()
614625
->getContextSubstitutionMap(
@@ -619,7 +630,7 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
619630
auto *memberRefExpr = new (ctx) MemberRefExpr(
620631
result, SourceLoc(), memberRef, DeclNameLoc(), /*Implicit=*/true);
621632
auto type = underlyingVar->getValueInterfaceType().subst(subs);
622-
if (isLValue)
633+
if (isWrapperRefLValue)
623634
type = LValueType::get(type);
624635
memberRefExpr->setType(type);
625636

@@ -635,6 +646,8 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
635646
SelfAccessorKind selfAccessKind;
636647
Type selfTypeForAccess = (selfDecl ? selfDecl->getType() : Type());
637648

649+
bool isMemberLValue = isLValue;
650+
638651
auto *genericEnv = accessor->getGenericEnvironment();
639652
SubstitutionMap subs;
640653
if (genericEnv)
@@ -709,22 +722,42 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
709722
firstWrapperIdx = 1;
710723

711724
// Perform accesses to the wrappedValues along the composition chain.
712-
for (unsigned i : range(firstWrapperIdx, lastWrapperIdx)) {
713-
auto wrapperInfo = var->getAttachedPropertyWrapperTypeInfo(i);
714-
auto wrappedValue = wrapperInfo.valueVar;
715-
716-
// Check for availability of wrappedValue.
717-
if (accessor->getAccessorKind() == AccessorKind::Get ||
718-
accessor->getAccessorKind() == AccessorKind::Read) {
719-
if (wrappedValue->getAttrs().getUnavailable(ctx)) {
720-
diagnoseExplicitUnavailability(
721-
wrappedValue,
722-
var->getAttachedPropertyWrappers()[i]->getRangeWithAt(),
723-
var->getDeclContext(), nullptr);
725+
if (firstWrapperIdx < lastWrapperIdx) {
726+
if (auto lvalueness = getPropertyWrapperLValueness(var)) {
727+
728+
// Figure out if the outermost wrapper instance should be an l-value
729+
bool isLValueForGet = lvalueness->isLValueForGetAccess[firstWrapperIdx];
730+
bool isLValueForSet = lvalueness->isLValueForSetAccess[firstWrapperIdx];
731+
isMemberLValue = (isLValueForGet && isUsedForGetAccess) ||
732+
(isLValueForSet && isUsedForSetAccess);
733+
734+
for (unsigned i : range(firstWrapperIdx, lastWrapperIdx)) {
735+
auto wrapperInfo = var->getAttachedPropertyWrapperTypeInfo(i);
736+
auto wrappedValue = wrapperInfo.valueVar;
737+
738+
// Figure out if the wrappedValue accesses should be l-values
739+
bool isWrapperRefLValue = isLValue;
740+
if (i < lastWrapperIdx - 1) {
741+
bool isLValueForGet = lvalueness->isLValueForGetAccess[i+1];
742+
bool isLValueForSet = lvalueness->isLValueForSetAccess[i+1];
743+
isWrapperRefLValue = (isLValueForGet && isUsedForGetAccess) ||
744+
(isLValueForSet && isUsedForSetAccess);
745+
}
746+
747+
// Check for availability of wrappedValue.
748+
if (accessor->getAccessorKind() == AccessorKind::Get ||
749+
accessor->getAccessorKind() == AccessorKind::Read) {
750+
if (wrappedValue->getAttrs().getUnavailable(ctx)) {
751+
diagnoseExplicitUnavailability(
752+
wrappedValue,
753+
var->getAttachedPropertyWrappers()[i]->getRangeWithAt(),
754+
var->getDeclContext(), nullptr);
755+
}
756+
}
757+
758+
underlyingVars.push_back({ wrappedValue, isWrapperRefLValue });
724759
}
725760
}
726-
727-
underlyingVars.push_back(wrappedValue);
728761
}
729762
semantics = AccessSemantics::DirectToStorage;
730763
selfAccessKind = SelfAccessorKind::Peer;
@@ -745,8 +778,15 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
745778
enclosingSelfAccess =
746779
getEnclosingSelfPropertyWrapperAccess(var, /*forProjected=*/true);
747780
if (!enclosingSelfAccess) {
781+
auto projectionVar = cast<VarDecl>(accessor->getStorage());
782+
if (auto lvalueness = getPropertyWrapperLValueness(projectionVar)) {
783+
isMemberLValue =
784+
(lvalueness->isLValueForGetAccess[0] && isUsedForGetAccess) ||
785+
(lvalueness->isLValueForSetAccess[0] && isUsedForSetAccess);
786+
}
748787
underlyingVars.push_back(
749-
var->getAttachedPropertyWrapperTypeInfo(0).projectedValueVar);
788+
{ var->getAttachedPropertyWrapperTypeInfo(0).projectedValueVar,
789+
isLValue });
750790
}
751791
semantics = AccessSemantics::DirectToStorage;
752792
selfAccessKind = SelfAccessorKind::Peer;
@@ -802,25 +842,6 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
802842

803843
// Build self.member or equivalent
804844

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

2497+
Optional<PropertyWrapperLValueness>
2498+
PropertyWrapperLValuenessRequest::evaluate(Evaluator &,
2499+
VarDecl *var) const {
2500+
VarDecl *VD = var;
2501+
unsigned numWrappers = var->getAttachedPropertyWrappers().size();
2502+
bool isProjectedValue = false;
2503+
if (numWrappers < 1) {
2504+
VD = var->getOriginalWrappedProperty(
2505+
PropertyWrapperSynthesizedPropertyKind::StorageWrapper);
2506+
numWrappers = 1; // Can't compose projected values
2507+
isProjectedValue = true;
2508+
}
2509+
2510+
if (!VD)
2511+
return None;
2512+
2513+
auto varMember = isProjectedValue
2514+
? &PropertyWrapperTypeInfo::projectedValueVar
2515+
: &PropertyWrapperTypeInfo::valueVar;
2516+
2517+
// Calling the getter (or setter) on the nth property wrapper in the chain
2518+
// is done as follows:
2519+
// 1. call the getter on the (n-1)th property wrapper instance to get the
2520+
// nth property wrapper instance
2521+
// 2. call the getter (or setter) on the nth property wrapper instance
2522+
// 3. if (2) is a mutating access, call the setter on the (n-1)th property
2523+
// wrapper instance to write back the mutated value
2524+
2525+
// Below, we determine which of these property wrapper instances need to be
2526+
// accessed mutating-ly, and therefore should be l-values.
2527+
2528+
// The variables used are:
2529+
//
2530+
// - prevWrappersMutabilityForGet:
2531+
//
2532+
// The mutability needed for the first (n-1) wrapper instances to be
2533+
// able to call the getter on the (n-1)th instance, for step (1) above
2534+
//
2535+
// - prevWrappersMutabilityForGetAndSet:
2536+
//
2537+
// The mutability needed for the first (n-1) wrapper instances to be
2538+
// able to call both the getter and setter on the (n-1)th instance, for
2539+
// steps (1) and (3) above
2540+
//
2541+
// - mutability:
2542+
//
2543+
// The mutability needed for calling the getter / setter on the
2544+
// nth wrapper instance, for step (2) above.
2545+
2546+
llvm::SmallVector<PropertyWrapperMutability::Value, 1>
2547+
prevWrappersMutabilityForGet, prevWrappersMutabilityForGetAndSet;
2548+
PropertyWrapperMutability mutability;
2549+
2550+
auto firstWrapperInfo = VD->getAttachedPropertyWrapperTypeInfo(0);
2551+
mutability.Getter = getGetterMutatingness(firstWrapperInfo.*varMember);
2552+
mutability.Setter = getSetterMutatingness(firstWrapperInfo.*varMember,
2553+
var->getInnermostDeclContext());
2554+
2555+
for (unsigned i : range(1, numWrappers)) {
2556+
if (mutability.Getter == PropertyWrapperMutability::Mutating) {
2557+
prevWrappersMutabilityForGet = prevWrappersMutabilityForGetAndSet;
2558+
}
2559+
if (mutability.Getter != PropertyWrapperMutability::Mutating &&
2560+
mutability.Setter != PropertyWrapperMutability::Mutating) {
2561+
prevWrappersMutabilityForGetAndSet = prevWrappersMutabilityForGet;
2562+
}
2563+
prevWrappersMutabilityForGet.push_back(mutability.Getter);
2564+
prevWrappersMutabilityForGetAndSet.push_back(
2565+
std::max(mutability.Getter, mutability.Setter));
2566+
auto wrapperInfo = VD->getAttachedPropertyWrapperTypeInfo(i);
2567+
mutability.Getter = getGetterMutatingness(wrapperInfo.*varMember);
2568+
mutability.Setter = getSetterMutatingness(wrapperInfo.*varMember,
2569+
var->getInnermostDeclContext());
2570+
}
2571+
2572+
auto mutabilitySequenceForLastGet =
2573+
(mutability.Getter == PropertyWrapperMutability::Mutating)
2574+
? &prevWrappersMutabilityForGetAndSet
2575+
: &prevWrappersMutabilityForGet;
2576+
auto mutabilitySequenceForLastSet =
2577+
(mutability.Setter == PropertyWrapperMutability::Mutating)
2578+
? &prevWrappersMutabilityForGetAndSet
2579+
: &prevWrappersMutabilityForGet;
2580+
2581+
PropertyWrapperLValueness lvalueness;
2582+
for (unsigned i : range(numWrappers - 1)) {
2583+
lvalueness.isLValueForGetAccess.push_back(
2584+
(*mutabilitySequenceForLastGet)[i] == PropertyWrapperMutability::Mutating);
2585+
lvalueness.isLValueForSetAccess.push_back(
2586+
(*mutabilitySequenceForLastSet)[i] == PropertyWrapperMutability::Mutating);
2587+
}
2588+
lvalueness.isLValueForGetAccess.push_back(
2589+
mutability.Getter == PropertyWrapperMutability::Mutating);
2590+
lvalueness.isLValueForSetAccess.push_back(
2591+
mutability.Setter == PropertyWrapperMutability::Mutating);
2592+
2593+
return lvalueness;
2594+
}
2595+
24762596
PropertyWrapperBackingPropertyInfo
24772597
PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
24782598
VarDecl *var) const {

0 commit comments

Comments
 (0)