Skip to content

Commit e8e1f4f

Browse files
slavapestovslavapestov
authored
Merge pull request swiftlang#20593 from slavapestov/keypath-resilience-fixes
Fix key paths for resilience and @inlinable
2 parents e4fb140 + 22c6d4e commit e8e1f4f

40 files changed

+638
-252
lines changed

‎include/swift/AST/ASTMangler.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,19 @@ class ASTMangler : public Mangler {
137137
std::string mangleKeyPathGetterThunkHelper(const AbstractStorageDecl *property,
138138
GenericSignature *signature,
139139
CanType baseType,
140-
ArrayRef<CanType> subs);
140+
SubstitutionMap subs,
141+
ResilienceExpansion expansion);
141142
std::string mangleKeyPathSetterThunkHelper(const AbstractStorageDecl *property,
142143
GenericSignature *signature,
143144
CanType baseType,
144-
ArrayRef<CanType> subs);
145+
SubstitutionMap subs,
146+
ResilienceExpansion expansion);
145147
std::string mangleKeyPathEqualsHelper(ArrayRef<CanType> indices,
146-
GenericSignature *signature);
148+
GenericSignature *signature,
149+
ResilienceExpansion expansion);
147150
std::string mangleKeyPathHashHelper(ArrayRef<CanType> indices,
148-
GenericSignature *signature);
151+
GenericSignature *signature,
152+
ResilienceExpansion expansion);
149153

150154
std::string mangleTypeForDebugger(Type decl, const DeclContext *DC);
151155

‎include/swift/AST/Decl.h

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ class alignas(1 << DeclAlignInBits) Decl {
324324
IsUserAccessible : 1
325325
);
326326

