Skip to content

Commit aab880d

Browse files
nate-chandlernate-chandler
authored
Merge pull request #77429 from nate-chandler/general-coro/20241104/1
[CoroutineAccessors] Synthesize default requirement implementations.
2 parents a24c448 + 742d5f2 commit aab880d

21 files changed

+602
-169
lines changed

‎include/swift/AST/AnyFunctionRef.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -272,10 +272,9 @@ class AnyFunctionRef {
272272
->getReferenceStorageReferent();
273273
if (mapIntoContext)
274274
valueTy = AD->mapTypeIntoContext(valueTy);
275-
YieldTypeFlags flags(
276-
isYieldingDefaultMutatingAccessor(AD->getAccessorKind())
277-
? ParamSpecifier::InOut
278-
: ParamSpecifier::LegacyShared);
275+
YieldTypeFlags flags(isYieldingMutableAccessor(AD->getAccessorKind())
276+
? ParamSpecifier::InOut
277+
: ParamSpecifier::LegacyShared);
279278
buffer.push_back(AnyFunctionType::Yield(valueTy, flags));
280279
return buffer;
281280
}

‎include/swift/AST/Decl.h

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

60476047
/// Given that CoroutineAccessors is enabled, is _read/_modify required for
60486048
/// ABI stability?
6049-
bool
6050-
requiresCorrespondingUnderscoredCoroutineAccessor(AccessorKind kind) const;
6049+
bool requiresCorrespondingUnderscoredCoroutineAccessor(
6050+
AccessorKind kind, AccessorDecl const *decl = nullptr) const;
60516051

60526052
/// Does this storage have any explicit observers (willSet or didSet) attached
60536053
/// to it?
@@ -6104,9 +6104,9 @@ class AbstractStorageDecl : public ValueDecl {
61046104

61056105
/// Determine how this storage declaration should actually be accessed.
61066106
AccessStrategy getAccessStrategy(AccessSemantics semantics,
6107-
AccessKind accessKind,
6108-
ModuleDecl *module,
6109-
ResilienceExpansion expansion) const;
6107+
AccessKind accessKind, ModuleDecl *module,
6108+
ResilienceExpansion expansion,
6109+
bool useOldABI) const;
61106110

61116111
/// Do we need to use resilient access patterns outside of this
61126112
/// property's resilience domain?
@@ -8441,6 +8441,10 @@ class AccessorDecl final : public FuncDecl {
84418441
/// accessed by it.
84428442
ArrayRef<VarDecl *> getAccessedProperties() const;
84438443

8444+
/// Whether this accessor should have a body. Note that this will be true
8445+
/// even when it does not have one _yet_.
8446+
bool doesAccessorHaveBody() const;
8447+
84448448
static bool classof(const Decl *D) {
84458449
return D->getKind() == DeclKind::Accessor;
84468450
}

‎include/swift/AST/StorageImpl.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ inline bool isYieldingAccessor(AccessorKind kind) {
9595
}
9696
}
9797

