Skip to content

Commit a0387ee

Browse files
aschwaighoferaschwaighofer
committed
Opaque result types: Dynamic replacement fixes for computed properties
1 parent bd2bee7 commit a0387ee

File tree

9 files changed

+185
-31
lines changed

9 files changed

+185
-31
lines changed

‎include/swift/AST/Decl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4685,6 +4685,10 @@ class AbstractStorageDecl : public ValueDecl {
46854685

46864686
bool hasDidSetOrWillSetDynamicReplacement() const;
46874687

4688+
bool hasAnyNativeDynamicAccessors() const;
4689+
4690+
bool hasAnyDynamicReplacementAccessors() const;
4691+
46884692
OpaqueTypeDecl *getOpaqueResultTypeDecl() const {
46894693
return OpaqueReturn;
46904694
}

‎lib/AST/Decl.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4518,6 +4518,21 @@ bool AbstractStorageDecl::hasDidSetOrWillSetDynamicReplacement() const {
45184518
return false;
45194519
}
45204520

4521+
bool AbstractStorageDecl::hasAnyNativeDynamicAccessors() const {
4522+
for (auto accessor : getAllAccessors()) {
4523+
if (accessor->isNativeDynamic())
4524+
return true;
4525+
}
4526+
return false;
4527+
}
4528+
4529+
bool AbstractStorageDecl::hasAnyDynamicReplacementAccessors() const {
4530+
for (auto accessor : getAllAccessors()) {
4531+
if (accessor->getAttrs().hasAttribute<DynamicReplacementAttr>())
4532+
return true;
4533+
}
4534+
return false;
4535+
}
45214536
void AbstractStorageDecl::setAccessors(StorageImplInfo implInfo,
45224537
SourceLoc lbraceLoc,
45234538
ArrayRef<AccessorDecl *> accessors,

‎lib/IRGen/GenDecl.cpp

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,7 +1348,9 @@ void IRGenerator::emitDynamicReplacements() {
13481348
assert(origFunc);
13491349
assert(origFunc->getLoweredFunctionType()->hasOpaqueArchetype());
13501350
auto conv = newFunc->getConventions();
1351-
assert(conv.hasIndirectSILResults());
1351+
// Storage setters don't have indirect results.
1352+
if (!conv.hasIndirectSILResults())
1353+
continue;
13521354
for (auto res : conv.getIndirectSILResultTypes()) {
13531355
if (!res.getASTType()->hasOpaqueArchetype())
13541356
continue;
@@ -2158,13 +2160,39 @@ static void emitDynamicallyReplaceableThunk(IRGenModule &IGM,
21582160
}
21592161

21602162
void IRGenModule::emitOpaqueTypeDescriptorAccessor(OpaqueTypeDecl *opaque) {
2163+
auto *namingDecl = opaque->getNamingDecl();
2164+
auto *abstractStorage = dyn_cast<AbstractStorageDecl>(namingDecl);
2165+
2166+
bool isNativeDynamic = false;
2167+
bool isDynamicReplacement = false;
2168+
2169+
// Don't emit accessors for abstract storage that is not dynamic or a dynamic
2170+
// replacement.
2171+
if (abstractStorage) {
2172+
isNativeDynamic = abstractStorage->hasAnyNativeDynamicAccessors();
2173+
isDynamicReplacement = abstractStorage->hasAnyDynamicReplacementAccessors();
2174+
if (!isNativeDynamic && !isDynamicReplacement)
2175+
return;
2176+
}
2177+
2178+
// Don't emit accessors for functions that are not dynamic or dynamic
2179+
// replacements.
2180+
if (!abstractStorage) {
2181+
isNativeDynamic = opaque->getNamingDecl()->isNativeDynamic();
2182+
isDynamicReplacement = opaque->getNamingDecl()
2183+
->getAttrs()
2184+
.hasAttribute<DynamicReplacementAttr>();
2185+
if (!isNativeDynamic && !isDynamicReplacement)
2186+
return;
2187+
}
2188+
21612189
auto accessor =
21622190
getAddrOfOpaqueTypeDescriptorAccessFunction(opaque, ForDefinition, false);
21632191

2164-
if (opaque->getNamingDecl()->isNativeDynamic()) {
2192+
if (isNativeDynamic) {
21652193
auto thunk = accessor;
2166-
auto impl =
2167-
getAddrOfOpaqueTypeDescriptorAccessFunction(opaque, ForDefinition, true);
2194+
auto impl = getAddrOfOpaqueTypeDescriptorAccessFunction(
2195+
opaque, ForDefinition, true);
21682196
auto varEntity = LinkEntity::forOpaqueTypeDescriptorAccessorVar(opaque);
21692197
auto keyEntity = LinkEntity::forOpaqueTypeDescriptorAccessorKey(opaque);
21702198

‎lib/Sema/TypeCheckAttr.cpp

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2146,6 +2146,26 @@ matchFunctionWithOpaqueResultType(const AbstractFunctionDecl *replacement,
21462146
}
21472147

21482148
static bool matchOpaqueResultType(Type replacement, Type replaced) {
2149+
if (replacement->isEqual(replaced))
2150+
return true;
2151+
2152+
if (auto fn1 = dyn_cast<AnyFunctionType>(replacement->getCanonicalType())) {
2153+
auto fn2 = dyn_cast<AnyFunctionType>(replaced->getCanonicalType());
2154+
if (!fn2)
2155+
return false;
2156+
auto fn2Params = fn2.getParams();
2157+
auto fn1Params = fn1.getParams();
2158+
if (fn2Params.size() != fn1Params.size()) {
2159+
return false;
2160+
}
2161+
for (auto i : indices(fn2Params)) {
2162+
if (!matchOpaqueResultType(fn2Params[i].getOldType(),
2163+
fn1Params[i].getOldType()))
2164+
return false;
2165+
}
2166+
return matchOpaqueResultType(fn1.getResult(), fn2.getResult());
2167+
}
2168+
21492169
auto resultTy = replacement->getAs<OpaqueTypeArchetypeType>();
21502170
if (!resultTy)
21512171
return false;
@@ -2230,16 +2250,6 @@ static FuncDecl *findReplacedAccessor(DeclName replacedVarName,
22302250
if (origAccessor->getAccessorKind() != replacement->getAccessorKind())
22312251
continue;
22322252

2233-
if (!matchFunctionWithOpaqueResultType(replacement, origAccessor) &&
2234-
!replacement->getInterfaceType()->getCanonicalType()->matches(
2235-
origAccessor->getInterfaceType()->getCanonicalType(),
2236-
TypeMatchFlags::AllowABICompatible)) {
2237-
TC.diagnose(attr->getLocation(),
2238-
diag::dynamic_replacement_accessor_type_mismatch,
2239-
replacedVarName);
2240-
attr->setInvalid();
2241-
return nullptr;
2242-
}
22432253
if (origAccessor->isImplicit() &&
22442254
!(origStorage->getReadImpl() == ReadImplKind::Stored &&
22452255
origStorage->getWriteImpl() == WriteImplKind::Stored)) {

‎lib/TBDGen/TBDGen.cpp

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,6 @@ void TBDGenVisitor::visitAbstractFunctionDecl(AbstractFunctionDecl *AFD) {
188188
return;
189189
}
190190

191-
if (auto opaqueDecl = AFD->getOpaqueResultTypeDecl()) {
192-
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessor(opaqueDecl));
193-
}
194-
195191
addSymbol(SILDeclRef(AFD));
196192

197193
// Add the global function pointer for a dynamically replaceable function.
@@ -204,21 +200,13 @@ void TBDGenVisitor::visitAbstractFunctionDecl(AbstractFunctionDecl *AFD) {
204200
addSymbol(
205201
LinkEntity::forDynamicallyReplaceableFunctionKey(AFD, useAllocator));
206202

207-
if (auto opaqueDecl = AFD->getOpaqueResultTypeDecl()) {
208-
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessorImpl(opaqueDecl));
209-
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessorKey(opaqueDecl));
210-
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessorVar(opaqueDecl));
211-
}
212203
}
213204
if (AFD->getAttrs().hasAttribute<DynamicReplacementAttr>()) {
214205
bool useAllocator = shouldUseAllocatorMangling(AFD);
215206
addSymbol(LinkEntity::forDynamicallyReplaceableFunctionVariable(
216207
AFD, useAllocator));
217208
addSymbol(
218209
LinkEntity::forDynamicallyReplaceableFunctionImpl(AFD, useAllocator));
219-
if (auto opaqueDecl = AFD->getOpaqueResultTypeDecl()) {
220-
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessorVar(opaqueDecl));
221-
}
222210
}
223211

224212
if (AFD->getAttrs().hasAttribute<CDeclAttr>()) {
@@ -247,6 +235,17 @@ void TBDGenVisitor::visitFuncDecl(FuncDecl *FD) {
247235
// If there's an opaque return type, its descriptor is exported.
248236
if (auto opaqueResult = FD->getOpaqueResultTypeDecl()) {
249237
addSymbol(LinkEntity::forOpaqueTypeDescriptor(opaqueResult));
238+
assert(opaqueResult->getNamingDecl() == FD);
239+
if (FD->isNativeDynamic()) {
240+
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessor(opaqueResult));
241+
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessorImpl(opaqueResult));
242+
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessorKey(opaqueResult));
243+
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessorVar(opaqueResult));
244+
}
245+
if (FD->getAttrs().hasAttribute<DynamicReplacementAttr>()) {
246+
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessor(opaqueResult));
247+
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessorVar(opaqueResult));
248+
}
250249
}
251250
visitAbstractFunctionDecl(FD);
252251
}
@@ -267,7 +266,17 @@ void TBDGenVisitor::visitAbstractStorageDecl(AbstractStorageDecl *ASD) {
267266
// ...and the opaque result decl if it has one.
268267
if (auto opaqueResult = ASD->getOpaqueResultTypeDecl()) {
269268
addSymbol(LinkEntity::forOpaqueTypeDescriptor(opaqueResult));
270-
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessor(opaqueResult));
269+
assert(opaqueResult->getNamingDecl() == ASD);
270+
if (ASD->hasAnyNativeDynamicAccessors()) {
271+
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessor(opaqueResult));
272+
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessorImpl(opaqueResult));
273+
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessorKey(opaqueResult));
274+
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessorVar(opaqueResult));
275+
}
276+
if (ASD->hasAnyDynamicReplacementAccessors()) {
277+
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessor(opaqueResult));
278+
addSymbol(LinkEntity::forOpaqueTypeDescriptorAccessorVar(opaqueResult));
279+
}
271280
}
272281

