Skip to content

Commit 8c3a077

Browse files
aschwaighoferaschwaighofer
authored
Merge pull request #22880 from aschwaighofer/dynamic_replacement_fixes_5.1
[5.1] Dynamic replacement fixes
2 parents c94688b + 8bfefc1 commit 8c3a077

20 files changed

+400
-52
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3924,6 +3924,14 @@ ERROR(dynamic_replacement_not_in_extension, none,
39243924
"dynamicReplacement(for:) of %0 is not defined in an extension or at the file level", (DeclName))
39253925
ERROR(dynamic_replacement_must_not_be_dynamic, none,
39263926
"dynamicReplacement(for:) of %0 must not be dynamic itself", (DeclName))
3927+
ERROR(dynamic_replacement_replaced_not_objc_dynamic, none,
3928+
"%0 is not marked @objc dynamic", (DeclName))
3929+
ERROR(dynamic_replacement_replacement_not_objc_dynamic, none,
3930+
"%0 is marked @objc dynamic", (DeclName))
3931+
ERROR(dynamic_replacement_replaced_constructor_is_convenience, none,
3932+
"replaced constructor %0 is marked as convenience", (DeclName))
3933+
ERROR(dynamic_replacement_replaced_constructor_is_not_convenience, none,
3934+
"replaced constructor %0 is not marked as convenience", (DeclName))
39273935

39283936
//------------------------------------------------------------------------------
39293937
// MARK: @available

‎include/swift/IRGen/Linking.h

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -894,9 +894,11 @@ class LinkEntity {
894894
}
895895

