Skip to content

Commit 40d1146

Browse files
xedinxedin
authored
Merge pull request #61427 from xedin/type-wrappers-add-support-for-let-properties
[Sema/DI] TypeWrappers: Add support for `let` properties
2 parents 6afeeb6 + 7bcd8ff commit 40d1146

13 files changed

+456
-20
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6571,6 +6571,17 @@ ERROR(type_wrapper_requires_subscript,none,
65716571
" - subscript(storedKeyPath:)",
65726572
(DeclName))
65736573

6574+
ERROR(type_wrapper_requires_readonly_subscript,none,
6575+
"type wrapper type %0 does not contain a required ready-only subscript",
6576+
(DeclName))
6577+
6578+
ERROR(type_wrapper_requires_writable_subscript,none,
6579+
"type wrapper type %0 does not contain a required writable subscript",
6580+
(DeclName))
6581+
6582+
NOTE(add_type_wrapper_subscript_stub_note,none,
6583+
"do you want to add a stub?", ())
6584+
65746585
ERROR(type_wrapper_failable_init,none,
65756586
"type wrapper initializer %0 cannot be failable", (DeclName))
65766587

‎lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -468,22 +468,35 @@ DIMemoryObjectInfo::getPathStringToElement(unsigned Element,
468468

469469
/// If the specified value is a 'let' property in an initializer, return true.
470470
bool DIMemoryObjectInfo::isElementLetProperty(unsigned Element) const {
471-
// If we aren't representing 'self' in a non-delegating initializer, then we
472-
// can't have 'let' properties.
473-
if (!isNonDelegatingInit())
474-
return IsLet;
475-
476-
auto &Module = MemoryInst->getModule();
471+
NullablePtr<NominalTypeDecl> NTD;
472+
473+
// If this is an element of a `_storage` tuple, we need to
474+
// check the `$Storage` to determine whether underlying storage
475+
// backing element is immutable.
476+
if (auto *storageVar = getAsTypeWrapperLocalStorageVar()) {
477+
auto *wrappedType = cast<NominalTypeDecl>(
478+
storageVar->getDeclContext()->getInnermostTypeContext());
479+
assert(wrappedType && "_storage reference without type wrapper");
480+
NTD = wrappedType->getTypeWrapperStorageDecl();
481+
} else {
482+
// If we aren't representing 'self' in a non-delegating initializer, then we
483+
// can't have 'let' properties.
484+
if (!isNonDelegatingInit())
485+
return IsLet;
486+
487+
NTD = MemorySILType.getNominalOrBoundGenericNominal();
488+
}
477489

478-
auto *NTD = MemorySILType.getNominalOrBoundGenericNominal();
479490
if (!NTD) {
480491
// Otherwise, we miscounted elements?
481492
assert(Element == 0 && "Element count problem");
482493
return false;
483494
}
484495

496+
auto &Module = MemoryInst->getModule();
497+
485498
auto expansionContext = TypeExpansionContext(*MemoryInst->getFunction());
486-
for (auto *VD : NTD->getStoredProperties()) {
499+
for (auto *VD : NTD.get()->getStoredProperties()) {
487500
auto FieldType = MemorySILType.getFieldType(VD, Module, expansionContext);
488501
unsigned NumFieldElements =
489502
getElementCountRec(expansionContext, Module, FieldType, false);

‎lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3869,8 +3869,21 @@ static bool checkDefiniteInitialization(SILFunction &Fn) {
38693869
// It has to be checked first because it injects initialization of
38703870
// `self.$storage`.
38713871
if (auto *storageVar = findLocalTypeWrapperStorageVar(Fn)) {
3872-
// Then process the memory object.
3873-
processMemoryObject(storageVar, blockStates);
3872+
{
3873+
auto &M = Fn.getModule();
3874+
3875+
DiagnosticTransaction T(M.getASTContext().Diags);
3876+
3877+
// process `_storage` object.
3878+
processMemoryObject(storageVar, blockStates);
3879+
3880+
// Stop if `_storage` initialization checking produced
3881+
// errors, otherwise DI is going to emit confusing
3882+
// "return without fully initialized self - self.$storage" error.
3883+
if (T.hasErrors())
3884+
return true;
3885+
}
3886+
38743887
storageVar->replaceAllUsesWith(storageVar->getOperand());
38753888
storageVar->eraseFromParent();
38763889
Changed = true;

‎lib/Sema/CSApply.cpp

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,6 +1651,56 @@ namespace {
16511651

16521652
// For properties, build member references.
16531653
if (auto *varDecl = dyn_cast<VarDecl>(member)) {
1654+
// \returns result of the given function type
1655+
auto resultType = [](Type fnTy) -> Type {
1656+
return fnTy->castTo<FunctionType>()->getResult();
1657+
};
1658+
1659+
auto isAssignmentDestination = [&](ConstraintLocatorBuilder locator) {
1660+
if (auto *anchor = getAsExpr(locator.getAnchor())) {
1661+
if (auto *assignment =
1662+
getAsExpr<AssignExpr>(cs.getParentExpr(anchor)))
1663+
return assignment->getDest() == anchor;
1664+
}
1665+
return false;
1666+
};
1667+
1668+
// If this is a reference to an immutable type wrapper
1669+
// managed property used as an assignment destination
1670+
// i.e. `self.<name> = ...` in an initializer context,
1671+
// let's rewrite member access from `self` to `_storage`
1672+
// injected by the compiler to support type wrapper
1673+
// initialization.
1674+
//
1675+
// Note that this is safe to do only for immutable
1676+
// properties because they do no support re-assignment.
1677+
if (isa<ConstructorDecl>(cs.DC) && varDecl->isLet() &&
1678+
varDecl->isAccessedViaTypeWrapper() &&
1679+
isAssignmentDestination(memberLocator)) {
1680+
auto *ctor = cast<ConstructorDecl>(cs.DC);
1681+
auto *storageVar = ctor->getLocalTypeWrapperStorageVar();
1682+
auto *storageVarTy =
1683+
storageVar->getInterfaceType()->castTo<TupleType>();
1684+
1685+
base =
1686+
new (context) DeclRefExpr(storageVar, DeclNameLoc(base->getLoc()),
1687+
/*implicit=*/true);
1688+
base->setType(
1689+
LValueType::get(ctor->mapTypeIntoContext(storageVarTy)));
1690+
1691+
cs.cacheType(base);
1692+
1693+
Expr *memberRefExpr = new (context) TupleElementExpr(
1694+
base, /*DotLoc=*/SourceLoc(),
1695+
storageVarTy->getNamedElementId(varDecl->getName()),
1696+
memberLoc.getBaseNameLoc(), resultType(refTy));
1697+
memberRefExpr->setImplicit();
1698+
1699+
cs.cacheType(memberRefExpr);
1700+
1701+
return forceUnwrapIfExpected(memberRefExpr, memberLocator);
1702+
}
1703+
16541704
if (isUnboundInstanceMember) {
16551705
assert(memberLocator.getBaseLocator() &&
16561706
cs.UnevaluatedRootExprs.count(
@@ -1677,11 +1727,6 @@ namespace {
16771727
containerTy, 1);
16781728
}
16791729

1680-
// \returns result of the given function type
1681-
auto resultType = [](Type fnTy) -> Type {
1682-
return fnTy->castTo<FunctionType>()->getResult();
1683-
};
1684-
16851730
cs.setType(memberRefExpr, resultType(refTy));
16861731

16871732
Expr *result = memberRefExpr;

‎lib/Sema/TypeCheckAttr.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3771,6 +3771,9 @@ void AttributeChecker::visitTypeWrapperAttr(TypeWrapperAttr *attr) {
37713771
llvm::SmallDenseMap<SubscriptDecl *, SmallVector<UnviabilityReason, 2>, 2>
37723772
nonViableSubscripts;
37733773

3774+
bool hasReadOnly = false;
3775+
bool hasWritable = false;
3776+
37743777
for (auto *decl : subscripts) {
37753778
auto *subscript = cast<SubscriptDecl>(decl);
37763779

@@ -3782,6 +3785,10 @@ void AttributeChecker::visitTypeWrapperAttr(TypeWrapperAttr *attr) {
37823785
BGT->isReferenceWritableKeyPath())) {
37833786
nonViableSubscripts[subscript].push_back(
37843787
UnviabilityReason::InvalidType);
3788+
} else {
3789+
hasReadOnly |= BGT->isKeyPath();
3790+
hasWritable |=
3791+
BGT->isWritableKeyPath() || BGT->isReferenceWritableKeyPath();
37853792
}
37863793
} else {
37873794
nonViableSubscripts[subscript].push_back(
@@ -3793,6 +3800,41 @@ void AttributeChecker::visitTypeWrapperAttr(TypeWrapperAttr *attr) {
37933800
UnviabilityReason::Inaccessible);
37943801
}
37953802

3803+
if (!hasReadOnly) {
3804+
auto &DE = ctx.Diags;
3805+
DE.diagnoseWithNotes(
3806+
DE.diagnose(nominal->getLoc(),
3807+
diag::type_wrapper_requires_readonly_subscript,
3808+
nominal->getName()),
3809+
[&]() {
3810+
DE.diagnose(nominal->getLoc(),
3811+
diag::add_type_wrapper_subscript_stub_note)
3812+
.fixItInsertAfter(
3813+
nominal->getBraces().Start,
3814+
"\nsubscript<Value>(storageKeyPath path: KeyPath<<#Base#>, "
3815+
"Value>) -> Value { get { <#code#> } }");
3816+
});
3817+
attr->setInvalid();
3818+
}
3819+
3820+
if (!hasWritable) {
3821+
auto &DE = ctx.Diags;
3822+
DE.diagnoseWithNotes(
3823+
DE.diagnose(nominal->getLoc(),
3824+
diag::type_wrapper_requires_writable_subscript,
3825+
nominal->getName()),
3826+
[&]() {
3827+
DE.diagnose(nominal->getLoc(),
3828+
diag::add_type_wrapper_subscript_stub_note)
3829+
.fixItInsertAfter(
3830+
nominal->getBraces().Start,
3831+
"\nsubscript<Value>(storageKeyPath path: "
3832+
"WritableKeyPath<<#Base#>, "
3833+
"Value>) -> Value { get { <#code#> } set { <#code#> } }");
3834+
});
3835+
attr->setInvalid();
3836+
}
3837+
37963838
if (subscripts.size() - nonViableSubscripts.size() == 0) {
37973839
for (const auto &entry : nonViableSubscripts) {
37983840
auto *subscript = entry.first;

‎lib/Sema/TypeCheckStorage.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3306,7 +3306,8 @@ static void finishStorageImplInfo(AbstractStorageDecl *storage,
33063306
} else if (var->hasAttachedPropertyWrapper()) {
33073307
finishPropertyWrapperImplInfo(var, info);
33083308
} else if (var->isAccessedViaTypeWrapper()) {
3309-
info = StorageImplInfo::getMutableComputed();
3309+
info = var->isLet() ? StorageImplInfo::getImmutableComputed()
3310+
: StorageImplInfo::getMutableComputed();
33103311
}
33113312
}
33123313

‎lib/Sema/TypeCheckTypeWrapper.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ bool IsPropertyAccessedViaTypeWrapper::evaluate(Evaluator &evaluator,
325325
if (!(parent && parent->hasTypeWrapper()))
326326
return false;
327327

328-
if (property->isStatic() || property->isLet())
328+
if (property->isStatic())
329329
return false;
330330

331331
// If this property has `@typeWrapperIgnored` attribute

‎test/Interpreter/Inputs/type_wrapper_defs.swift

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@ public struct Wrapper<S> {
77
self.underlying = memberwise
88
}
99

10+
public subscript<V>(storageKeyPath path: KeyPath<S, V>) -> V {
11+
get {
12+
print("in read-only getter")
13+
return underlying[keyPath: path]
14+
}
15+
}
16+
1017
public subscript<V>(storageKeyPath path: WritableKeyPath<S, V>) -> V {
1118
get {
1219
print("in getter")
@@ -303,3 +310,31 @@ public class ClassWithConvenienceInit<T> {
303310
print(self.b)
304311
}
305312
}
313+
314+
@Wrapper
315+
public struct TypeWithLetProperties<T> {
316+
let a: T
317+
let b: Int
318+
319+
public init(a: T, b: Int? = nil, onSet: (() -> Void)? = nil) {
320+
self.a = a
321+
if let b {
322+
self.b = b
323+
} else {
324+
self.b = 0
325+
}
326+
327+
print("--Before onSet--")
328+
329+
print(self.a)
330+
print(self.b)
331+
332+
if let onSet {
333+
onSet()
334+
335+
print("--After onSet--")
336+
print(self.a)
337+
print(self.b)
338+
}
339+
}
340+
}

‎test/Interpreter/type_wrappers.swift

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,10 +358,11 @@ testPropertyWrappers()
358358

359359
do {
360360
var person = PersonWithUnmanagedTest(name: "Arthur Dent")
361-
// CHECK: Wrapper.init($Storage(_favoredColor: type_wrapper_defs.PropWrapper<Swift.String>(value: "red")))
361+
// CHECK: Wrapper.init($Storage(name: "Arthur Dent", _favoredColor: type_wrapper_defs.PropWrapper<Swift.String>(value: "red")))
362362

363363
print(person.name)
364-
// CHECK: Arthur Dent
364+
// CHECK: in read-only getter
365+
// CHECK-NEXT: Arthur Dent
365366

366367
print(person.age)
367368
// CHECK: 30
@@ -587,3 +588,38 @@ do {
587588
// CHECK-NEXT: in getter
588589
// CHECK-NEXT: Optional((-1, "ultimate question", (a: 1, b: 2.0, c: 3)))
589590
}
591+
592+
do {
593+
class X : CustomStringConvertible {
594+
var x: [Int] = []
595+
596+
var description: String {
597+
"X(x: \(x))"
598+
}
599+
}
600+
601+
var arg = X()
602+
603+
let test1 = TypeWithLetProperties(a: arg, b: 42) {
604+
arg.x.append(1)
605+
}
606+
// CHECK: Wrapper.init($Storage(a: X(x: []), b: 42))
607+
// CHECK-NEXT: --Before onSet--
608+
// CHECK-NEXT: in read-only getter
609+
// CHECK-NEXT: X(x: [])
610+
// CHECK-NEXT: in read-only getter
611+
// CHECK-NEXT: 42
612+
// CHECK-NEXT: --After onSet--
613+
// CHECK-NEXT: in read-only getter
614+
// CHECK-NEXT: X(x: [1])
615+
// CHECK-NEXT: in read-only getter
616+
// CHECK-nEXT: 42
617+
618+
let test2 = TypeWithLetProperties(a: Optional.some([1, 2, 3]))
619+
// CHECK: Wrapper.init($Storage(a: Optional([1, 2, 3]), b: 0))
620+
// CHECK-NEXT: --Before onSet--
621+
// CHECK-NEXT: in read-only getter
622+
// CHECK-NEXT: Optional([1, 2, 3])
623+
// CHECK-NEXT: in read-only getter
624+
// CHECK-NEXT: 0
625+
}

0 commit comments

Comments
 (0)