Skip to content

Commit 22eb7e6

Browse files
committed
SILGen: Emit property descriptors for conditionally Copyable and Escapable types.
Key paths can't reference non-escapable or non-copyable storage declarations, so we don't need to refer to them resiliently, and can elide their property descriptors. However, declarations may still be conditionally Copyable and Escapable, and if so, then they still need a property descriptor for resilient key path references. When a property or subscript can be used in a context where it is fully Copyable and Escapable, emit the property descriptor in a generic environment constrained by the necessary conditional constraints. Fixes rdar://151628396.
1 parent 7387f85 commit 22eb7e6

File tree

12 files changed

+406
-179
lines changed

12 files changed

+406
-179
lines changed

include/swift/AST/Decl.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6276,9 +6276,12 @@ class AbstractStorageDecl : public ValueDecl {
62766276
/// Otherwise, its override must be referenced.
62776277
bool isValidKeyPathComponent() const;
62786278

6279-
/// True if the storage exports a property descriptor for key paths in
6280-
/// other modules.
6281-
bool exportsPropertyDescriptor() const;
6279+
/// If the storage exports a property descriptor for key paths in other
6280+
/// modules, this returns the generic signature in which its member methods
6281+
/// are emitted. If the storage does not export a property descriptor,
6282+
/// returns `std::nullopt`.
6283+
std::optional<GenericSignature>
6284+
getPropertyDescriptorGenericSignature() const;
62826285

62836286
/// True if any of the accessors to the storage is private or fileprivate.
62846287
bool hasPrivateAccessor() const;

include/swift/AST/GenericSignature.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,23 @@ using GenericSignatureErrors = OptionSet<GenericSignatureErrorFlags>;
617617
using GenericSignatureWithError = llvm::PointerIntPair<GenericSignature, 3,
618618
GenericSignatureErrors>;
619619

620+
/// Build a generic signature from the given requirements, which are not
621+
/// required to be minimal or canonical, and may contain unresolved
622+
/// DependentMemberTypes. The generic signature is returned with the
623+
/// error flags (if any) that were raised while building the signature.
624+
///
625+
/// \param baseSignature if non-null, the new parameters and requirements
626+
///// are added on; existing requirements of the base signature might become
627+
///// redundant. Otherwise if null, build a new signature from scratch.
628+
/// \param allowInverses if true, default requirements to Copyable/Escapable are
629+
/// expanded for generic parameters.
630+
GenericSignatureWithError buildGenericSignatureWithError(
631+
ASTContext &ctx,
632+
GenericSignature baseSignature,
633+
SmallVector<GenericTypeParamType *, 2> addedParameters,
634+
SmallVector<Requirement, 2> addedRequirements,
635+
bool allowInverses);
636+
620637
} // end namespace swift
621638

