Skip to content

Commit 8d9b9f3

Browse files
aschwaighoferaschwaighofer
committed
SILGen: Fix the logic of dynamic replacements for class constructors
To correctly call designated super class initializers the designated intializer (and not the allocator) is dynamically replaceable. Convenience allocators are dynamically replaceable as before.
1 parent f820c6e commit 8d9b9f3

File tree

12 files changed

+144
-22
lines changed

12 files changed

+144
-22
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3925,6 +3925,10 @@ ERROR(dynamic_replacement_replaced_not_objc_dynamic, none,
39253925
"%0 is not marked @objc dynamic", (DeclName))
39263926
ERROR(dynamic_replacement_replacement_not_objc_dynamic, none,
39273927
"%0 is marked @objc dynamic", (DeclName))
3928+
ERROR(dynamic_replacement_replaced_constructor_is_convenience, none,
3929+
"replaced constructor %0 is marked as convenience", (DeclName))
3930+
ERROR(dynamic_replacement_replaced_constructor_is_not_convenience, none,
3931+
"replaced constructor %0 is not marked as convenience", (DeclName))
39283932

39293933
//------------------------------------------------------------------------------
39303934
// MARK: @available

‎include/swift/IRGen/Linking.h

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -896,9 +896,11 @@ class LinkEntity {
896896
}
897897

