Skip to content

Commit 109ec5d

Browse files
hamishknighthamishknight
authored
Merge pull request swiftlang#30792 from hamishknight/storage-access-codes
[Sema] Fix a couple of property override crashers
2 parents 54a6be9 + 6a7b2e0 commit 109ec5d

File tree

5 files changed

+128
-26
lines changed

5 files changed

+128
-26
lines changed

‎lib/Sema/CodeSynthesis.cpp

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,20 +41,49 @@ const bool IsImplicit = true;
4141

4242
Expr *swift::buildSelfReference(VarDecl *selfDecl,
4343
SelfAccessorKind selfAccessorKind,
44-
bool isLValue,
45-
ASTContext &ctx) {
44+
bool isLValue, Type convertTy) {
45+
auto &ctx = selfDecl->getASTContext();
46+
auto selfTy = selfDecl->getType();
47+
4648
switch (selfAccessorKind) {
4749
case SelfAccessorKind::Peer:
50+
assert(!convertTy || convertTy->isEqual(selfTy));
4851
return new (ctx) DeclRefExpr(selfDecl, DeclNameLoc(), IsImplicit,
4952
AccessSemantics::Ordinary,
50-
isLValue
51-
? LValueType::get(selfDecl->getType())
52-
: selfDecl->getType());
53+
isLValue ? LValueType::get(selfTy) : selfTy);
5354

54-
case SelfAccessorKind::Super:
55+
case SelfAccessorKind::Super: {
5556
assert(!isLValue);
56-
return new (ctx) SuperRefExpr(selfDecl, SourceLoc(), IsImplicit,
57-
selfDecl->getType()->getSuperclass());
57+
58+
// Get the superclass type of self, looking through a metatype if needed.
59+
auto isMetatype = false;
60+
if (auto *metaTy = selfTy->getAs<MetatypeType>()) {
61+
isMetatype = true;
62+
selfTy = metaTy->getInstanceType();
63+
}
64+
selfTy = selfTy->getSuperclass();
65+
if (isMetatype)
66+
selfTy = MetatypeType::get(selfTy);
67+
68+
auto *superRef =
69+
new (ctx) SuperRefExpr(selfDecl, SourceLoc(), IsImplicit, selfTy);
70+
71+
// If no conversion type was specified, or we're already at that type, we're
72+
// done.
73+
if (!convertTy || convertTy->isEqual(selfTy))
74+
return superRef;
75+
76+
// Insert the appropriate expr to handle the upcast.
77+
if (isMetatype) {
78+
assert(convertTy->castTo<MetatypeType>()
79+
->getInstanceType()
80+
->isExactSuperclassOf(selfTy->getMetatypeInstanceType()));
81+
return new (ctx) MetatypeConversionExpr(superRef, convertTy);
82+
} else {
83+
assert(convertTy->isExactSuperclassOf(selfTy));
84+
return new (ctx) DerivedToBaseExpr(superRef, convertTy);
85+
}
86+
}
5887
}
5988
llvm_unreachable("bad self access kind");
6089
}
@@ -537,7 +566,7 @@ synthesizeDesignatedInitOverride(AbstractFunctionDecl *fn, void *context) {
537566
// Reference to super.init.
538567
auto *selfDecl = ctor->getImplicitSelfDecl();
539568
auto *superRef = buildSelfReference(selfDecl, SelfAccessorKind::Super,
540-
/*isLValue=*/false, ctx);
569+
/*isLValue=*/false);
541570

542571
SubstitutionMap subs;
543572
if (auto *genericEnv = fn->getGenericEnvironment())

‎lib/Sema/CodeSynthesis.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,15 @@ enum class SelfAccessorKind {
5050
Super,
5151
};
5252

53-
Expr *buildSelfReference(VarDecl *selfDecl,
54-
SelfAccessorKind selfAccessorKind,
55-
bool isLValue,
56-
ASTContext &ctx);
53+
/// Builds a reference to the \c self decl in a function.
54+
///
55+
/// \param selfDecl The self decl to reference.
56+
/// \param selfAccessorKind The kind of access being performed.
57+
/// \param isLValue Whether the resulting expression is an lvalue.
58+
/// \param convertTy The type of the resulting expression. For a reference to
59+
/// super, this can be a superclass type to upcast to.
60+
Expr *buildSelfReference(VarDecl *selfDecl, SelfAccessorKind selfAccessorKind,
61+
bool isLValue, Type convertTy = Type());
5762