273282
// Explicitly look at each accessor here: see visitAccessorDecl.

‎test/IRGen/dynamic_replaceable_opaque_return.swift

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@
99
// CHECK: @"$s1A3baryQrSiFQOMj" = constant %swift.dyn_repl_key { {{.*}}%swift.dyn_repl_link_entry* @"$s1A3baryQrSiFQOMk"{{.*}}, i32 0 }, section "__TEXT,__const"
1010
// CHECK: @"$s1A16_replacement_bar1yQrSi_tFQOMk" = global %swift.dyn_repl_link_entry zeroinitializer
1111
// CHECK: @"\01l_unnamed_dynamic_replacements" =
12-
// CHECK: private constant { i32, i32, [2 x { i32, i32, i32, i32 }] }
13-
// CHECK: { i32 0, i32 2, [2 x { i32, i32, i32, i32 }] [
14-
// CHECK: { i32, i32, i32, i32 } { {{.*}}%swift.dyn_repl_key* @"$s1A3baryQrSiFTx"{{.*}}@"$s1A16_replacement_bar1yQrSi_tF"{{.*}}%swift.dyn_repl_link_entry* @"$s1A16_replacement_bar1yQrSi_tFTX"{{.*}}, i32 0 },
15-
// CHECK: { i32, i32, i32, i32 } { {{.*}}%swift.dyn_repl_key* @"$s1A3baryQrSiFQOMj"{{.*}},{{.*}}@"$s1A16_replacement_bar1yQrSi_tFQOMg"{{.*}},{{.*}}@"$s1A16_replacement_bar1yQrSi_tFQOMk"{{.*}}, i32 0 }] }, section "__TEXT,__const", align 8
12+
// CHECK: private constant { i32, i32, [4 x { i32, i32, i32, i32 }] }
13+
// CHECK: { i32 0, i32 4, [4 x { i32, i32, i32, i32 }] [
14+
// CHECK: { i32, i32, i32, i32 } { {{.*}}%swift.dyn_repl_key* @"$s1A3baryQrSiFTx"{{.*}}@"$s1A16_replacement_bar1yQrSi_tF"{{.*}}%swift.dyn_repl_link_entry* @"$s1A16_replacement_bar1yQrSi_tFTX"{{.*}}, i32 0 }, { i32, i32, i32, i32 } { {{.*}}%swift.dyn_repl_key* @"$s1A9ContainerV4propQrvgTx"{{.*}}@"$s1A9ContainerV7_r_propQrvg"{{.*}}, i32 0 }, { i32, i32, i32, i32 } { {{.*}}%swift.dyn_repl_key* @"$s1A3baryQrSiFQOMj"{{.*}},{{.*}}@"$s1A16_replacement_bar1yQrSi_tFQOMg"{{.*}},{{.*}}@"$s1A16_replacement_bar1yQrSi_tFQOMk"{{.*}}, i32 0 }, { i32, i32, i32, i32 } { {{.*}}%swift.dyn_repl_key* @"$s1A9ContainerV4propQrvpQOMj"{{.*}},{{.*}}@"$s1A9ContainerV7_r_propQrvpQOMg"{{.*}},{{.*}}@"$s1A9ContainerV7_r_propQrvpQOMk"{{.*}}, i32 0 }] }
15+
// CHECK: , section "__TEXT,__const"
1616

