Skip to content

Commit de7af46

Browse files
nate-chandlernate-chandler
committed
[CoroutineAccessors] Synthesize default req impls.
When a protocol which has a read (or modify) requirement is built with the CoroutineAccessors feature, it gains a read2 (or modify2, respectively) requirement. For this to be compatible with binaries built without the feature, a default implementation for these new requirements must be provided. Cause these new accessor requirements to have default implementations by returning `true` from `doesAccessorHaveBody` when the context is a `ProtocolDecl` and the relevant availability check passes.
1 parent 7fd15be commit de7af46

15 files changed

+540
-104
lines changed

‎include/swift/AST/Decl.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6050,8 +6050,8 @@ class AbstractStorageDecl : public ValueDecl {
60506050

60516051
/// Given that CoroutineAccessors is enabled, is _read/_modify required for
60526052
/// ABI stability?
6053-
bool
6054-
requiresCorrespondingUnderscoredCoroutineAccessor(AccessorKind kind) const;
6053+
bool requiresCorrespondingUnderscoredCoroutineAccessor(
6054+
AccessorKind kind, AccessorDecl const *decl = nullptr) const;
60556055

60566056
/// Does this storage have any explicit observers (willSet or didSet) attached
60576057
/// to it?
@@ -6108,9 +6108,9 @@ class AbstractStorageDecl : public ValueDecl {
61086108

61096109
/// Determine how this storage declaration should actually be accessed.
61106110
AccessStrategy getAccessStrategy(AccessSemantics semantics,
6111-
AccessKind accessKind,
6112-
ModuleDecl *module,
6113-
ResilienceExpansion expansion) const;
6111+
AccessKind accessKind, ModuleDecl *module,
6112+
ResilienceExpansion expansion,
6113+
bool useOldABI) const;
61146114

61156115
/// Do we need to use resilient access patterns outside of this
61166116
/// property's resilience domain?

‎lib/AST/Decl.cpp

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2819,7 +2819,8 @@ getDirectWriteAccessStrategy(const AbstractStorageDecl *storage) {
28192819
}
28202820

28212821
static AccessStrategy
2822-
getOpaqueReadAccessStrategy(const AbstractStorageDecl *storage, bool dispatch);
2822+
getOpaqueReadAccessStrategy(const AbstractStorageDecl *storage, bool dispatch,
2823+
bool useOldABI);
28232824
static AccessStrategy
28242825
getOpaqueWriteAccessStrategy(const AbstractStorageDecl *storage, bool dispatch);
28252826

@@ -2834,7 +2835,7 @@ getDirectReadWriteAccessStrategy(const AbstractStorageDecl *storage) {
28342835
// If the storage isDynamic (and not @objc) use the accessors.
28352836
if (storage->shouldUseNativeDynamicDispatch())
28362837
return AccessStrategy::getMaterializeToTemporary(
2837-
getOpaqueReadAccessStrategy(storage, false),
2838+
getOpaqueReadAccessStrategy(storage, false, false),
28382839
getOpaqueWriteAccessStrategy(storage, false));
28392840
return AccessStrategy::getStorage();
28402841
}
@@ -2872,7 +2873,13 @@ getDirectReadWriteAccessStrategy(const AbstractStorageDecl *storage) {
28722873
}
28732874

28742875
static AccessStrategy
2875-
getOpaqueReadAccessStrategy(const AbstractStorageDecl *storage, bool dispatch) {
2876+
getOpaqueReadAccessStrategy(const AbstractStorageDecl *storage, bool dispatch,
2877+
bool useOldABI) {
2878+
if (useOldABI) {
2879+
assert(storage->requiresOpaqueRead2Coroutine());
2880+
assert(storage->requiresOpaqueReadCoroutine());
2881+
return AccessStrategy::getAccessor(AccessorKind::Read, dispatch);
2882+
}
28762883
if (storage->requiresOpaqueRead2Coroutine())
28772884
return AccessStrategy::getAccessor(AccessorKind::Read2, dispatch);
28782885
if (storage->requiresOpaqueReadCoroutine())
@@ -2889,35 +2896,39 @@ getOpaqueWriteAccessStrategy(const AbstractStorageDecl *storage, bool dispatch)
28892896

28902897
static AccessStrategy
28912898
getOpaqueReadWriteAccessStrategy(const AbstractStorageDecl *storage,
2892-
bool dispatch) {
2899+
bool dispatch, bool useOldABI) {
2900+
if (useOldABI) {
2901+
assert(storage->requiresOpaqueModify2Coroutine());
2902+
assert(storage->requiresOpaqueModifyCoroutine());
2903+
return AccessStrategy::getAccessor(AccessorKind::Modify, dispatch);
2904+
}
28932905
if (storage->requiresOpaqueModify2Coroutine())
28942906
return AccessStrategy::getAccessor(AccessorKind::Modify2, dispatch);
28952907
if (storage->requiresOpaqueModifyCoroutine())
28962908
return AccessStrategy::getAccessor(AccessorKind::Modify, dispatch);
28972909
return AccessStrategy::getMaterializeToTemporary(
2898-
getOpaqueReadAccessStrategy(storage, dispatch),
2899-
getOpaqueWriteAccessStrategy(storage, dispatch));
2910+
getOpaqueReadAccessStrategy(storage, dispatch, false),
2911+
getOpaqueWriteAccessStrategy(storage, dispatch));
29002912
}
29012913

29022914
static AccessStrategy
29032915
getOpaqueAccessStrategy(const AbstractStorageDecl *storage,
2904-
AccessKind accessKind, bool dispatch) {
2916+
AccessKind accessKind, bool dispatch, bool useOldABI) {
29052917
switch (accessKind) {
29062918
case AccessKind::Read:
2907-
return getOpaqueReadAccessStrategy(storage, dispatch);
2919+
return getOpaqueReadAccessStrategy(storage, dispatch, useOldABI);
29082920
case AccessKind::Write:
2921+
assert(!useOldABI);
29092922
return getOpaqueWriteAccessStrategy(storage, dispatch);
29102923
case AccessKind::ReadWrite:
2911-
return getOpaqueReadWriteAccessStrategy(storage, dispatch);
2924+
return getOpaqueReadWriteAccessStrategy(storage, dispatch, useOldABI);
29122925
}
29132926
llvm_unreachable("bad access kind");
29142927
}
29152928

2916-
AccessStrategy
2917-
AbstractStorageDecl::getAccessStrategy(AccessSemantics semantics,
2918-
AccessKind accessKind,
2919-
ModuleDecl *module,
2920-
ResilienceExpansion expansion) const {
2929+
AccessStrategy AbstractStorageDecl::getAccessStrategy(
2930+
AccessSemantics semantics, AccessKind accessKind, ModuleDecl *module,
2931+
ResilienceExpansion expansion, bool useOldABI) const {
29212932
switch (semantics) {
29222933
case AccessSemantics::DirectToStorage:
29232934
assert(hasStorage() || getASTContext().Diags.hadAnyError());
@@ -2933,10 +2944,12 @@ AbstractStorageDecl::getAccessStrategy(AccessSemantics semantics,
29332944
// If the property is defined in a non-final class or a protocol, the
29342945
// accessors are dynamically dispatched, and we cannot do direct access.
29352946
if (isPolymorphic(this))
2936-
return getOpaqueAccessStrategy(this, accessKind, /*dispatch*/ true);
2947+
return getOpaqueAccessStrategy(this, accessKind, /*dispatch*/ true,
2948+
useOldABI);
29372949

29382950
if (shouldUseNativeDynamicDispatch())
2939-
return getOpaqueAccessStrategy(this, accessKind, /*dispatch*/ false);
2951+
return getOpaqueAccessStrategy(this, accessKind, /*dispatch*/ false,
2952+
useOldABI);
29402953

29412954
// If the storage is resilient from the given module and resilience
29422955
// expansion, we cannot use direct access.
@@ -2958,7 +2971,8 @@ AbstractStorageDecl::getAccessStrategy(AccessSemantics semantics,
29582971
resilient = isResilient();
29592972

29602973
if (resilient)
2961-
return getOpaqueAccessStrategy(this, accessKind, /*dispatch*/ false);
2974+
return getOpaqueAccessStrategy(this, accessKind, /*dispatch*/ false,
2975+
useOldABI);
29622976
}
29632977

29642978
LLVM_FALLTHROUGH;

‎lib/AST/TypeCheckRequests.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -852,9 +852,13 @@ SynthesizeAccessorRequest::getCachedResult() const {
852852
auto *storage = std::get<0>(getStorage());
853853
auto kind = std::get<1>(getStorage());
854854
auto *accessor = storage->getAccessor(kind);
855-
if (accessor)
856-
return accessor;
857-
return std::nullopt;
855+
if (!accessor)
856+
return std::nullopt;
857+
858+
if (accessor->doesAccessorHaveBody() && !accessor->hasBody())
859+
return std::nullopt;
860+
861+
return accessor;
858862
}
859863

860864
void SynthesizeAccessorRequest::cacheResult(AccessorDecl *accessor) const {

‎lib/SILGen/SILGen.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1928,12 +1928,11 @@ SILGenModule::canStorageUseStoredKeyPathComponent(AbstractStorageDecl *decl,
19281928
if (decl->isResilient(M.getSwiftModule(), expansion))
19291929
return false;
19301930

1931-
auto strategy = decl->getAccessStrategy(AccessSemantics::Ordinary,
1932-
decl->supportsMutation()
1933-
? AccessKind::ReadWrite
1934-
: AccessKind::Read,
1935-
M.getSwiftModule(),
1936-
expansion);
1931+
auto strategy = decl->getAccessStrategy(
1932+
AccessSemantics::Ordinary,
1933+
decl->supportsMutation() ? AccessKind::ReadWrite : AccessKind::Read,
1934+
M.getSwiftModule(), expansion,
1935+
/*useOldABI=*/false);
19371936
switch (strategy.getKind()) {
19381937
case AccessStrategy::Storage: {
19391938
// Keypaths rely on accessors to handle the special behavior of weak,

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3669,8 +3669,8 @@ static SILFunction *getOrCreateKeyPathSetter(SILGenModule &SGM,
36693669

36703670
auto semantics = AccessSemantics::Ordinary;
36713671
auto strategy = property->getAccessStrategy(semantics, AccessKind::Write,
3672-
SGM.M.getSwiftModule(),
3673-
expansion);
3672+
SGM.M.getSwiftModule(), expansion,
3673+
/*useOldABI=*/false);
36743674

36753675
LValueOptions lvOptions;
36763676
lv.addMemberComponent(subSGF, loc, property, subs, lvOptions,
@@ -4231,12 +4231,11 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
42314231
return true;
42324232
};
42334233

4234-
auto strategy = storage->getAccessStrategy(AccessSemantics::Ordinary,
4235-
storage->supportsMutation()
4236-
? AccessKind::ReadWrite
4237-
: AccessKind::Read,
4238-
M.getSwiftModule(),
4239-
expansion);
4234+
auto strategy = storage->getAccessStrategy(
4235+
AccessSemantics::Ordinary,
4236+
storage->supportsMutation() ? AccessKind::ReadWrite : AccessKind::Read,
4237+
M.getSwiftModule(), expansion,
4238+
/*useOldABI=*/false);
42404239

42414240
AbstractStorageDecl *externalDecl = nullptr;
42424241
SubstitutionMap externalSubs;

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 48 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3068,9 +3068,12 @@ class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
30683068
assert(!e->getType()->is<LValueType>());
30693069

30703070
auto accessSemantics = e->getAccessSemantics();
3071+
// FIXME: FIXME_NOW: CoroutineAccessors: Is this the right place to pass
3072+
// true sometimes?
30713073
AccessStrategy strategy = var->getAccessStrategy(
30723074
accessSemantics, getFormalAccessKind(accessKind),
3073-
SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion());
3075+
SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(),
3076+
/*useOldABI=*/false);
30743077

30753078
auto baseFormalType = getBaseFormalType(e->getBase());
30763079
LValue lv = visit(
@@ -3292,9 +3295,9 @@ static LValue emitLValueForNonMemberVarDecl(
32923295
subs = SGF.F.getForwardingSubstitutionMap();
32933296

32943297
auto access = getFormalAccessKind(accessKind);
3295-
auto strategy =
3296-
var->getAccessStrategy(semantics, access, SGF.SGM.M.getSwiftModule(),
3297-
SGF.F.getResilienceExpansion());
3298+
auto strategy = var->getAccessStrategy(
3299+
semantics, access, SGF.SGM.M.getSwiftModule(),
3300+
SGF.F.getResilienceExpansion(), /*useOldABI=*/false);
32983301

32993302
lv.addNonMemberVarComponent(SGF, loc, var, subs, options, accessKind,
33003303
strategy, formalRValueType, actorIso);
@@ -3887,6 +3890,25 @@ static bool isCurrentFunctionAccessor(SILGenFunction &SGF,
38873890
contextAccessorDecl->getAccessorKind() == accessorKind;
38883891
}
38893892

3893+
static bool isSynthesizedDefaultImplementionatThunk(SILGenFunction &SGF) {
3894+
if (!SGF.FunctionDC)
3895+
return false;
3896+
3897+
auto *decl = SGF.FunctionDC->getAsDecl();
3898+
if (!decl)
3899+
return false;
3900+
3901+
auto *dc = decl->getDeclContext();
3902+
if (!dc)
3903+
return false;
3904+
3905+
auto *proto = dyn_cast<ProtocolDecl>(dc);
3906+
if (!proto)
3907+
return false;
3908+
3909+
return true;
3910+
}
3911+
38903912
LValue SILGenLValue::visitMemberRefExpr(MemberRefExpr *e,
38913913
SGFAccessKind accessKind,
38923914
LValueOptions options) {
@@ -3908,11 +3930,10 @@ LValue SILGenLValue::visitMemberRefExpr(MemberRefExpr *e,
39083930
}
39093931

39103932
auto accessSemantics = e->getAccessSemantics();
3911-
AccessStrategy strategy =
3912-
var->getAccessStrategy(accessSemantics,
3913-
getFormalAccessKind(accessKind),
3914-
SGF.SGM.M.getSwiftModule(),
3915-
SGF.F.getResilienceExpansion());
3933+
AccessStrategy strategy = var->getAccessStrategy(
3934+
accessSemantics, getFormalAccessKind(accessKind),
3935+
SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(),
3936+
/*useOldABI=*/isSynthesizedDefaultImplementionatThunk(SGF));
39163937

39173938
bool isOnSelfParameter = isCallToSelfOfCurrentFunction(SGF, e);
39183939

@@ -3931,10 +3952,9 @@ LValue SILGenLValue::visitMemberRefExpr(MemberRefExpr *e,
39313952
SGF, readAccessor.getAbstractFunctionDecl(), isObjC)) {
39323953
accessSemantics = AccessSemantics::DirectToImplementation;
39333954
strategy = var->getAccessStrategy(
3934-
accessSemantics,
3935-
getFormalAccessKind(accessKind),
3936-
SGF.SGM.M.getSwiftModule(),
3937-
SGF.F.getResilienceExpansion());
3955+
accessSemantics, getFormalAccessKind(accessKind),
3956+
SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(),
3957+
/*useOldABI=*/false);
39383958
}
39393959
}
39403960

@@ -4119,11 +4139,10 @@ LValue SILGenLValue::visitSubscriptExpr(SubscriptExpr *e,
41194139

41204140

41214141
auto accessSemantics = e->getAccessSemantics();
4122-
auto strategy =
4123-
decl->getAccessStrategy(accessSemantics,
4124-
getFormalAccessKind(accessKind),
4125-
SGF.SGM.M.getSwiftModule(),
4126-
SGF.F.getResilienceExpansion());
4142+
auto strategy = decl->getAccessStrategy(
4143+
accessSemantics, getFormalAccessKind(accessKind),
4144+
SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(),
4145+
/*useOldABI=*/isSynthesizedDefaultImplementionatThunk(SGF));
41274146

41284147
bool isOnSelfParameter = isCallToSelfOfCurrentFunction(SGF, e);
41294148
bool isContextRead = isCurrentFunctionAccessor(SGF, AccessorKind::Read);
@@ -4141,10 +4160,9 @@ LValue SILGenLValue::visitSubscriptExpr(SubscriptExpr *e,
41414160
SGF, readAccessor.getAbstractFunctionDecl(), isObjC)) {
41424161
accessSemantics = AccessSemantics::DirectToImplementation;
41434162
strategy = decl->getAccessStrategy(
4144-
accessSemantics,
4145-
getFormalAccessKind(accessKind),
4146-
SGF.SGM.M.getSwiftModule(),
4147-
SGF.F.getResilienceExpansion());
4163+
accessSemantics, getFormalAccessKind(accessKind),
4164+
SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(),
4165+
/*useOldABI=*/false);
41484166
}
41494167
}
41504168

@@ -4597,11 +4615,10 @@ LValue SILGenFunction::emitPropertyLValue(SILLocation loc, ManagedValue base,
45974615
auto baseType = base.getType().getASTType();
45984616
auto subMap = baseType->getContextSubstitutionMap(ivar->getDeclContext());
45994617

4600-
AccessStrategy strategy =
4601-
ivar->getAccessStrategy(semantics,
4602-
getFormalAccessKind(accessKind),
4603-
SGM.M.getSwiftModule(),
4604-
F.getResilienceExpansion());
4618+
// FIXME: FIXME_NOW: CoroutineAccessors: Is this the place?
4619+
AccessStrategy strategy = ivar->getAccessStrategy(
4620+
semantics, getFormalAccessKind(accessKind), SGM.M.getSwiftModule(),
4621+
F.getResilienceExpansion(), /*useOldABI=*/false);
46054622

46064623
auto baseAccessKind =
46074624
getBaseAccessKind(SGM, ivar, accessKind, strategy, baseFormalType,
@@ -5316,10 +5333,10 @@ RValue SILGenFunction::emitRValueForStorageLoad(
53165333
SubstitutionMap substitutions,
53175334
AccessSemantics semantics, Type propTy, SGFContext C,
53185335
bool isBaseGuaranteed) {
5319-
AccessStrategy strategy =
5320-
storage->getAccessStrategy(semantics, AccessKind::Read,
5321-
SGM.M.getSwiftModule(),
5322-
F.getResilienceExpansion());
5336+
// FIXME: FIXME_NOW: CoroutineAccessors: Is this the place?
5337+
AccessStrategy strategy = storage->getAccessStrategy(
5338+
semantics, AccessKind::Read, SGM.M.getSwiftModule(),
5339+
F.getResilienceExpansion(), /*useOldABI=*/false);
53235340

53245341
// If we should call an accessor of some kind, do so.
53255342
if (strategy.getKind() != AccessStrategy::Storage) {

‎lib/SILGen/SILGenPattern.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3316,11 +3316,11 @@ static bool isBorrowableSubject(SILGenFunction &SGF,
33163316
}
33173317

33183318
// Check the access strategy used to read the storage.
3319-
auto strategy = storage->getAccessStrategy(access,
3320-
AccessKind::Read,
3321-
SGF.SGM.SwiftModule,
3322-
SGF.F.getResilienceExpansion());
3323-
3319+
auto strategy =
3320+
storage->getAccessStrategy(access, AccessKind::Read, SGF.SGM.SwiftModule,
3321+
SGF.F.getResilienceExpansion(),
3322+
/*useOldABI=*/false);
3323+
33243324
switch (strategy.getKind()) {
33253325
case AccessStrategy::Kind::Storage:
33263326
// Accessing storage directly benefits from borrowing.

‎lib/SILGen/SILGenType.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -835,12 +835,16 @@ SILFunction *SILGenModule::emitProtocolWitness(
835835
if (witnessRef.isAlwaysInline())
836836
InlineStrategy = AlwaysInline;
837837

838+
SILFunction *f = M.lookUpFunction(nameBuffer);
839+
if (f)
840+
return f;
841+
838842
SILGenFunctionBuilder builder(*this);
839-
auto *f = builder.createFunction(
843+
f = builder.createFunction(
840844
linkage, nameBuffer, witnessSILFnType, genericEnv,
841-
SILLocation(witnessRef.getDecl()), IsNotBare, IsTransparent, serializedKind,
842-
IsNotDynamic, IsNotDistributed, IsNotRuntimeAccessible, ProfileCounter(),
843-
IsThunk, SubclassScope::NotApplicable, InlineStrategy);
845+
SILLocation(witnessRef.getDecl()), IsNotBare, IsTransparent,
846+
serializedKind, IsNotDynamic, IsNotDistributed, IsNotRuntimeAccessible,
847+
ProfileCounter(), IsThunk, SubclassScope::NotApplicable, InlineStrategy);
844848

845849
f->setDebugScope(new (M)
846850
SILDebugScope(RegularLocation(witnessRef.getDecl()), f));
@@ -1055,6 +1059,10 @@ class SILGenDefaultWitnessTable
10551059
SILDeclRef witnessRef,
10561060
IsFreeFunctionWitness_t isFree,
10571061
Witness witness) {
1062+
if (!cast<AbstractFunctionDecl>(witnessRef.getDecl())->hasBody()) {
1063+
addMissingDefault();
1064+
return;
1065+
}
10581066
SILFunction *witnessFn = SGM.emitProtocolWitness(
10591067
ProtocolConformanceRef(Proto), SILLinkage::Private, IsNotSerialized,
10601068
requirementRef, witnessRef, isFree, witness);

0 commit comments

Comments
 (0)