5863
/// Build an expression that evaluates the specified parameter list as a tuple
5964
/// or paren expr, suitable for use in an apply expr.

‎lib/Sema/TypeCheckStorage.cpp

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,13 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
654654
semantics = AccessSemantics::Ordinary;
655655
selfAccessKind = SelfAccessorKind::Super;
656656

657+
auto isMetatype = false;
658+
if (auto *metaTy = selfTypeForAccess->getAs<MetatypeType>()) {
659+
isMetatype = true;
660+
selfTypeForAccess = metaTy->getInstanceType();
661+
}
662+
663+
// Adjust the self type of the access to refer to the relevant superclass.
657664
auto *baseClass = override->getDeclContext()->getSelfClassDecl();
658665
selfTypeForAccess = selfTypeForAccess->getSuperclassForDecl(baseClass);
659666
subs =
@@ -663,6 +670,9 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
663670

664671
storage = override;
665672

673+
if (isMetatype)
674+
selfTypeForAccess = MetatypeType::get(selfTypeForAccess);
675+
666676
// Otherwise do a self-reference, which is dynamically bogus but
667677
// should be statically valid. This should only happen in invalid cases.
668678
} else {
@@ -751,6 +761,11 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
751761
bool isMemberLValue = isLValue;
752762
auto propertyWrapperMutability =
753763
[&](Decl *decl) -> Optional<std::pair<bool, bool>> {
764+
// If we're not accessing via a property wrapper, we don't need to adjust
765+
// the mutability.
766+
if (target != TargetImpl::Wrapper && target != TargetImpl::WrapperStorage)
767+
return None;
768+
754769
auto var = dyn_cast<VarDecl>(decl);
755770
if (!var)
756771
return None;
@@ -781,17 +796,8 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
781796
isSelfLValue |= mut->second;
782797
}
783798

784-
Expr *selfDRE =
785-
buildSelfReference(selfDecl, selfAccessKind, isSelfLValue,
786-
ctx);
787-
if (isSelfLValue)
788-
selfTypeForAccess = LValueType::get(selfTypeForAccess);
789-
790-
if (!selfDRE->getType()->isEqual(selfTypeForAccess)) {
791-
assert(selfAccessKind == SelfAccessorKind::Super);
792-
selfDRE = new (ctx) DerivedToBaseExpr(selfDRE, selfTypeForAccess);
793-
}
794-
799+
Expr *selfDRE = buildSelfReference(selfDecl, selfAccessKind, isSelfLValue,
800+
/*convertTy*/ selfTypeForAccess);
795801
Expr *lookupExpr;
796802
ConcreteDeclRef memberRef(storage, subs);
797803
auto type = storage->getValueInterfaceType().subst(subs);
@@ -1452,8 +1458,8 @@ synthesizeObservedSetterBody(AccessorDecl *Set, TargetImpl target,
14521458
ValueDRE->setType(arg->getType());
14531459

14541460
if (SelfDecl) {
1455-
auto *SelfDRE = buildSelfReference(SelfDecl, SelfAccessorKind::Peer,
1456-
IsSelfLValue, Ctx);
1461+
auto *SelfDRE =
1462+
buildSelfReference(SelfDecl, SelfAccessorKind::Peer, IsSelfLValue);
14571463
SelfDRE = maybeWrapInOutExpr(SelfDRE, Ctx);
14581464
auto *DSCE = new (Ctx) DotSyntaxCallExpr(Callee, SourceLoc(), SelfDRE);
14591465

‎test/SILGen/accessors.swift

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,40 @@ func test_rec2(_ outer: inout Rec2Outer) -> Int {
133133
// CHECK: sil hidden [ossa] @$s9accessors9test_rec2ySiAA9Rec2OuterVzF : $@convention(thin) (@inout Rec2Outer) -> Int {
134134
// CHECK: function_ref @$s9accessors9Rec2OuterV5innerAA0B5InnerVvau : $@convention(method) (@inout Rec2Outer) -> UnsafeMutablePointer<Rec2Inner>
135135

136+
// SR-12456: Compiler crash on class var override adding observer.
137+
class SR12456Base {
138+
open class var instance: SR12456Base {
139+
get {
140+
return SR12456Base()
141+
}
142+
set {}
143+
}
144+
}
145+
146+
class SR12456Subclass : SR12456Base {
147+
override class var instance: SR12456Base {
148+
didSet {}
149+
}
150+
}
151+
152+
// Make sure we can handle more complicated overrides.
153+
class Parent<V> {
154+
class C<T, U> {
155+
class var foo: Int { get { 0 } set {} }
156+
}
157+
class D<T> : C<T, Int> {}
158+
class E : D<Double> {
159+
// CHECK-LABEL: sil hidden [transparent] [ossa] @$s9accessors6ParentC1EC3fooSivgZ : $@convention(method) <V> (@thick Parent<V>.E.Type) -> Int
160+
// CHECK: [[TY:%.+]] = upcast {{%.+}} : $@thick Parent<V>.E.Type to $@thick Parent<V>.D<Double>.Type
161+
// CHECK: [[UPCASTTY:%.+]] = upcast [[TY]] : $@thick Parent<V>.D<Double>.Type to $@thick Parent<V>.C<Double, Int>.Type
162+
// CHECK: [[GETTER:%.+]] = function_ref @$s9accessors6ParentC1CC3fooSivgZ : $@convention(method) <τ_0_0><τ_1_0, τ_1_1> (@thick Parent<τ_0_0>.C<τ_1_0, τ_1_1>.Type) -> Int
163+
// CHECK: apply [[GETTER]]<V, Double, Int>([[UPCASTTY]])
164+
override class var foo: Int {
165+
didSet {}
166+
}
167+
}
168+
}
169+
136170
struct Foo {
137171
private subscript(privateSubscript x: Void) -> Void {
138172
// CHECK-DAG: sil private [ossa] @$s9accessors3FooV16privateSubscriptyyt_tc33_D7F31B09EE737C687DC580B2014D759CLlig : $@convention(method) (Foo) -> () {

‎test/SILGen/property_wrappers.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,34 @@ struct ObservedObject<ObjectType : AnyObject > {
653653
}
654654
}
655655

656+
// SR-12443: Crash on property with wrapper override that adds observer.
657+
@propertyWrapper
658+
struct BasicIntWrapper {
659+
var wrappedValue: Int
660+
}
661+
662+
class Someclass {
663+
@BasicIntWrapper var property: Int = 0
664+
}
665+
666+
class Somesubclass : Someclass {
667+
override var property: Int {
668+
// Make sure we don't interact with the property wrapper, we just delegate
669+
// to the superclass' accessors.
670+
// CHECK-LABEL: sil hidden [ossa] @$s17property_wrappers12SomesubclassC0A0Sivs : $@convention(method) (Int, @guaranteed Somesubclass) -> ()
671+
// CHECK: bb0([[NEW:%.+]] : $Int, {{%.+}} : @guaranteed $Somesubclass):
672+
// CHECK: [[GETTER:%.+]] = function_ref @$s17property_wrappers9SomeclassC0A0Sivg : $@convention(method) (@guaranteed Someclass) -> Int
673+
// CHECK: [[OLD:%.+]] = apply [[GETTER]]({{%.+}}) : $@convention(method) (@guaranteed Someclass) -> Int
674+
// CHECK: [[SETTER:%.+]] = function_ref @$s17property_wrappers9SomeclassC0A0Sivs : $@convention(method) (Int, @guaranteed Someclass) -> ()
675+
// CHECK: apply [[SETTER]]([[NEW]], {{%.+}}) : $@convention(method) (Int, @guaranteed Someclass) -> ()
676+
// CHECK: [[DIDSET:%.+]] = function_ref @$s17property_wrappers12SomesubclassC0A0SivW : $@convention(method) (Int, @guaranteed Somesubclass) -> ()
677+
// CHECK: apply [[DIDSET]]([[OLD]], {{%.+}}) : $@convention(method) (Int, @guaranteed Somesubclass) -> ()
678+
didSet {
679+
print("Subclass")
680+
}
681+
}
682+
}
683+
656684
// rdar://problem/58986940 - composition of wrappers with autoclosure
657685
@propertyWrapper
658686
struct Once<Value> {

0 commit comments

Comments
 (0)