1717
public protocol P {
1818
func myValue() -> Int
@@ -58,3 +58,41 @@ struct Pair : P {
5858
public func _replacement_bar(y x: Int) -> some P {
5959
return Pair()
6060
}
61+
62+
struct Container {
63+
dynamic var prop : some P {
64+
get {
65+
return 0
66+
}
67+
}
68+
}
69+
70+
// CHECK: define hidden swiftcc %swift.type_descriptor* @"$s1A9ContainerV4propQrvpQOMg"()
71+
// CHECK: entry:
72+
// CHECK: %0 = load i8*, i8** getelementptr inbounds (%swift.dyn_repl_link_entry, %swift.dyn_repl_link_entry* @"$s1A9ContainerV4propQrvpQOMk", i32 0, i32 0)
73+
// CHECK: %1 = bitcast i8* %0 to %swift.type_descriptor* ()*
74+
// CHECK: %2 = tail call swiftcc %swift.type_descriptor* %1()
75+
// CHECK: ret %swift.type_descriptor* %2
76+
77+
// CHECK: define hidden swiftcc %swift.type_descriptor* @"$s1A9ContainerV4propQrvpQOMh"()
78+
// CHECK: entry:
79+
// CHECK: ret %swift.type_descriptor* bitcast ({{.*}} @"$s1A9ContainerV4propQrvpQOMQ" to %swift.type_descriptor*)
80+
81+
extension Container {
82+
@_dynamicReplacement(for: prop)
83+
var _r_prop : some P {
84+
get {
85+
return 1
86+
}
87+
}
88+
}
89+
90+
// CHECK: define hidden swiftcc %swift.type_descriptor* @"$s1A9ContainerV7_r_propQrvpQOMg"()
91+
// CHECK: entry:
92+
// CHECK: ret %swift.type_descriptor* bitcast ({{.*}} @"$s1A9ContainerV7_r_propQrvpQOMQ" to %swift.type_descriptor*)
93+
94+
95+
// CHECK-NOT: s1A16noOpaqueAccessor{{.*}}Mg
96+
public func noOpaqueAccessor() -> some P {
97+
return 0
98+
}

‎test/Interpreter/Inputs/dynamic_replacement_opaque1.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ extension Int: P {
66
public func myValue() -> Int {
77
return self
88
}
9+
910
}
1011

1112
func bar(_ x: Int) -> some P {
@@ -16,4 +17,22 @@ struct Container {
1617
func bar(_ x: Int) -> some P {
1718
return x
1819
}
20+
21+
var computedProperty : some P {
22+
get {
23+
return 2
24+
}
25+
set {
26+
print("original \(newValue)")
27+
}
28+
}
29+
30+
subscript(_ x: Int) -> some P {
31+
get {
32+
return 2
33+
}
34+
set {
35+
print("original \(newValue)")
36+
}
37+
}
1938
}

‎test/Interpreter/Inputs/dynamic_replacement_opaque2.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,23 @@ extension Container {
1818
func _replacement_bar(y x: Int) -> some P {
1919
return Pair()
2020
}
21+
22+
@_dynamicReplacement(for: computedProperty)
23+
var _replacement_computedProperty : some P {
24+
get {
25+
return Pair()
26+
}
27+
set {
28+
print("replacement \(newValue)")
29+
}
30+
}
31+
@_dynamicReplacement(for: subscript(_:))
32+
subscript(y x: Int) -> some P {
33+
get {
34+
return Pair()
35+
}
36+
set {
37+
print("replacement \(newValue)")
38+
}
39+
}
2140
}

‎test/Interpreter/dynamic_replacement_opaque_result.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,20 @@ func test() {
3838
print(MemoryLayout.size(ofValue: Container().bar(5)))
3939
print(bar(5).myValue())
4040
print(Container().bar(5).myValue())
41+
print(MemoryLayout.size(ofValue:Container().computedProperty))
42+
print(Container().computedProperty.myValue())
43+
print(MemoryLayout.size(ofValue:Container()[0]))
44+
print(Container()[0].myValue())
4145
}
4246

4347
// CHECK: 8
4448
// CHECK: 8
4549
// CHECK: 5
4650
// CHECK: 5
51+
// CHECK: 8
52+
// CHECK: 2
53+
// CHECK: 8
54+
// CHECK: 2
4755
test()
4856

4957
var executablePath = CommandLine.arguments[0]
@@ -61,4 +69,8 @@ executablePath.removeLast(4)
6169
// CHECK: 16
6270
// CHECK: 1
6371
// CHECK: 1
72+
// CHECK: 16
73+
// CHECK: 1
74+
// CHECK: 16
75+
// CHECK: 1
6476
test()

0 commit comments

Comments
 (0)