327-
SWIFT_INLINE_BITFIELD(AbstractStorageDecl, ValueDecl, 1+1+1+1+2,
327+
SWIFT_INLINE_BITFIELD(AbstractStorageDecl, ValueDecl, 1+1+1+1+2+1+1,
328328
/// Whether the getter is mutating.
329329
IsGetterMutating : 1,
330330

@@ -338,7 +338,12 @@ class alignas(1 << DeclAlignInBits) Decl {
338338
SupportsMutation : 1,
339339

340340
/// Whether an opaque read of this storage produces an owned value.
341-
OpaqueReadOwnership : 2
341+
OpaqueReadOwnership : 2,
342+
343+
/// Whether a keypath component can directly reference this storage,
344+
/// or if it must use the overridden declaration instead.
345+
HasComputedValidKeyPathComponent : 1,
346+
ValidKeyPathComponent : 1
342347
);
343348

344349
SWIFT_INLINE_BITFIELD(VarDecl, AbstractStorageDecl, 1+4+1+1+1+1,
@@ -4263,6 +4268,8 @@ class AbstractStorageDecl : public ValueDecl {
42634268
Bits.AbstractStorageDecl.SupportsMutation = supportsMutation;
42644269
}
42654270

4271+
void computeIsValidKeyPathComponent();
4272+
42664273
public:
42674274

42684275
/// \brief Should this declaration be treated as if annotated with transparent
@@ -4503,7 +4510,8 @@ class AbstractStorageDecl : public ValueDecl {
45034510
/// Determine how this storage declaration should actually be accessed.
45044511
AccessStrategy getAccessStrategy(AccessSemantics semantics,
45054512
AccessKind accessKind,
4506-
DeclContext *accessFromDC = nullptr) const;
4513+
ModuleDecl *module,
4514+
ResilienceExpansion expansion) const;
45074515

45084516
/// \brief Should this declaration behave as if it must be accessed
45094517
/// resiliently, even when we're building a non-resilient module?
@@ -4541,7 +4549,20 @@ class AbstractStorageDecl : public ValueDecl {
45414549
BehaviorRecord *getMutableBehavior() {
45424550
return BehaviorInfo.getPointer();
45434551
}
4544-
4552+
4553+
void setIsValidKeyPathComponent(bool value) {
4554+
Bits.AbstractStorageDecl.HasComputedValidKeyPathComponent = true;
4555+
Bits.AbstractStorageDecl.ValidKeyPathComponent = value;
4556+
}
4557+
4558+
/// True if the storage can be referenced by a keypath directly.
4559+
/// Otherwise, its override must be referenced.
4560+
bool isValidKeyPathComponent() const {
4561+
if (!Bits.AbstractStorageDecl.HasComputedValidKeyPathComponent)
4562+
const_cast<AbstractStorageDecl *>(this)->computeIsValidKeyPathComponent();
4563+
return Bits.AbstractStorageDecl.ValidKeyPathComponent;
4564+
}
4565+
45454566
/// True if the storage exports a property descriptor for key paths in
45464567
/// other modules.
45474568
bool exportsPropertyDescriptor() const;

‎include/swift/Basic/LangOptions.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,6 @@ namespace swift {
241241
/// This is for bootstrapping. It can't be in SILOptions because the
242242
/// TypeChecker uses it to set resolve the ParameterConvention.
243243
bool EnableSILOpaqueValues = false;
244-
245-
/// Enables key path resilience.
246-
bool EnableKeyPathResilience = true;
247244

248245
/// If set to true, the diagnosis engine can assume the emitted diagnostics
249246
/// will be used in editor. This usually leads to more aggressive fixit.

‎include/swift/Demangling/DemangleNodes.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ NODE(SILBoxMutableField)
183183
NODE(SILBoxImmutableField)
184184
CONTEXT_NODE(Setter)
185185
NODE(SpecializationPassID)
186-
NODE(SpecializationIsFragile)
186+
NODE(IsSerialized)
187187
CONTEXT_NODE(Static)
188188
CONTEXT_NODE(Structure)
189189
CONTEXT_NODE(Subscript)

‎include/swift/Option/FrontendOptions.td

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -547,9 +547,6 @@ def enable_verify_exclusivity : Flag<["-"], "enable-verify-exclusivity">,
547547
def disable_verify_exclusivity : Flag<["-"], "disable-verify-exclusivity">,
548548
HelpText<"Diable verification of access markers used to enforce exclusivity.">;
549549

550-
def enable_key_path_resilience : Flag<["-"], "enable-key-path-resilience">,
551-
HelpText<"Enable key path resilience.">;
552-
553550
def type_info_dump_filter_EQ : Joined<["-"], "type-info-dump-filter=">,
554551
Flags<[FrontendOption]>,
555552
HelpText<"One of 'all', 'resilient' or 'fragile'">;

‎include/swift/SIL/TypeLowering.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -971,8 +971,6 @@ class TypeConverter {
971971
CanSILBoxType getBoxTypeForEnumElement(SILType enumType,
972972
EnumElementDecl *elt);
973973

974-
bool canStorageUseStoredKeyPathComponent(AbstractStorageDecl *decl);
975-
976974
private:
977975
CanType getLoweredRValueType(AbstractionPattern origType, CanType substType);
978976

‎lib/AST/ASTMangler.cpp

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,8 @@ std::string ASTMangler::mangleKeyPathGetterThunkHelper(
267267
const AbstractStorageDecl *property,
268268
GenericSignature *signature,
269269
CanType baseType,
270-
ArrayRef<CanType> subs) {
270+
SubstitutionMap subs,
271+
ResilienceExpansion expansion) {
271272
beginMangling();
272273
appendEntity(property);
273274
if (signature)
@@ -276,19 +277,22 @@ std::string ASTMangler::mangleKeyPathGetterThunkHelper(
276277
if (isa<SubscriptDecl>(property)) {
277278
// Subscripts can be generic, and different key paths could capture the same
278279
// subscript at different generic arguments.
279-
for (auto &sub : subs) {
280-
appendType(sub);
280+
for (auto sub : subs.getReplacementTypes()) {
281+
appendType(sub->mapTypeOutOfContext()->getCanonicalType());
281282
}
282283
}
283284
appendOperator("TK");
285+
if (expansion == ResilienceExpansion::Minimal)
286+
appendOperator("q");
284287
return finalize();
285288
}
286289

287290
std::string ASTMangler::mangleKeyPathSetterThunkHelper(
288291
const AbstractStorageDecl *property,
289292
GenericSignature *signature,
290293
CanType baseType,
291-
ArrayRef<CanType> subs) {
294+
SubstitutionMap subs,
295+
ResilienceExpansion expansion) {
292296
beginMangling();
293297
appendEntity(property);
294298
if (signature)
@@ -297,33 +301,41 @@ std::string ASTMangler::mangleKeyPathSetterThunkHelper(
297301
if (isa<SubscriptDecl>(property)) {
298302
// Subscripts can be generic, and different key paths could capture the same
299303
// subscript at different generic arguments.
300-
for (auto &sub : subs) {
301-
appendType(sub);
304+
for (auto sub : subs.getReplacementTypes()) {
305+
appendType(sub->mapTypeOutOfContext()->getCanonicalType());
302306
}
303307
}
304308
appendOperator("Tk");
309+
if (expansion == ResilienceExpansion::Minimal)
310+
appendOperator("q");
305311
return finalize();
306312
}
307313

308314
std::string ASTMangler::mangleKeyPathEqualsHelper(ArrayRef<CanType> indices,
309-
GenericSignature *signature) {
315+
GenericSignature *signature,
316+
ResilienceExpansion expansion) {
310317
beginMangling();
311318
for (auto &index : indices)
312319
appendType(index);
313320
if (signature)
314321
appendGenericSignature(signature);
315322
appendOperator("TH");
323+
if (expansion == ResilienceExpansion::Minimal)
324+
appendOperator("q");
316325
return finalize();
317326
}
318327

319328
std::string ASTMangler::mangleKeyPathHashHelper(ArrayRef<CanType> indices,
320-
GenericSignature *signature) {
329+
GenericSignature *signature,
330+
ResilienceExpansion expansion) {
321331
beginMangling();
322332
for (auto &index : indices)
323333
appendType(index);
324334
if (signature)
325335
appendGenericSignature(signature);
326336
appendOperator("Th");
337+
if (expansion == ResilienceExpansion::Minimal)
338+
appendOperator("q");
327339
return finalize();
328340
}
329341

‎lib/AST/Decl.cpp

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1608,7 +1608,8 @@ getOpaqueAccessStrategy(const AbstractStorageDecl *storage,
16081608
AccessStrategy
16091609
AbstractStorageDecl::getAccessStrategy(AccessSemantics semantics,
16101610
AccessKind accessKind,
1611-
DeclContext *accessFromDC) const {
1611+
ModuleDecl *module,
1612+
ResilienceExpansion expansion) const {
16121613
switch (semantics) {
16131614
case AccessSemantics::DirectToStorage:
16141615
assert(hasStorage());
@@ -1626,9 +1627,9 @@ AbstractStorageDecl::getAccessStrategy(AccessSemantics semantics,
16261627
if (isNativeDynamic())
16271628
return getOpaqueAccessStrategy(this, accessKind, /*dispatch*/ false);
16281629

1629-
// If the storage is resilient to the given use DC (perhaps because
1630-
// it's @_transparent and we have to be careful about it being inlined
1631-
// across module lines), we cannot use direct access.
1630+
// If the storage is resilient from the given module and resilience
1631+
// expansion, we cannot use direct access.
1632+
//
16321633
// If we end up here with a stored property of a type that's resilient
16331634
// from some resilience domain, we cannot do direct access.
16341635
//
@@ -1640,9 +1641,8 @@ AbstractStorageDecl::getAccessStrategy(AccessSemantics semantics,
16401641
// understanding that the access semantics are with respect to the
16411642
// resilience domain of the accessor's caller.
16421643
bool resilient;
1643-
if (accessFromDC)
1644-
resilient = isResilient(accessFromDC->getParentModule(),
1645-
accessFromDC->getResilienceExpansion());
1644+
if (module)
1645+
resilient = isResilient(module, expansion);
16461646
else
16471647
resilient = isResilient();
16481648

@@ -1673,7 +1673,7 @@ AbstractStorageDecl::getAccessStrategy(AccessSemantics semantics,
16731673
case AccessKind::Read:
16741674
case AccessKind::ReadWrite:
16751675
return getAccessStrategy(AccessSemantics::Ordinary,
1676-
accessKind, accessFromDC);
1676+
accessKind, module, expansion);
16771677
}
16781678
}
16791679
llvm_unreachable("bad access semantics");
@@ -1824,6 +1824,34 @@ bool AbstractStorageDecl::isResilient(ModuleDecl *M,
18241824
llvm_unreachable("bad resilience expansion");
18251825
}
18261826

1827+
static bool isValidKeyPathComponent(AbstractStorageDecl *decl) {
1828+
// If this property or subscript is not an override, we can reference it
1829+
// from a keypath component.
1830+
auto base = decl->getOverriddenDecl();
1831+
if (!base)
1832+
return true;
1833+
1834+
// Otherwise, we can only reference it if the type is not ABI-compatible
1835+
// with the type of the base.
1836+
//
1837+
// If the type is ABI compatible with the type of the base, we have to
1838+
// reference the base instead.
1839+
auto baseInterfaceTy = base->getInterfaceType();
1840+
auto derivedInterfaceTy = decl->getInterfaceType();
1841+
1842+
auto selfInterfaceTy = decl->getDeclContext()->getDeclaredInterfaceType();
1843+
1844+
auto overrideInterfaceTy = selfInterfaceTy->adjustSuperclassMemberDeclType(
1845+
base, decl, baseInterfaceTy);
1846+
1847+
return !derivedInterfaceTy->matches(overrideInterfaceTy,
1848+
TypeMatchFlags::AllowABICompatible);
1849+
}
1850+
1851+
void AbstractStorageDecl::computeIsValidKeyPathComponent() {
1852+
setIsValidKeyPathComponent(::isValidKeyPathComponent(this));
1853+
}
1854+
18271855
bool ValueDecl::isInstanceMember() const {
18281856
DeclContext *DC = getDeclContext();
18291857
if (!DC->isTypeContext())

‎lib/Demangling/Demangler.cpp

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1997,6 +1997,9 @@ NodePointer Demangler::demangleThunkOrSpecialization() {
19971997
case 'k': {
19981998
auto nodeKind = c == 'K' ? Node::Kind::KeyPathGetterThunkHelper
19991999
: Node::Kind::KeyPathSetterThunkHelper;
2000+
2001+
bool isSerialized = nextIf('q');
2002+
20002003
std::vector<NodePointer> types;
20012004
auto node = popNode();
20022005
if (!node || node->getKind() != Node::Kind::Type)
@@ -2022,6 +2025,10 @@ NodePointer Demangler::demangleThunkOrSpecialization() {
20222025
for (auto i = types.rbegin(), e = types.rend(); i != e; ++i) {
20232026
result->addChild(*i, *this);
20242027
}
2028+
2029+
if (isSerialized)
2030+
result->addChild(createNode(Node::Kind::IsSerialized), *this);
2031+
20252032
return result;
20262033
}
20272034
case 'l': {
@@ -2060,6 +2067,9 @@ NodePointer Demangler::demangleThunkOrSpecialization() {
20602067
case 'h': {
20612068
auto nodeKind = c == 'H' ? Node::Kind::KeyPathEqualsThunkHelper
20622069
: Node::Kind::KeyPathHashThunkHelper;
2070+
2071+
bool isSerialized = nextIf('q');
2072+
20632073
NodePointer genericSig = nullptr;
20642074
std::vector<NodePointer> types;
20652075

@@ -2089,6 +2099,10 @@ NodePointer Demangler::demangleThunkOrSpecialization() {
20892099
}
20902100
if (genericSig)
20912101
result->addChild(genericSig, *this);
2102+
2103+
if (isSerialized)
2104+
result->addChild(createNode(Node::Kind::IsSerialized), *this);
2105+
20922106
return result;
20932107
}
20942108
case 'v': {
@@ -2344,15 +2358,15 @@ NodePointer Demangler::addFuncSpecParamNumber(NodePointer Param,
23442358
}
23452359

23462360
NodePointer Demangler::demangleSpecAttributes(Node::Kind SpecKind) {
2347-
bool isFragile = nextIf('q');
2361+
bool isSerialized = nextIf('q');
23482362

23492363
int PassID = (int)nextChar() - '0';
23502364
if (PassID < 0 || PassID > 9)
23512365
return nullptr;
23522366

23532367
NodePointer SpecNd = createNode(SpecKind);
2354-
if (isFragile)
2355-
SpecNd->addChild(createNode(Node::Kind::SpecializationIsFragile),
2368+
if (isSerialized)
2369+
SpecNd->addChild(createNode(Node::Kind::IsSerialized),
23562370
*this);
23572371

23582372
SpecNd->addChild(createNode(Node::Kind::SpecializationPassID, PassID),

0 commit comments

Comments
 (0)