98-
inline bool isYieldingDefaultNonmutatingAccessor(AccessorKind kind) {
98+
inline bool isYieldingImmutableAccessor(AccessorKind kind) {
9999
switch (kind) {
100100
case AccessorKind::Read:
101101
case AccessorKind::Read2:
@@ -114,7 +114,7 @@ inline bool isYieldingDefaultNonmutatingAccessor(AccessorKind kind) {
114114
}
115115
}
116116

117-
inline bool isYieldingDefaultMutatingAccessor(AccessorKind kind) {
117+
inline bool isYieldingMutableAccessor(AccessorKind kind) {
118118
switch (kind) {
119119
case AccessorKind::Modify:
120120
case AccessorKind::Modify2:

‎lib/AST/Decl.cpp

Lines changed: 37 additions & 22 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;
@@ -3050,10 +3064,8 @@ bool AbstractStorageDecl::requiresOpaqueModify2Coroutine() const {
30503064
false);
30513065
}
30523066

3053-
AccessorDecl *AbstractStorageDecl::getSynthesizedAccessor(AccessorKind kind) const {
3054-
if (auto *accessor = getAccessor(kind))
3055-
return accessor;
3056-
3067+
AccessorDecl *
3068+
AbstractStorageDecl::getSynthesizedAccessor(AccessorKind kind) const {
30573069
ASTContext &ctx = getASTContext();
30583070
return evaluateOrDefault(ctx.evaluator,
30593071
SynthesizeAccessorRequest{const_cast<AbstractStorageDecl *>(this), kind},
@@ -7112,7 +7124,10 @@ void AbstractStorageDecl::setComputedSetter(AccessorDecl *setter) {
71127124
void
71137125
AbstractStorageDecl::setSynthesizedAccessor(AccessorKind kind,
71147126
AccessorDecl *accessor) {
7115-
assert(!getAccessor(kind) && "accessor already exists");
7127+
if (auto *current = getAccessor(kind)) {
7128+
assert(current == accessor && "different accessor of this kind exists");
7129+
return;
7130+
}
71167131
assert(accessor->getAccessorKind() == kind);
71177132

71187133
auto accessors = Accessors.getPointer();

‎lib/AST/LifetimeDependence.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ static ValueOwnership getLoweredOwnership(AbstractFunctionDecl *afd) {
109109
}
110110
if (auto *ad = dyn_cast<AccessorDecl>(afd)) {
111111
if (ad->getAccessorKind() == AccessorKind::Set ||
112-
isYieldingDefaultMutatingAccessor(ad->getAccessorKind())) {
112+
isYieldingMutableAccessor(ad->getAccessorKind())) {
113113
return ValueOwnership::InOut;
114114
}
115115
}

‎lib/AST/TypeCheckRequests.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -885,9 +885,13 @@ SynthesizeAccessorRequest::getCachedResult() const {
885885
auto *storage = std::get<0>(getStorage());
886886
auto kind = std::get<1>(getStorage());
887887
auto *accessor = storage->getAccessor(kind);
888-
if (accessor)
889-
return accessor;
890-
return std::nullopt;
888+
if (!accessor)
889+
return std::nullopt;
890+
891+
if (accessor->doesAccessorHaveBody() && !accessor->hasBody())
892+
return std::nullopt;
893+
894+
return accessor;
891895
}
892896

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

‎lib/SIL/IR/SILFunctionType.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2202,15 +2202,15 @@ static void destructureYieldsForCoroutine(TypeConverter &TC,
22022202
return;
22032203

22042204
// 'modify' yields an inout of the target type.
2205-
if (isYieldingDefaultMutatingAccessor(accessor->getAccessorKind())) {
2205+
if (isYieldingMutableAccessor(accessor->getAccessorKind())) {
22062206
auto loweredValueTy =
22072207
TC.getLoweredType(origType, canValueType, expansion);
22082208
yields.push_back(SILYieldInfo(loweredValueTy.getASTType(),
22092209
ParameterConvention::Indirect_Inout));
22102210
} else {
22112211
// 'read' yields a borrowed value of the target type, destructuring
22122212
// tuples as necessary.
2213-
assert(isYieldingDefaultNonmutatingAccessor(accessor->getAccessorKind()));
2213+
assert(isYieldingImmutableAccessor(accessor->getAccessorKind()));
22142214
destructureYieldsForReadAccessor(TC, expansion, origType, canValueType,
22152215
yields);
22162216
}

‎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
@@ -3649,8 +3649,8 @@ static SILFunction *getOrCreateKeyPathSetter(SILGenModule &SGM,
36493649

36503650
auto semantics = AccessSemantics::Ordinary;
36513651
auto strategy = property->getAccessStrategy(semantics, AccessKind::Write,
3652-
SGM.M.getSwiftModule(),
3653-
expansion);
3652+
SGM.M.getSwiftModule(), expansion,
3653+
/*useOldABI=*/false);
36543654

36553655
LValueOptions lvOptions;
36563656
lv.addMemberComponent(subSGF, loc, property, subs, lvOptions,
@@ -4211,12 +4211,11 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
42114211
return true;
42124212
};
42134213

4214-
auto strategy = storage->getAccessStrategy(AccessSemantics::Ordinary,
4215-
storage->supportsMutation()
4216-
? AccessKind::ReadWrite
4217-
: AccessKind::Read,
4218-
M.getSwiftModule(),
4219-
expansion);
4214+
auto strategy = storage->getAccessStrategy(
4215+
AccessSemantics::Ordinary,
4216+
storage->supportsMutation() ? AccessKind::ReadWrite : AccessKind::Read,
4217+
M.getSwiftModule(), expansion,
4218+
/*useOldABI=*/false);
42204219

42214220
AbstractStorageDecl *externalDecl = nullptr;
42224221
SubstitutionMap externalSubs;

0 commit comments

Comments
 (0)