898898
static LinkEntity
899-
forDynamicallyReplaceableFunctionVariable(AbstractFunctionDecl *decl) {
899+
forDynamicallyReplaceableFunctionVariable(AbstractFunctionDecl *decl,
900+
bool isAllocator) {
900901
LinkEntity entity;
901902
entity.setForDecl(Kind::DynamicallyReplaceableFunctionVariableAST, decl);
903+
entity.SecondaryPointer = isAllocator ? decl : nullptr;
902904
return entity;
903905
}
904906

@@ -912,16 +914,20 @@ class LinkEntity {
912914
}
913915

914916
static LinkEntity
915-
forDynamicallyReplaceableFunctionKey(AbstractFunctionDecl *decl) {
917+
forDynamicallyReplaceableFunctionKey(AbstractFunctionDecl *decl,
918+
bool isAllocator) {
916919
LinkEntity entity;
917920
entity.setForDecl(Kind::DynamicallyReplaceableFunctionKeyAST, decl);
921+
entity.SecondaryPointer = isAllocator ? decl : nullptr;
918922
return entity;
919923
}
920924

921925
static LinkEntity
922-
forDynamicallyReplaceableFunctionImpl(AbstractFunctionDecl *decl) {
926+
forDynamicallyReplaceableFunctionImpl(AbstractFunctionDecl *decl,
927+
bool isAllocator) {
923928
LinkEntity entity;
924929
entity.setForDecl(Kind::DynamicallyReplaceableFunctionImpl, decl);
930+
entity.SecondaryPointer = isAllocator ? decl : nullptr;
925931
return entity;
926932
}
927933

@@ -999,6 +1005,12 @@ class LinkEntity {
9991005
assert(getKind() == Kind::SILFunction);
10001006
return LINKENTITY_GET_FIELD(Data, IsDynamicallyReplaceableImpl);
10011007
}
1008+
bool isAllocator() const {
1009+
assert(getKind() == Kind::DynamicallyReplaceableFunctionImpl ||
1010+
getKind() == Kind::DynamicallyReplaceableFunctionKeyAST ||
1011+
getKind() == Kind::DynamicallyReplaceableFunctionVariableAST);
1012+
return SecondaryPointer != nullptr;
1013+
}
10021014
bool isValueWitness() const { return getKind() == Kind::ValueWitness; }
10031015
CanType getType() const {
10041016
assert(isTypeKind(getKind()));

‎include/swift/SIL/SILDeclRef.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,8 @@ struct SILDeclRef {
392392

393393
bool isDynamicallyReplaceable() const;
394394

395+
bool canBeDynamicReplacement() const;
396+
395397
private:
396398
friend struct llvm::DenseMapInfo<swift::SILDeclRef>;
397399
/// Produces a SILDeclRef from an opaque value.

‎lib/IRGen/Linking.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ std::string LinkEntity::mangleAsString() const {
318318
assert(isa<AbstractFunctionDecl>(getDecl()));
319319
std::string Result;
320320
if (auto *Constructor = dyn_cast<ConstructorDecl>(getDecl())) {
321-
Result = mangler.mangleConstructorEntity(Constructor, true,
321+
Result = mangler.mangleConstructorEntity(Constructor, isAllocator(),
322322
/*isCurried=*/false);
323323
} else {
324324
Result = mangler.mangleEntity(getDecl(), /*isCurried=*/false);
@@ -344,8 +344,9 @@ std::string LinkEntity::mangleAsString() const {
344344
assert(isa<AbstractFunctionDecl>(getDecl()));
345345
std::string Result;
346346
if (auto *Constructor = dyn_cast<ConstructorDecl>(getDecl())) {
347-
Result = mangler.mangleConstructorEntity(Constructor, true,
348-
/*isCurried=*/false);
347+
Result =
348+
mangler.mangleConstructorEntity(Constructor, isAllocator(),
349+
/*isCurried=*/false);
349350
} else {
350351
Result = mangler.mangleEntity(getDecl(), /*isCurried=*/false);
351352
}
@@ -357,8 +358,9 @@ std::string LinkEntity::mangleAsString() const {
357358
assert(isa<AbstractFunctionDecl>(getDecl()));
358359
std::string Result;
359360
if (auto *Constructor = dyn_cast<ConstructorDecl>(getDecl())) {
360-
Result = mangler.mangleConstructorEntity(Constructor, true,
361-
/*isCurried=*/false);
361+
Result =
362+
mangler.mangleConstructorEntity(Constructor, isAllocator(),
363+
/*isCurried=*/false);
362364
} else {
363365
Result = mangler.mangleEntity(getDecl(), /*isCurried=*/false);
364366
}

‎lib/SIL/SILDeclRef.cpp

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1019,12 +1019,35 @@ unsigned SILDeclRef::getParameterListCount() const {
10191019
}
10201020
}
10211021

1022+
static bool isDesignatedConstructorForClass(ValueDecl *decl) {
1023+
if (auto *ctor = dyn_cast_or_null<ConstructorDecl>(decl))
1024+
if (ctor->getDeclContext()->getSelfClassDecl())
1025+
return ctor->isDesignatedInit();
1026+
return false;
1027+
}
1028+
1029+
bool SILDeclRef::canBeDynamicReplacement() const {
1030+
if (kind == SILDeclRef::Kind::Destroyer)
1031+
return false;
1032+
if (kind == SILDeclRef::Kind::Initializer)
1033+
return isDesignatedConstructorForClass(getDecl());
1034+
if (kind == SILDeclRef::Kind::Allocator)
1035+
return !isDesignatedConstructorForClass(getDecl());
1036+
return true;
1037+
}
1038+
10221039
bool SILDeclRef::isDynamicallyReplaceable() const {
10231040
if (isStoredPropertyInitializer())
10241041
return false;
10251042

1043+
// Class allocators are not dynamic replaceable.
1044+
if (kind == SILDeclRef::Kind::Allocator &&
1045+
isDesignatedConstructorForClass(getDecl()))
1046+
return false;
1047+
10261048
if (kind == SILDeclRef::Kind::Destroyer ||
1027-
kind == SILDeclRef::Kind::Initializer ||
1049+
(kind == SILDeclRef::Kind::Initializer &&
1050+
!isDesignatedConstructorForClass(getDecl())) ||
10281051
kind == SILDeclRef::Kind::GlobalAccessor) {
10291052
return false;
10301053
}

‎lib/SIL/SILFunctionBuilder.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ void SILFunctionBuilder::addFunctionAttributes(SILFunction *F,
8686
return;
8787
}
8888

89-
if (constant.isInitializerOrDestroyer())
89+
if (!constant.canBeDynamicReplacement())
9090
return;
9191

9292
SILDeclRef declRef(replacedDecl, constant.kind, false);

‎lib/Sema/TypeCheckAttr.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2253,6 +2253,22 @@ void TypeChecker::checkDynamicReplacementAttribute(ValueDecl *D) {
22532253
DeclAttributes &attrs = replacements[index]->getAttrs();
22542254
attrs.add(newAttr);
22552255
}
2256+
if (auto *CD = dyn_cast<ConstructorDecl>(D)) {
2257+
auto *attr = CD->getAttrs().getAttribute<DynamicReplacementAttr>();
2258+
auto replacedIsConvenienceInit =
2259+
cast<ConstructorDecl>(attr->getReplacedFunction())->isConvenienceInit();
2260+
if (replacedIsConvenienceInit &&!CD->isConvenienceInit()) {
2261+
diagnose(attr->getLocation(),
2262+
diag::dynamic_replacement_replaced_constructor_is_convenience,
2263+
attr->getReplacedFunctionName());
2264+
} else if (!replacedIsConvenienceInit && CD->isConvenienceInit()) {
2265+
diagnose(
2266+
attr->getLocation(),
2267+
diag::dynamic_replacement_replaced_constructor_is_not_convenience,
2268+
attr->getReplacedFunctionName());
2269+
}
2270+
}
2271+
22562272

22572273
// Remove the attribute on the abstract storage (we have moved it to the
22582274
// accessor decl).

‎lib/TBDGen/TBDGen.cpp

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,21 @@ void TBDGenVisitor::addConformances(DeclContext *DC) {
166166
}
167167
}
168168

169+
/// Determine whether dynamic replacement should be emitted for the allocator or
170+
/// the initializer given a decl.
171+
/// The rule is that structs and convenience init of classes emit a
172+
/// dynamic replacement for the allocator.
173+
/// Designated init of classes emit a dynamic replacement for the intializer.
174+
/// This is because the super class init call is emitted to the initializer and
175+
/// needs to be dynamic.
176+
static bool shouldUseAllocatorMangling(const AbstractFunctionDecl *afd) {
177+
auto constructor = dyn_cast<ConstructorDecl>(afd);
178+
if (!constructor)
179+
return false;
180+
return constructor->getParent()->getSelfClassDecl() == nullptr ||
181+
constructor->isConvenienceInit();
182+
}
183+
169184
void TBDGenVisitor::visitAbstractFunctionDecl(AbstractFunctionDecl *AFD) {
170185
// A @_silgen_name("...") function without a body only exists
171186
// to forward-declare a symbol from another library.
@@ -177,13 +192,20 @@ void TBDGenVisitor::visitAbstractFunctionDecl(AbstractFunctionDecl *AFD) {
177192

178193
// Add the global function pointer for a dynamically replaceable function.
179194
if (AFD->isNativeDynamic()) {
180-
addSymbol(LinkEntity::forDynamicallyReplaceableFunctionVariable(AFD));
181-
addSymbol(LinkEntity::forDynamicallyReplaceableFunctionImpl(AFD));
182-
addSymbol(LinkEntity::forDynamicallyReplaceableFunctionKey(AFD));
195+
bool useAllocator = shouldUseAllocatorMangling(AFD);
196+
addSymbol(LinkEntity::forDynamicallyReplaceableFunctionVariable(
197+
AFD, useAllocator));
198+
addSymbol(
199+
LinkEntity::forDynamicallyReplaceableFunctionImpl(AFD, useAllocator));
200+
addSymbol(
201+
LinkEntity::forDynamicallyReplaceableFunctionKey(AFD, useAllocator));
183202
}
184203
if (AFD->getAttrs().hasAttribute<DynamicReplacementAttr>()) {
185-
addSymbol(LinkEntity::forDynamicallyReplaceableFunctionVariable(AFD));
186-
addSymbol(LinkEntity::forDynamicallyReplaceableFunctionImpl(AFD));
204+
bool useAllocator = shouldUseAllocatorMangling(AFD);
205+
addSymbol(LinkEntity::forDynamicallyReplaceableFunctionVariable(
206+
AFD, useAllocator));
207+
addSymbol(
208+
LinkEntity::forDynamicallyReplaceableFunctionImpl(AFD, useAllocator));
187209
}
188210

189211
if (AFD->getAttrs().hasAttribute<CDeclAttr>()) {

‎test/Interpreter/Inputs/dynamic_replacement_module.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,7 @@ public func replacement_for_public_global_generic_func<T>(_ t: T.Type) -> String
155155

156156
extension PublicClass {
157157
@_dynamicReplacement(for: init(x:))
158-
convenience public init(y: Int) {
159-
self.init(x: y)
158+
public init(y: Int) {
160159
str = "replacement of public_class_init"
161160
}
162161

‎test/SILGen/dynamically_replaceable.swift

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ struct Strukt {
3838
}
3939
}
4040
}
41-
// CHECK-LABEL: sil hidden [dynamically_replacable] [ossa] @$s23dynamically_replaceable5KlassC1xACSi_tcfC : $@convention(method) (Int, @thick Klass.Type) -> @owned Klass
41+
// CHECK-LABEL: sil hidden [dynamically_replacable] [ossa] @$s23dynamically_replaceable5KlassC1xACSi_tcfc : $@convention(method) (Int, @owned Klass) -> @owned Klass
4242
// CHECK-LABEL: sil hidden [dynamically_replacable] [ossa] @$s23dynamically_replaceable5KlassC08dynamic_B0yyF : $@convention(method) (@guaranteed Klass) -> () {
4343
// CHECK-LABEL: sil hidden [dynamically_replacable] [ossa] @$s23dynamically_replaceable5KlassC08dynamic_B4_varSivg
4444
// CHECK-LABEL: sil hidden [dynamically_replacable] [ossa] @$s23dynamically_replaceable5KlassC08dynamic_B4_varSivs
@@ -47,6 +47,13 @@ struct Strukt {
4747
class Klass {
4848
dynamic init(x: Int) {
4949
}
50+
51+
dynamic convenience init(c: Int) {
52+
self.init(x: c)
53+
}
54+
55+
dynamic convenience init(a: Int, b: Int) {
56+
}
5057
dynamic func dynamic_replaceable() {
5158
}
5259
dynamic func dynamic_replaceable2() {
@@ -67,6 +74,15 @@ class Klass {
6774
}
6875
}
6976

77+
class SubKlass : Klass {
78+
// CHECK-LABEL: sil hidden [dynamically_replacable] [ossa] @$s23dynamically_replaceable8SubKlassC1xACSi_tcfc
79+
// CHECK: // dynamic_function_ref Klass.init(x:)
80+
// CHECK: [[FUN:%.*]] = dynamic_function_ref @$s23dynamically_replaceable5KlassC1xACSi_tcfc
81+
// CHECK: apply [[FUN]]
82+
dynamic override init(x: Int) {
83+
super.init(x: x)
84+
}
85+
}
7086
// CHECK-LABEL: sil hidden [dynamically_replacable] [ossa] @$s23dynamically_replaceable6globalSivg : $@convention(thin) () -> Int {
7187
dynamic var global : Int {
7288
return 1
@@ -93,12 +109,25 @@ extension Klass {
93109
dynamic_replaceable2()
94110
}
95111

96-
// CHECK-LABEL: sil hidden [dynamic_replacement_for "$s23dynamically_replaceable5KlassC1xACSi_tcfC"] [ossa] @$s23dynamically_replaceable5KlassC1yACSi_tcfC : $@convention(method) (Int, @thick Klass.Type) -> @owned Klass {
97-
// CHECK: [[FUN:%.*]] = prev_dynamic_function_ref @$s23dynamically_replaceable5KlassC1yACSi_tcfC
112+
// CHECK-LABEL: sil hidden [dynamic_replacement_for "$s23dynamically_replaceable5KlassC1cACSi_tcfC"] [ossa] @$s23dynamically_replaceable5KlassC2crACSi_tcfC : $@convention(method) (Int, @thick Klass.Type) -> @owned Klass {
113+
// CHECK: [[FUN:%.*]] = prev_dynamic_function_ref @$s23dynamically_replaceable5KlassC2crACSi_tcfC
98114
// CHECK: apply [[FUN]]({{.*}}, %1)
115+
@_dynamicReplacement(for: init(c:))
116+
convenience init(cr: Int) {
117+
self.init(c: cr + 1)
118+
}
119+
120+
// CHECK-LABEL: sil hidden [dynamic_replacement_for "$s23dynamically_replaceable5KlassC1a1bACSi_SitcfC"] [ossa] @$s23dynamically_replaceable5KlassC2ar2brACSi_SitcfC
121+
// CHECK: // dynamic_function_ref Klass.__allocating_init(c:)
122+
// CHECK: [[FUN:%.*]] = dynamic_function_ref @$s23dynamically_replaceable5KlassC1cACSi_tcfC
123+
// CHECK: apply [[FUN]]({{.*}}, %2)
124+
@_dynamicReplacement(for: init(a: b:))
125+
convenience init(ar: Int, br: Int) {
126+
self.init(c: ar + br)
127+
}
128+
99129
@_dynamicReplacement(for: init(x:))
100-
convenience init(y: Int) {
101-
self.init(x: y + 1)
130+
init(xr: Int) {
102131
}
103132

104133
// CHECK-LABEL: sil hidden [dynamic_replacement_for "$s23dynamically_replaceable5KlassC08dynamic_B4_varSivg"] [ossa] @$s23dynamically_replaceable5KlassC1rSivg : $@convention(method) (@guaranteed Klass) -> Int {

0 commit comments

Comments
 (0)