Skip to content

Commit 3aedcc0

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 21c1790 commit 3aedcc0

File tree

13 files changed

+417
-172
lines changed

13 files changed

+417
-172
lines changed

include/swift/AST/Decl.h

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

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

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

include/swift/AST/GenericEnvironment.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,14 @@ class alignas(1 << DeclAlignInBits) GenericEnvironment final
327327

328328
/// Returns a substitution map that sends every generic parameter to its
329329
/// corresponding archetype in this generic environment.
330-
SubstitutionMap getForwardingSubstitutionMap() const;
330+
///
331+
/// If \c sig is provided, it must be a generic signature that has the same
332+
/// generic parameters as the generic environment, and a subset of the
333+
/// requirements (in other words, it must be a superset of `this` generic
334+
/// environment). By default, the generic environment's own signature is
335+
/// used.
336+
SubstitutionMap getForwardingSubstitutionMap(
337+
GenericSignature sig = GenericSignature()) const;
331338

332339
void dump(raw_ostream &os) const;
333340

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: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -749,8 +749,12 @@ Type BuildForwardingSubstitutions::operator()(SubstitutableType *type) const {
749749
return Type();
750750
}
751751

752-
SubstitutionMap GenericEnvironment::getForwardingSubstitutionMap() const {
753-
auto genericSig = getGenericSignature();
752+
SubstitutionMap
753+
GenericEnvironment::getForwardingSubstitutionMap(GenericSignature genericSig)
754+
const {
755+
if (!genericSig) {
756+
genericSig = getGenericSignature();
757+
}
754758
return SubstitutionMap::get(genericSig,
755759
BuildForwardingSubstitutions(this),
756760
MakeAbstractConformanceForGenericType());

lib/AST/GenericSignature.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,8 +1306,8 @@ void swift::validateGenericSignaturesInModule(ModuleDecl *module) {
13061306
}
13071307
}
13081308

1309-
GenericSignature
1310-
swift::buildGenericSignature(ASTContext &ctx,
1309+
GenericSignatureWithError
1310+
swift::buildGenericSignatureWithError(ASTContext &ctx,
13111311
GenericSignature baseSignature,
13121312
SmallVector<GenericTypeParamType *, 2> addedParameters,
13131313
SmallVector<Requirement, 2> addedRequirements,
@@ -1319,7 +1319,18 @@ swift::buildGenericSignature(ASTContext &ctx,
13191319
addedParameters,
13201320
addedRequirements,
13211321
allowInverses},
1322-
GenericSignatureWithError()).getPointer();
1322+
GenericSignatureWithError());
1323+
}
1324+
1325+
GenericSignature
1326+
swift::buildGenericSignature(ASTContext &ctx,
1327+
GenericSignature baseSignature,
1328+
SmallVector<GenericTypeParamType *, 2> addedParameters,
1329+
SmallVector<Requirement, 2> addedRequirements,
1330+
bool allowInverses) {
1331+
return buildGenericSignatureWithError(ctx, baseSignature,
1332+
addedParameters, addedRequirements,
1333+
allowInverses).getPointer();
13231334
}
13241335

13251336
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)