622639
namespace llvm {

include/swift/IRGen/Linking.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1043,7 +1043,7 @@ class LinkEntity {
10431043
}
10441044

10451045
static LinkEntity forPropertyDescriptor(AbstractStorageDecl *decl) {
1046-
assert(decl->exportsPropertyDescriptor());
1046+
assert((bool)decl->getPropertyDescriptorGenericSignature());
10471047
LinkEntity entity;
10481048
entity.setForDecl(Kind::PropertyDescriptor, decl);
10491049
return entity;

lib/AST/GenericEnvironment.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,8 @@ Type BuildForwardingSubstitutions::operator()(SubstitutableType *type) const {
749749
return Type();
750750
}
751751

752-
SubstitutionMap GenericEnvironment::getForwardingSubstitutionMap() const {
752+
SubstitutionMap
753+
GenericEnvironment::getForwardingSubstitutionMap() const {
753754
auto genericSig = getGenericSignature();
754755
return SubstitutionMap::get(genericSig,
755756
BuildForwardingSubstitutions(this),

lib/AST/GenericSignature.cpp

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1179,15 +1179,11 @@ void swift::validateGenericSignature(ASTContext &context,
11791179
{
11801180
PrettyStackTraceGenericSignature debugStack("verifying", sig);
11811181

1182-
auto newSigWithError = evaluateOrDefault(
1183-
context.evaluator,
1184-
AbstractGenericSignatureRequest{
1185-
nullptr,
1186-
genericParams,
1187-
requirements,
1188-
/*allowInverses=*/false},
1189-
GenericSignatureWithError());
1190-
1182+
auto newSigWithError = buildGenericSignatureWithError(context,
1183+
GenericSignature(),
1184+
genericParams,
1185+
requirements,
1186+
/*allowInverses*/ false);
11911187
// If there were any errors, the signature was invalid.
11921188
auto errorFlags = newSigWithError.getInt();
11931189
if (errorFlags.contains(GenericSignatureErrorFlags::HasInvalidRequirements) ||
@@ -1307,8 +1303,8 @@ void swift::validateGenericSignaturesInModule(ModuleDecl *module) {
13071303
}
13081304
}
13091305

1310-
GenericSignature
1311-
swift::buildGenericSignature(ASTContext &ctx,
1306+
GenericSignatureWithError
1307+
swift::buildGenericSignatureWithError(ASTContext &ctx,
13121308
GenericSignature baseSignature,
13131309
SmallVector<GenericTypeParamType *, 2> addedParameters,
13141310
SmallVector<Requirement, 2> addedRequirements,
@@ -1320,7 +1316,18 @@ swift::buildGenericSignature(ASTContext &ctx,
13201316
addedParameters,
13211317
addedRequirements,
13221318
allowInverses},
1323-
GenericSignatureWithError()).getPointer();
1319+
GenericSignatureWithError());
1320+
}
1321+
1322+
GenericSignature
1323+
swift::buildGenericSignature(ASTContext &ctx,
1324+
GenericSignature baseSignature,
1325+
SmallVector<GenericTypeParamType *, 2> addedParameters,
1326+
SmallVector<Requirement, 2> addedRequirements,
1327+
bool allowInverses) {
1328+
return buildGenericSignatureWithError(ctx, baseSignature,
1329+
addedParameters, addedRequirements,
1330+
allowInverses).getPointer();
13241331
}
13251332

13261333
GenericSignature GenericSignature::withoutMarkerProtocols() const {

lib/SIL/IR/SIL.cpp

Lines changed: 140 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "swift/SIL/SILUndef.h"
1919
#include "swift/AST/ASTContext.h"
2020
#include "swift/AST/AnyFunctionRef.h"
21+
#include "swift/AST/ConformanceLookup.h"
2122
#include "swift/AST/Decl.h"
2223
#include "swift/AST/GenericEnvironment.h"
2324
#include "swift/AST/Pattern.h"
@@ -307,18 +308,106 @@ bool SILModule::isTypeMetadataForLayoutAccessible(SILType type) {
307308
return ::isTypeMetadataForLayoutAccessible(*this, type);
308309
}
309310

310-
static bool isUnsupportedKeyPathValueType(Type ty) {
311+
// Given the type `ty`, which should be in the generic environment of the signature
312+
// `sig`, return a generic signature with all of the requirements of `sig`,
313+
// combined with all of the requirements necessary for `ty` to be both
314+
// `Copyable` and `Escapable`, if possible. Returns `nullopt` if the type
315+
// can never be both Copyable and Escapable.
316+
static std::optional<GenericSignature>
317+
getKeyPathSupportingGenericSignature(Type ty, GenericSignature contextSig) {
318+
auto &C = ty->getASTContext();
319+
320+
// If the type is already unconditionally Copyable and Escapable, we don't
321+
// need any further requirements.
322+
if (!ty->isNoncopyable() && ty->isEscapable()) {
323+
return contextSig;
324+
}
325+
326+
ProtocolConformanceRef copyable, escapable;
327+
auto copyableProtocol = C.getProtocol(KnownProtocolKind::Copyable);
328+
auto escapableProtocol = C.getProtocol(KnownProtocolKind::Escapable);
329+
330+
// If the type is an archetype, then it just needs Copyable and Escapable
331+
// constraints imposed.
332+
if (ty->is<ArchetypeType>()) {
333+
copyable = ProtocolConformanceRef::forAbstract(ty->mapTypeOutOfContext(),
334+
copyableProtocol);
335+
escapable = ProtocolConformanceRef::forAbstract(ty->mapTypeOutOfContext(),
336+
escapableProtocol);
337+
} else {
338+
// Look for any conditional conformances.
339+
copyable = lookupConformance(ty, copyableProtocol);
340+
escapable = lookupConformance(ty, escapableProtocol);
341+
}
342+
343+
// If the type is never copyable or escapable, that's it.
344+
if (copyable.isInvalid() || escapable.isInvalid()) {
345+
return std::nullopt;
346+
}
347+
348+
// Otherwise, let's see if we get a viable generic signature combining the
349+
// requirements for those conformances with the requirements of the
350+
// declaration context.
351+
SmallVector<Requirement, 2> ceRequirements;
352+
353+
auto getRequirementsFromConformance = [&](ProtocolConformanceRef ref) {
354+
if (ref.isAbstract()) {
355+
// The only requirements are that the abstract type itself be copyable
356+
// and escapable.
357+
ceRequirements.push_back(Requirement(RequirementKind::Conformance,
358+
ty->mapTypeOutOfContext(), copyableProtocol->getDeclaredType()));
359+
ceRequirements.push_back(Requirement(RequirementKind::Conformance,
360+
ty->mapTypeOutOfContext(), escapableProtocol->getDeclaredType()));
361+
return;
362+
}
363+
364+
if (!ref.isConcrete()) {
365+
return;
366+
}
367+
auto conformance = ref.getConcrete();
368+
369+
for (auto reqt : conformance->getRootConformance()
370+
->getConditionalRequirements()) {
371+
ceRequirements.push_back(reqt);
372+
}
373+
};
374+
getRequirementsFromConformance(copyable);
375+
getRequirementsFromConformance(escapable);
376+
377+
auto regularSignature = buildGenericSignatureWithError(C,
378+
contextSig,
379+
{},
380+
std::move(ceRequirements),
381+
/*allowInverses*/ false);
382+
383+
// If the resulting signature has conflicting requirements, then it is
384+
// impossible for the type to be copyable and equatable.
385+
if (regularSignature.getInt()) {
386+
return std::nullopt;
387+
}
388+
389+
// Otherwise, we have the signature we're looking for.
390+
return regularSignature.getPointer();
391+
}
392+
393+
static std::optional<GenericSignature>
394+
getKeyPathSupportingGenericSignatureForValueType(Type ty,
395+
GenericSignature sig) {
396+
std::optional<GenericSignature> contextSig = sig;
397+
311398
// Visit lowered positions.
312399
if (auto tupleTy = ty->getAs<TupleType>()) {
313400
for (auto eltTy : tupleTy->getElementTypes()) {
314401
if (eltTy->is<PackExpansionType>())
315-
return true;
402+
return std::nullopt;
316403

317-
if (isUnsupportedKeyPathValueType(eltTy))
318-
return true;
404+
contextSig = getKeyPathSupportingGenericSignatureForValueType(
405+
eltTy, *contextSig);
406+
if (!contextSig)
407+
return std::nullopt;
319408
}
320409

321-
return false;
410+
return contextSig;
322411
}
323412

324413
if (auto objTy = ty->getOptionalObjectType())
@@ -330,66 +419,78 @@ static bool isUnsupportedKeyPathValueType(Type ty) {
330419
for (auto param : funcTy->getParams()) {
331420
auto paramTy = param.getPlainType();
332421
if (paramTy->is<PackExpansionType>())
333-
return true;
422+
return std::nullopt;
334423

335-
if (isUnsupportedKeyPathValueType(paramTy))
336-
return true;
424+
contextSig = getKeyPathSupportingGenericSignatureForValueType(paramTy,
425+
*contextSig);
426+
if (!contextSig) {
427+
return std::nullopt;
428+
}
337429
}
338430

339-
if (isUnsupportedKeyPathValueType(funcTy->getResult()))
340-
return true;
431+
contextSig = getKeyPathSupportingGenericSignatureForValueType(funcTy->getResult(),
432+
*contextSig);
433+
434+
if (!contextSig) {
435+
return std::nullopt;
436+
}
341437
}
342438

343439
// Noncopyable types aren't supported by key paths in their current form.
344440
// They would also need a new ABI that's yet to be implemented in order to
345441
// be properly supported, so let's suppress the descriptor for now if either
346442
// the container or storage type of the declaration is non-copyable.
347-
if (ty->isNoncopyable())
348-
return true;
349-
350-
return false;
443+
return getKeyPathSupportingGenericSignature(ty, *contextSig);
351444
}
352445

353-
bool AbstractStorageDecl::exportsPropertyDescriptor() const {
446+
std::optional<GenericSignature>
447+
AbstractStorageDecl::getPropertyDescriptorGenericSignature() const {
354448
// The storage needs a descriptor if it sits at a module's ABI boundary,
355-
// meaning it has public linkage.
449+
// meaning it has public linkage, and it is eligible to be part of a key path.
356450

357-
if (!isStatic()) {
358-
if (auto contextTy = getDeclContext()->getDeclaredTypeInContext()) {
359-
if (contextTy->isNoncopyable()) {
360-
return false;
361-
}
451+
auto contextTy = getDeclContext()->getDeclaredTypeInContext();
452+
auto contextSig = getInnermostDeclContext()->getGenericSignatureOfContext();
453+
454+
// If the root type is never `Copyable` or `Escapable`, then instance
455+
// members can't be used in key paths, at least as they are implemented
456+
// today.
457+
if (!isStatic() && contextTy) {
458+
auto ceContextSig = getKeyPathSupportingGenericSignature(contextTy,
459+
contextSig);
460+
if (!ceContextSig) {
461+
return std::nullopt;
362462
}
463+
contextSig = *ceContextSig;
363464
}
364465

365466
// TODO: Global properties ought to eventually be referenceable
366467
// as key paths from ().
367468
if (!getDeclContext()->isTypeContext())
368-
return false;
469+
return std::nullopt;
369470

370471
// Protocol requirements do not need property descriptors.
371472
if (isa<ProtocolDecl>(getDeclContext()))
372-
return false;
473+
return std::nullopt;
373474

374475
// Static properties declared directly in protocol do not need
375476
// descriptors as existential Any.Type will not resolve to a value.
376477
if (isStatic() && isa<ProtocolDecl>(getDeclContext()))
377-
return false;
478+
return std::nullopt;
378479

379480
// FIXME: We should support properties and subscripts with '_read' accessors;
380481
// 'get' is not part of the opaque accessor set there.
381482
auto *getter = getOpaqueAccessor(AccessorKind::Get);
382483
if (!getter)
383-
return false;
484+
return std::nullopt;
384485

385486
// If the getter is mutating, we cannot form a keypath to it at all.
386487
if (isGetterMutating())
387-
return false;
488+
return std::nullopt;
388489

389490
// If the storage is an ABI-compatible override of another declaration, we're
390491
// not going to be emitting a property descriptor either.
391492
if (!isValidKeyPathComponent())
392-
return false;
493+
return std::nullopt;
393494

394495
// TODO: If previous versions of an ABI-stable binary needed the descriptor,
395496
// then we still do.
@@ -409,27 +510,30 @@ bool AbstractStorageDecl::exportsPropertyDescriptor() const {
409510
case SILLinkage::Private:
410511
case SILLinkage::Hidden:
411512
// Don't need a public descriptor.
412-
return false;
513+
return std::nullopt;
413514

414515
case SILLinkage::HiddenExternal:
415516
case SILLinkage::PublicExternal:
416517
case SILLinkage::PackageExternal:
417518
llvm_unreachable("should be definition linkage?");
418519
}
419520

420-
auto typeInContext = getInnermostDeclContext()->mapTypeIntoContext(
521+
auto typeInContext = contextSig.getGenericEnvironment()->mapTypeIntoContext(
421522
getValueInterfaceType());
422-
if (isUnsupportedKeyPathValueType(typeInContext)) {
423-
return false;
523+
auto valueTypeSig = getKeyPathSupportingGenericSignatureForValueType(typeInContext, contextSig);
524+
if (!valueTypeSig) {
525+
return std::nullopt;
424526
}
527+
contextSig = *valueTypeSig;
425528

426529
// Subscripts with inout arguments (FIXME)and reabstracted arguments(/FIXME)
427530
// don't have descriptors either.
428531
if (auto sub = dyn_cast<SubscriptDecl>(this)) {
429532
for (auto *index : *sub->getIndices()) {
430533
// Keypaths can't capture inout indices.
431-
if (index->isInOut())
432-
return false;
534+
if (index->isInOut()) {
535+
return std::nullopt;
536+
}
433537

434538
auto indexTy = index->getInterfaceType()
435539
->getReducedType(sub->getGenericSignatureOfContext());
@@ -439,17 +543,17 @@ bool AbstractStorageDecl::exportsPropertyDescriptor() const {
439543
// had only one abstraction level and no explosion.
440544

441545
if (isa<TupleType>(indexTy))
442-
return false;
546+
return std::nullopt;
443547

444548
auto indexObjTy = indexTy;
445549
if (auto objTy = indexObjTy.getOptionalObjectType())
446550
indexObjTy = objTy;
447551

448552
if (isa<AnyFunctionType>(indexObjTy)
449553
|| isa<AnyMetatypeType>(indexObjTy))
450-
return false;
554+
return std::nullopt;
451555
}
452556
}
453557

454-
return true;
558+
return contextSig;
455559
}

lib/SIL/IR/SILSymbolVisitor.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ class SILSymbolVisitorImpl : public ASTVisitor<SILSymbolVisitorImpl> {
569569

570570
void visitAbstractStorageDecl(AbstractStorageDecl *ASD) {
571571
// Add the property descriptor if the decl needs it.
572-
if (ASD->exportsPropertyDescriptor()) {
572+
if (ASD->getPropertyDescriptorGenericSignature()) {
573573
Visitor.addPropertyDescriptor(ASD);
574574
}
575575

0 commit comments

Comments
 (0)