Skip to content

Commit a7a47e5

Browse files
authored
Merge pull request #34230 from hborla/build-storage-ref-lvalue
[Sema] Correct lvalue computation in `buildStorageRef` for synthesizing local accessors.
2 parents 15fc0fe + 0c6ef58 commit a7a47e5

File tree

2 files changed

+26
-30
lines changed

2 files changed

+26
-30
lines changed

lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2362,6 +2362,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
23622362
} else if (FD->getDeclContext()->isLocalContext()) {
23632363
// Check local function bodies right away.
23642364
(void)FD->getTypecheckedBody();
2365+
TypeChecker::computeCaptures(FD);
23652366
} else if (shouldSkipBodyTypechecking(FD)) {
23662367
FD->setBodySkipped(FD->getBodySourceRange());
23672368
} else {

lib/Sema/TypeCheckStorage.cpp

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -830,45 +830,42 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
830830
}
831831
}
832832

833+
// If the base is not 'self', default get access to nonmutating and set access to mutating.
834+
bool getterMutatesBase = selfDecl && storage->isGetterMutating();
835+
bool setterMutatesBase = !selfDecl || storage->isSetterMutating();
836+
// If we're not accessing via a property wrapper, we don't need to adjust
837+
// the mutability.
838+
if (target == TargetImpl::Wrapper || target == TargetImpl::WrapperStorage) {
839+
auto var = cast<VarDecl>(accessor->getStorage());
840+
auto mutability = var->getPropertyWrapperMutability();
841+
// Only adjust mutability if it's possible to mutate the base.
842+
if (mutability && !var->isStatic() &&
843+
!(selfDecl && selfTypeForAccess->hasReferenceSemantics())) {
844+
getterMutatesBase = (mutability->Getter == PropertyWrapperMutability::Mutating);
845+
setterMutatesBase = (mutability->Setter == PropertyWrapperMutability::Mutating);
846+
}
847+
}
848+
849+
// If the accessor is mutating, then the base should be referred as an l-value
850+
bool isBaseLValue = (getterMutatesBase && isUsedForGetAccess) ||
851+
(setterMutatesBase && isUsedForSetAccess);
852+
833853
if (!selfDecl) {
834854
assert(target != TargetImpl::Super);
835855
auto *storageDRE = new (ctx) DeclRefExpr(storage, DeclNameLoc(),
836856
/*IsImplicit=*/true, semantics);
837857
auto type = storage->getValueInterfaceType().subst(subs);
838-
if (isLValue)
858+
if (isBaseLValue)
839859
type = LValueType::get(type);
840860
storageDRE->setType(type);
841861

842862
return finish(storageDRE);
843863
}
844864

845865
// Build self
846-
847-
bool isGetterMutating = storage->isGetterMutating();
848-
bool isSetterMutating = storage->isSetterMutating();
849-
// If we're not accessing via a property wrapper, we don't need to adjust
850-
// the mutability.
851-
if (target == TargetImpl::Wrapper || target == TargetImpl::WrapperStorage) {
852-
auto var = cast<VarDecl>(accessor->getStorage());
853-
if (auto mutability = var->getPropertyWrapperMutability()) {
854-
// We consider the storage's mutability too because the wrapped property
855-
// might be part of a class, in case of which nothing is mutating.
856-
isGetterMutating = (mutability->Getter == PropertyWrapperMutability::Mutating)
857-
? (storage->isGetterMutating() || storage->isSetterMutating())
858-
: storage->isGetterMutating();
859-
isSetterMutating = (mutability->Setter == PropertyWrapperMutability::Mutating)
860-
? (storage->isGetterMutating() || storage->isSetterMutating())
861-
: storage->isGetterMutating();
862-
}
863-
}
864-
865-
// If the accessor is mutating, then self should be referred as an l-value
866-
bool isSelfLValue = (isGetterMutating && isUsedForGetAccess) ||
867-
(isSetterMutating && isUsedForSetAccess);
868-
869-
Expr *selfDRE = buildSelfReference(selfDecl, selfAccessKind, isSelfLValue,
866+
Expr *selfDRE = buildSelfReference(selfDecl, selfAccessKind, isBaseLValue,
870867
/*convertTy*/ selfTypeForAccess);
871-
if (isSelfLValue)
868+
if (isBaseLValue)
872869
selfTypeForAccess = LValueType::get(selfTypeForAccess);
873870

874871
if (!selfDRE->getType()->isEqual(selfTypeForAccess)) {
@@ -1169,9 +1166,8 @@ synthesizeTrivialGetterBody(AccessorDecl *getter, TargetImpl target,
11691166
body.push_back(returnStmt);
11701167
}
11711168

1172-
// Don't mark local accessors as type-checked - captures still need to be computed.
11731169
return { BraceStmt::create(ctx, loc, body, loc, true),
1174-
/*isTypeChecked=*/!getter->getDeclContext()->isLocalContext() };
1170+
/*isTypeChecked=*/true };
11751171
}
11761172

11771173
/// Synthesize the body of a getter which just directly accesses the
@@ -1446,9 +1442,8 @@ synthesizeTrivialSetterBodyWithStorage(AccessorDecl *setter,
14461442

14471443
createPropertyStoreOrCallSuperclassSetter(setter, valueDRE, storageToUse,
14481444
target, setterBody, ctx);
1449-
// Don't mark local accessors as type-checked - captures still need to be computed.
14501445
return { BraceStmt::create(ctx, loc, setterBody, loc, true),
1451-
/*isTypeChecked=*/!setter->getDeclContext()->isLocalContext() };
1446+
/*isTypeChecked=*/true };
14521447
}
14531448

14541449
static std::pair<BraceStmt *, bool>

0 commit comments

Comments
 (0)