896896
static LinkEntity
897-
forDynamicallyReplaceableFunctionVariable(AbstractFunctionDecl *decl) {
897+
forDynamicallyReplaceableFunctionVariable(AbstractFunctionDecl *decl,
898+
bool isAllocator) {
898899
LinkEntity entity;
899900
entity.setForDecl(Kind::DynamicallyReplaceableFunctionVariableAST, decl);
901+
entity.SecondaryPointer = isAllocator ? decl : nullptr;
900902
return entity;
901903
}
902904

@@ -910,16 +912,20 @@ class LinkEntity {
910912
}
911913

912914
static LinkEntity
913-
forDynamicallyReplaceableFunctionKey(AbstractFunctionDecl *decl) {
915+
forDynamicallyReplaceableFunctionKey(AbstractFunctionDecl *decl,
916+
bool isAllocator) {
914917
LinkEntity entity;
915918
entity.setForDecl(Kind::DynamicallyReplaceableFunctionKeyAST, decl);
919+
entity.SecondaryPointer = isAllocator ? decl : nullptr;
916920
return entity;
917921
}
918922

919923
static LinkEntity
920-
forDynamicallyReplaceableFunctionImpl(AbstractFunctionDecl *decl) {
924+
forDynamicallyReplaceableFunctionImpl(AbstractFunctionDecl *decl,
925+
bool isAllocator) {
921926
LinkEntity entity;
922927
entity.setForDecl(Kind::DynamicallyReplaceableFunctionImpl, decl);
928+
entity.SecondaryPointer = isAllocator ? decl : nullptr;
923929
return entity;
924930
}
925931

@@ -997,6 +1003,12 @@ class LinkEntity {
9971003
assert(getKind() == Kind::SILFunction);
9981004
return LINKENTITY_GET_FIELD(Data, IsDynamicallyReplaceableImpl);
9991005
}
1006+
bool isAllocator() const {
1007+
assert(getKind() == Kind::DynamicallyReplaceableFunctionImpl ||
1008+
getKind() == Kind::DynamicallyReplaceableFunctionKeyAST ||
1009+
getKind() == Kind::DynamicallyReplaceableFunctionVariableAST);
1010+
return SecondaryPointer != nullptr;
1011+
}
10001012
bool isValueWitness() const { return getKind() == Kind::ValueWitness; }
10011013
CanType getType() const {
10021014
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/AST/Decl.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6347,8 +6347,10 @@ ConstructorDecl::getDelegatingOrChainedInitKind(DiagnosticEngine *diags,
63476347
// Prior to Swift 5, cross-module initializers were permitted to be
63486348
// non-delegating. However, if the struct isn't fixed-layout, we have to
63496349
// be delegating because, well, we don't know the layout.
6350+
// A dynamic replacement is permitted to be non-delegating.
63506351
if (NTD->isResilient() ||
6351-
containingModule->getASTContext().isSwiftVersionAtLeast(5)) {
6352+
(containingModule->getASTContext().isSwiftVersionAtLeast(5) &&
6353+
!getAttrs().getAttribute<DynamicReplacementAttr>())) {
63526354
if (containingModule != NTD->getParentModule())
63536355
Kind = BodyInitKind::Delegating;
63546356
}

‎lib/IRGen/Linking.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ std::string LinkEntity::mangleAsString() const {
317317
assert(isa<AbstractFunctionDecl>(getDecl()));
318318
std::string Result;
319319
if (auto *Constructor = dyn_cast<ConstructorDecl>(getDecl())) {
320-
Result = mangler.mangleConstructorEntity(Constructor, true,
320+
Result = mangler.mangleConstructorEntity(Constructor, isAllocator(),
321321
/*isCurried=*/false);
322322
} else {
323323
Result = mangler.mangleEntity(getDecl(), /*isCurried=*/false);
@@ -343,8 +343,9 @@ std::string LinkEntity::mangleAsString() const {
343343
assert(isa<AbstractFunctionDecl>(getDecl()));
344344
std::string Result;
345345
if (auto *Constructor = dyn_cast<ConstructorDecl>(getDecl())) {
346-
Result = mangler.mangleConstructorEntity(Constructor, true,
347-
/*isCurried=*/false);
346+
Result =
347+
mangler.mangleConstructorEntity(Constructor, isAllocator(),
348+
/*isCurried=*/false);
348349
} else {
349350
Result = mangler.mangleEntity(getDecl(), /*isCurried=*/false);
350351
}
@@ -356,8 +357,9 @@ std::string LinkEntity::mangleAsString() const {
356357
assert(isa<AbstractFunctionDecl>(getDecl()));
357358
std::string Result;
358359
if (auto *Constructor = dyn_cast<ConstructorDecl>(getDecl())) {
359-
Result = mangler.mangleConstructorEntity(Constructor, true,
360-
/*isCurried=*/false);
360+
Result =
361+
mangler.mangleConstructorEntity(Constructor, isAllocator(),
362+
/*isCurried=*/false);
361363
} else {
362364
Result = mangler.mangleEntity(getDecl(), /*isCurried=*/false);
363365
}

‎lib/SIL/SILDeclRef.cpp

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1003,12 +1003,35 @@ unsigned SILDeclRef::getParameterListCount() const {
10031003
}
10041004
}
10051005

1006+
static bool isDesignatedConstructorForClass(ValueDecl *decl) {
1007+
if (auto *ctor = dyn_cast_or_null<ConstructorDecl>(decl))
1008+
if (ctor->getDeclContext()->getSelfClassDecl())
1009+
return ctor->isDesignatedInit();
1010+
return false;
1011+
}
1012+
1013+
bool SILDeclRef::canBeDynamicReplacement() const {
1014+
if (kind == SILDeclRef::Kind::Destroyer)
1015+
return false;
1016+
if (kind == SILDeclRef::Kind::Initializer)
1017+
return isDesignatedConstructorForClass(getDecl());
1018+
if (kind == SILDeclRef::Kind::Allocator)
1019+
return !isDesignatedConstructorForClass(getDecl());
1020+
return true;
1021+
}
1022+
10061023
bool SILDeclRef::isDynamicallyReplaceable() const {
10071024
if (isStoredPropertyInitializer())
10081025
return false;
10091026

1027+
// Class allocators are not dynamic replaceable.
1028+
if (kind == SILDeclRef::Kind::Allocator &&
1029+
isDesignatedConstructorForClass(getDecl()))
1030+
return false;
1031+
10101032
if (kind == SILDeclRef::Kind::Destroyer ||
1011-
kind == SILDeclRef::Kind::Initializer ||
1033+
(kind == SILDeclRef::Kind::Initializer &&
1034+
!isDesignatedConstructorForClass(getDecl())) ||
10121035
kind == SILDeclRef::Kind::GlobalAccessor) {
10131036
return false;
10141037
}

‎lib/SIL/SILFunctionBuilder.cpp

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

88-
if (constant.isInitializerOrDestroyer())
88+
if (!constant.canBeDynamicReplacement())
8989
return;
9090

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

‎lib/Sema/TypeCheckAttr.cpp

Lines changed: 116 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2036,8 +2036,8 @@ void AttributeChecker::visitDiscardableResultAttr(DiscardableResultAttr *attr) {
20362036

20372037
/// Lookup the replaced decl in the replacments scope.
20382038
void lookupReplacedDecl(DeclName replacedDeclName,
2039-
DynamicReplacementAttr *attr,
2040-
AbstractFunctionDecl *replacement,
2039+
const DynamicReplacementAttr *attr,
2040+
const ValueDecl *replacement,
20412041
SmallVectorImpl<ValueDecl *> &results) {
20422042
auto *declCtxt = replacement->getDeclContext();
20432043

@@ -2047,9 +2047,9 @@ void lookupReplacedDecl(DeclName replacedDeclName,
20472047
declCtxt = storage->getDeclContext();
20482048
}
20492049

2050+
auto *moduleScopeCtxt = declCtxt->getModuleScopeContext();
20502051
if (isa<FileUnit>(declCtxt)) {
2051-
UnqualifiedLookup lookup(replacedDeclName,
2052-
replacement->getModuleScopeContext(), nullptr,
2052+
UnqualifiedLookup lookup(replacedDeclName, moduleScopeCtxt, nullptr,
20532053
attr->getLocation());
20542054
if (lookup.isSuccess()) {
20552055
for (auto entry : lookup.Results) {
@@ -2064,8 +2064,8 @@ void lookupReplacedDecl(DeclName replacedDeclName,
20642064
if (!typeCtx)
20652065
typeCtx = cast<ExtensionDecl>(declCtxt->getAsDecl())->getExtendedNominal();
20662066

2067-
replacement->getModuleScopeContext()->lookupQualified(
2068-
{typeCtx}, replacedDeclName, NL_QualifiedDefault, results);
2067+
moduleScopeCtxt->lookupQualified({typeCtx}, replacedDeclName,
2068+
NL_QualifiedDefault, results);
20692069
}
20702070

20712071
/// Remove any argument labels from the interface type of the given value that
@@ -2183,51 +2183,107 @@ static FuncDecl *findReplacedAccessor(DeclName replacedVarName,
21832183

21842184
static AbstractFunctionDecl *
21852185
findReplacedFunction(DeclName replacedFunctionName,
2186-
AbstractFunctionDecl *replacement,
2187-
DynamicReplacementAttr *attr, TypeChecker &TC) {
2186+
const AbstractFunctionDecl *replacement,
2187+
DynamicReplacementAttr *attr, TypeChecker *TC) {
21882188

2189+
// Note: we might pass a constant attribute when typechecker is nullptr.
2190+
// Any modification to attr must be guarded by a null check on TC.
2191+
//
21892192
SmallVector<ValueDecl *, 4> results;
21902193
lookupReplacedDecl(replacedFunctionName, attr, replacement, results);
21912194

21922195
for (auto *result : results) {
21932196
// Check for static/instance mismatch.
21942197
if (result->isStatic() != replacement->isStatic())
21952198
continue;
2196-
2197-
TC.validateDecl(result);
2199+
if (TC)
2200+
TC->validateDecl(result);
21982201
if (result->getInterfaceType()->getCanonicalType()->matches(
21992202
replacement->getInterfaceType()->getCanonicalType(),
22002203
TypeMatchFlags::AllowABICompatible)) {
22012204
if (!result->isDynamic()) {
2202-
TC.diagnose(attr->getLocation(),
2203-
diag::dynamic_replacement_function_not_dynamic,
2204-
replacedFunctionName);
2205-
attr->setInvalid();
2205+
if (TC) {
2206+
TC->diagnose(attr->getLocation(),
2207+
diag::dynamic_replacement_function_not_dynamic,
2208+
replacedFunctionName);
2209+
attr->setInvalid();
2210+
}
22062211
return nullptr;
22072212
}
22082213
return cast<AbstractFunctionDecl>(result);
22092214
}
22102215
}
2211-
if (results.empty())
2212-
TC.diagnose(attr->getLocation(),
2213-
diag::dynamic_replacement_function_not_found,
2214-
attr->getReplacedFunctionName());
2215-
else {
2216-
TC.diagnose(attr->getLocation(),
2217-
diag::dynamic_replacement_function_of_type_not_found,
2218-
attr->getReplacedFunctionName(),
2219-
replacement->getInterfaceType()->getCanonicalType());
2216+
2217+
if (!TC)
2218+
return nullptr;
2219+
2220+
if (results.empty()) {
2221+
TC->diagnose(attr->getLocation(),
2222+
diag::dynamic_replacement_function_not_found,
2223+
attr->getReplacedFunctionName());
2224+
} else {
2225+
TC->diagnose(attr->getLocation(),
2226+
diag::dynamic_replacement_function_of_type_not_found,
2227+
attr->getReplacedFunctionName(),
2228+
replacement->getInterfaceType()->getCanonicalType());
22202229

22212230
for (auto *result : results) {
2222-
TC.diagnose(SourceLoc(), diag::dynamic_replacement_found_function_of_type,
2223-
attr->getReplacedFunctionName(),
2224-
result->getInterfaceType()->getCanonicalType());
2231+
TC->diagnose(SourceLoc(),
2232+
diag::dynamic_replacement_found_function_of_type,
2233+
attr->getReplacedFunctionName(),
2234+
result->getInterfaceType()->getCanonicalType());
22252235
}
22262236
}
22272237
attr->setInvalid();
22282238
return nullptr;
22292239
}
22302240

2241+
static AbstractStorageDecl *
2242+
findReplacedStorageDecl(DeclName replacedFunctionName,
2243+
const AbstractStorageDecl *replacement,
2244+
const DynamicReplacementAttr *attr) {
2245+
2246+
SmallVector<ValueDecl *, 4> results;
2247+
lookupReplacedDecl(replacedFunctionName, attr, replacement, results);
2248+
2249+
for (auto *result : results) {
2250+
// Check for static/instance mismatch.
2251+
if (result->isStatic() != replacement->isStatic())
2252+
continue;
2253+
if (result->getInterfaceType()->getCanonicalType()->matches(
2254+
replacement->getInterfaceType()->getCanonicalType(),
2255+
TypeMatchFlags::AllowABICompatible)) {
2256+
if (!result->isDynamic()) {
2257+
return nullptr;
2258+
}
2259+
return cast<AbstractStorageDecl>(result);
2260+
}
2261+
}
2262+
return nullptr;
2263+
}
2264+
2265+
ValueDecl *TypeChecker::findReplacedDynamicFunction(const ValueDecl *vd) {
2266+
assert(isa<AbstractFunctionDecl>(vd) || isa<AbstractStorageDecl>(vd));
2267+
if (isa<AccessorDecl>(vd))
2268+
return nullptr;
2269+
2270+
auto *attr = vd->getAttrs().getAttribute<DynamicReplacementAttr>();
2271+
if (!attr)
2272+
return nullptr;
2273+
2274+
auto *afd = dyn_cast<AbstractFunctionDecl>(vd);
2275+
if (afd) {
2276+
// When we pass nullptr as the type checker argument attr is truely const.
2277+
return findReplacedFunction(attr->getReplacedFunctionName(), afd,
2278+
const_cast<DynamicReplacementAttr *>(attr),
2279+
nullptr);
2280+
}
2281+
auto *storageDecl = dyn_cast<AbstractStorageDecl>(vd);
2282+
if (!storageDecl)
2283+
return nullptr;
2284+
return findReplacedStorageDecl(attr->getReplacedFunctionName(), storageDecl, attr);
2285+
}
2286+
22312287
void TypeChecker::checkDynamicReplacementAttribute(ValueDecl *D) {
22322288
assert(isa<AbstractFunctionDecl>(D) || isa<AbstractStorageDecl>(D));
22332289

@@ -2276,7 +2332,7 @@ void TypeChecker::checkDynamicReplacementAttribute(ValueDecl *D) {
22762332
// Otherwise, find the matching function.
22772333
auto *fun = cast<AbstractFunctionDecl>(D);
22782334
if (auto *orig = findReplacedFunction(attr->getReplacedFunctionName(), fun,
2279-
attr, *this)) {
2335+
attr, this)) {
22802336
origs.push_back(orig);
22812337
replacements.push_back(fun);
22822338
} else
@@ -2288,14 +2344,46 @@ void TypeChecker::checkDynamicReplacementAttribute(ValueDecl *D) {
22882344
if (auto *attr = replacements[index]
22892345
->getAttrs()
22902346
.getAttribute<DynamicReplacementAttr>()) {
2291-
attr->setReplacedFunction(origs[index]);
2347+
auto *replacedFun = origs[index];
2348+
auto *replacement = replacements[index];
2349+
if (replacedFun->isObjC() && !replacement->isObjC()) {
2350+
diagnose(attr->getLocation(),
2351+
diag::dynamic_replacement_replacement_not_objc_dynamic,
2352+
attr->getReplacedFunctionName());
2353+
attr->setInvalid();
2354+
return;
2355+
}
2356+
if (!replacedFun->isObjC() && replacement->isObjC()) {
2357+
diagnose(attr->getLocation(),
2358+
diag::dynamic_replacement_replaced_not_objc_dynamic,
2359+
attr->getReplacedFunctionName());
2360+
attr->setInvalid();
2361+
return;
2362+
}
2363+
attr->setReplacedFunction(replacedFun);
22922364
continue;
22932365
}
22942366
auto *newAttr = DynamicReplacementAttr::create(
22952367
D->getASTContext(), attr->getReplacedFunctionName(), origs[index]);
22962368
DeclAttributes &attrs = replacements[index]->getAttrs();
22972369
attrs.add(newAttr);
22982370
}
2371+
if (auto *CD = dyn_cast<ConstructorDecl>(D)) {
2372+
auto *attr = CD->getAttrs().getAttribute<DynamicReplacementAttr>();
2373+
auto replacedIsConvenienceInit =
2374+
cast<ConstructorDecl>(attr->getReplacedFunction())->isConvenienceInit();
2375+
if (replacedIsConvenienceInit &&!CD->isConvenienceInit()) {
2376+
diagnose(attr->getLocation(),
2377+
diag::dynamic_replacement_replaced_constructor_is_convenience,
2378+
attr->getReplacedFunctionName());
2379+
} else if (!replacedIsConvenienceInit && CD->isConvenienceInit()) {
2380+
diagnose(
2381+
attr->getLocation(),
2382+
diag::dynamic_replacement_replaced_constructor_is_not_convenience,
2383+
attr->getReplacedFunctionName());
2384+
}
2385+
}
2386+
22992387

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

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4047,7 +4047,8 @@ void TypeChecker::validateDecl(ValueDecl *D) {
40474047
// (or the same file) to add vtable entries, we can re-evaluate this
40484048
// restriction.
40494049
if (extType->getClassOrBoundGenericClass() &&
4050-
isa<ExtensionDecl>(CD->getDeclContext())) {
4050+
isa<ExtensionDecl>(CD->getDeclContext()) &&
4051+
!(CD->getAttrs().hasAttribute<DynamicReplacementAttr>())) {
40514052
diagnose(CD->getLoc(), diag::designated_init_in_extension, extType)
40524053
.fixItInsert(CD->getLoc(), "convenience ");
40534054
CD->setInitKind(CtorInitializerKind::Convenience);

0 commit comments

Comments
 (0)