Skip to content

[Sema] Correct lvalue computation in buildStorageRef for synthesizing local accessors. #34230

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.

Sign up for GitHub

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 2 commits into from
Oct 8, 2020
Merged

[Sema] Correct lvalue computation in buildStorageRef for synthesizing local accessors. #34230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
456a505
[Sema] Adjust mutability in buildStorageRef appropriately for accessors
hborla Oct 7, 2020
0c6ef58
[DeclChecker] Explicitly compute captures for local functions in the …
hborla Oct 7, 2020
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 lib/Sema/TypeCheckDeclPrimary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2362,6 +2362,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
} else if (FD->getDeclContext()->isLocalContext()) {
// Check local function bodies right away.
(void)FD->getTypecheckedBody();
TypeChecker::computeCaptures(FD);
} else if (shouldSkipBodyTypechecking(FD)) {
FD->setBodySkipped(FD->getBodySourceRange());
} else {
Expand Down
55 changes: 25 additions & 30 deletions lib/Sema/TypeCheckStorage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,45 +830,42 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
}
}

// If the base is not 'self', default get access to nonmutating and set access to mutating.
bool getterMutatesBase = selfDecl && storage->isGetterMutating();
bool setterMutatesBase = !selfDecl || storage->isSetterMutating();
// If we're not accessing via a property wrapper, we don't need to adjust
// the mutability.
if (target == TargetImpl::Wrapper || target == TargetImpl::WrapperStorage) {
auto var = cast<VarDecl>(accessor->getStorage());
auto mutability = var->getPropertyWrapperMutability();
// Only adjust mutability if it's possible to mutate the base.
if (mutability && !var->isStatic() &&
!(selfDecl && selfTypeForAccess->hasReferenceSemantics())) {
getterMutatesBase = (mutability->Getter == PropertyWrapperMutability::Mutating);
setterMutatesBase = (mutability->Setter == PropertyWrapperMutability::Mutating);
}
}

// If the accessor is mutating, then the base should be referred as an l-value
bool isBaseLValue = (getterMutatesBase && isUsedForGetAccess) ||
(setterMutatesBase && isUsedForSetAccess);

if (!selfDecl) {
assert(target != TargetImpl::Super);
auto *storageDRE = new (ctx) DeclRefExpr(storage, DeclNameLoc(),
/*IsImplicit=*/true, semantics);
auto type = storage->getValueInterfaceType().subst(subs);
if (isLValue)
if (isBaseLValue)
type = LValueType::get(type);
storageDRE->setType(type);

return finish(storageDRE);
}

// Build self

bool isGetterMutating = storage->isGetterMutating();
bool isSetterMutating = storage->isSetterMutating();
// If we're not accessing via a property wrapper, we don't need to adjust
// the mutability.
if (target == TargetImpl::Wrapper || target == TargetImpl::WrapperStorage) {
auto var = cast<VarDecl>(accessor->getStorage());
if (auto mutability = var->getPropertyWrapperMutability()) {
// We consider the storage's mutability too because the wrapped property
// might be part of a class, in case of which nothing is mutating.
isGetterMutating = (mutability->Getter == PropertyWrapperMutability::Mutating)
? (storage->isGetterMutating() || storage->isSetterMutating())
: storage->isGetterMutating();
isSetterMutating = (mutability->Setter == PropertyWrapperMutability::Mutating)
? (storage->isGetterMutating() || storage->isSetterMutating())
: storage->isGetterMutating();
}
}

// If the accessor is mutating, then self should be referred as an l-value
bool isSelfLValue = (isGetterMutating && isUsedForGetAccess) ||
(isSetterMutating && isUsedForSetAccess);

Expr *selfDRE = buildSelfReference(selfDecl, selfAccessKind, isSelfLValue,
Expr *selfDRE = buildSelfReference(selfDecl, selfAccessKind, isBaseLValue,
/*convertTy*/ selfTypeForAccess);
if (isSelfLValue)
if (isBaseLValue)
selfTypeForAccess = LValueType::get(selfTypeForAccess);

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

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

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

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

static std::pair<BraceStmt *, bool>
Expand Down