Skip to content

Commit 65a6d03

Browse files
nate-chandlernate-chandler
authored
Merge pull request #77265 from nate-chandler/general-coro/20241024/1
[CoroutineAccessors] Key table membership off availability.
2 parents 0d6f074 + b87b42c commit 65a6d03

File tree

13 files changed

+906
-221
lines changed

13 files changed

+906
-221
lines changed

‎include/swift/AST/Decl.h‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5765,6 +5765,8 @@ class AbstractStorageDecl : public ValueDecl {
57655765
unsigned RequiresOpaqueAccessors : 1;
57665766
unsigned RequiresOpaqueModifyCoroutineComputed : 1;
57675767
unsigned RequiresOpaqueModifyCoroutine : 1;
5768+
unsigned RequiresOpaqueModify2CoroutineComputed : 1;
5769+
unsigned RequiresOpaqueModify2Coroutine : 1;
57685770
} LazySemanticInfo = { };
57695771

57705772
/// The implementation info for the accessors.
@@ -6029,17 +6031,28 @@ class AbstractStorageDecl : public ValueDecl {
60296031
return getOpaqueReadOwnership() != OpaqueReadOwnership::Borrowed;
60306032
}
60316033

6034+
/// Does this storage require a '_read' accessor in its opaque-accessors set?
6035+
bool requiresOpaqueReadCoroutine() const;
6036+
60326037
/// Does this storage require a 'read' accessor in its opaque-accessors set?
6033-
bool requiresOpaqueReadCoroutine() const {
6034-
return getOpaqueReadOwnership() != OpaqueReadOwnership::Owned;
6035-
}
6038+
bool requiresOpaqueRead2Coroutine() const;
60366039

60376040
/// Does this storage require a 'set' accessor in its opaque-accessors set?
60386041
bool requiresOpaqueSetter() const { return supportsMutation(); }
60396042

6040-
/// Does this storage require a 'modify' accessor in its opaque-accessors set?
6043+
/// Does this storage require a '_modify' accessor in its opaque-accessors
6044+
/// set?
60416045
bool requiresOpaqueModifyCoroutine() const;
60426046

6047+
/// Does this storage require a 'modify' accessor in its opaque-accessors
6048+
/// set?
6049+
bool requiresOpaqueModify2Coroutine() const;
6050+
6051+
/// Given that CoroutineAccessors is enabled, is _read/_modify required for
6052+
/// ABI stability?
6053+
bool
6054+
requiresCorrespondingUnderscoredCoroutineAccessor(AccessorKind kind) const;
6055+
60436056
/// Does this storage have any explicit observers (willSet or didSet) attached
60446057
/// to it?
60456058
bool hasObservers() const {

‎include/swift/AST/FeatureAvailability.def‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ FEATURE(ClearSensitive, FUTURE)
8080
FEATURE(UpdatePureObjCClassMetadata, FUTURE)
8181
FEATURE(ValueGenericType, FUTURE)
8282
FEATURE(InitRawStructMetadata2, FUTURE)
83+
// TODO: CoroutineAccessors: Change to correct version number.
84+
FEATURE(CoroutineAccessors, FUTURE)
8385

8486
#undef FEATURE
8587
#undef FUTURE

‎include/swift/AST/TypeCheckRequests.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,19 +1937,19 @@ class RequiresOpaqueAccessorsRequest :
19371937
void cacheResult(bool value) const;
19381938
};
19391939

1940-
class RequiresOpaqueModifyCoroutineRequest :
1941-
public SimpleRequest<RequiresOpaqueModifyCoroutineRequest,
1942-
bool(AbstractStorageDecl *),
1943-
RequestFlags::SeparatelyCached> {
1940+
class RequiresOpaqueModifyCoroutineRequest
1941+
: public SimpleRequest<RequiresOpaqueModifyCoroutineRequest,
1942+
bool(AbstractStorageDecl *, bool isUnderscored),
1943+
RequestFlags::SeparatelyCached> {
19441944
public:
19451945
using SimpleRequest::SimpleRequest;
19461946

19471947
private:
19481948
friend SimpleRequest;
19491949

19501950
// Evaluation.
1951-
bool
1952-
evaluate(Evaluator &evaluator, AbstractStorageDecl *decl) const;
1951+
bool evaluate(Evaluator &evaluator, AbstractStorageDecl *decl,
1952+
bool isUnderscored) const;
19531953

19541954
public:
19551955
// Separate caching.

‎lib/AST/Decl.cpp‎

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2849,8 +2849,13 @@ getDirectReadWriteAccessStrategy(const AbstractStorageDecl *storage) {
28492849
/*dispatch*/ false);
28502850
case ReadWriteImplKind::StoredWithDidSet:
28512851
case ReadWriteImplKind::InheritedWithDidSet:
2852-
if (storage->requiresOpaqueModifyCoroutine() &&
2852+
if (storage->requiresOpaqueModify2Coroutine() &&
28532853
storage->getParsedAccessor(AccessorKind::DidSet)->isSimpleDidSet()) {
2854+
return AccessStrategy::getAccessor(AccessorKind::Modify2,
2855+
/*dispatch*/ false);
2856+
} else if (storage->requiresOpaqueModifyCoroutine() &&
2857+
storage->getParsedAccessor(AccessorKind::DidSet)
2858+
->isSimpleDidSet()) {
28542859
return AccessStrategy::getAccessor(AccessorKind::Modify,
28552860
/*dispatch*/ false);
28562861
} else {
@@ -2868,6 +2873,8 @@ getDirectReadWriteAccessStrategy(const AbstractStorageDecl *storage) {
28682873

28692874
static AccessStrategy
28702875
getOpaqueReadAccessStrategy(const AbstractStorageDecl *storage, bool dispatch) {
2876+
if (storage->requiresOpaqueRead2Coroutine())
2877+
return AccessStrategy::getAccessor(AccessorKind::Read2, dispatch);
28712878
if (storage->requiresOpaqueReadCoroutine())
28722879
return AccessStrategy::getAccessor(AccessorKind::Read, dispatch);
28732880
return AccessStrategy::getAccessor(AccessorKind::Get, dispatch);
@@ -2883,6 +2890,8 @@ getOpaqueWriteAccessStrategy(const AbstractStorageDecl *storage, bool dispatch)
28832890
static AccessStrategy
28842891
getOpaqueReadWriteAccessStrategy(const AbstractStorageDecl *storage,
28852892
bool dispatch) {
2893+
if (storage->requiresOpaqueModify2Coroutine())
2894+
return AccessStrategy::getAccessor(AccessorKind::Modify2, dispatch);
28862895
if (storage->requiresOpaqueModifyCoroutine())
28872896
return AccessStrategy::getAccessor(AccessorKind::Modify, dispatch);
28882897
return AccessStrategy::getMaterializeToTemporary(
@@ -2990,11 +2999,13 @@ bool AbstractStorageDecl::requiresOpaqueAccessor(AccessorKind kind) const {
29902999
case AccessorKind::Set:
29913000
return requiresOpaqueSetter();
29923001
case AccessorKind::Read:
2993-
case AccessorKind::Read2:
29943002
return requiresOpaqueReadCoroutine();
3003+
case AccessorKind::Read2:
3004+
return requiresOpaqueRead2Coroutine();
29953005
case AccessorKind::Modify:
2996-
case AccessorKind::Modify2:
29973006
return requiresOpaqueModifyCoroutine();
3007+
case AccessorKind::Modify2:
3008+
return requiresOpaqueModify2Coroutine();
29983009

29993010
// Other accessors are never part of the opaque-accessors set.
30003011
#define OPAQUE_ACCESSOR(ID, KEYWORD)
@@ -3006,11 +3017,37 @@ bool AbstractStorageDecl::requiresOpaqueAccessor(AccessorKind kind) const {
30063017
llvm_unreachable("bad accessor kind");
30073018
}
30083019

3020+
bool AbstractStorageDecl::requiresOpaqueReadCoroutine() const {
3021+
ASTContext &ctx = getASTContext();
3022+
if (ctx.LangOpts.hasFeature(Feature::CoroutineAccessors))
3023+
return requiresCorrespondingUnderscoredCoroutineAccessor(
3024+
AccessorKind::Read2);
3025+
return getOpaqueReadOwnership() != OpaqueReadOwnership::Owned;
3026+
}
3027+
3028+
bool AbstractStorageDecl::requiresOpaqueRead2Coroutine() const {
3029+
ASTContext &ctx = getASTContext();
3030+
if (!ctx.LangOpts.hasFeature(Feature::CoroutineAccessors))
3031+
return false;
3032+
return getOpaqueReadOwnership() != OpaqueReadOwnership::Owned;
3033+
}
3034+
30093035
bool AbstractStorageDecl::requiresOpaqueModifyCoroutine() const {
30103036
ASTContext &ctx = getASTContext();
3011-
return evaluateOrDefault(ctx.evaluator,
3012-
RequiresOpaqueModifyCoroutineRequest{const_cast<AbstractStorageDecl *>(this)},
3013-
false);
3037+
return evaluateOrDefault(
3038+
ctx.evaluator,
3039+
RequiresOpaqueModifyCoroutineRequest{
3040+
const_cast<AbstractStorageDecl *>(this), /*isUnderscored=*/true},
3041+
false);
3042+
}
3043+
3044+
bool AbstractStorageDecl::requiresOpaqueModify2Coroutine() const {
3045+
ASTContext &ctx = getASTContext();
3046+
return evaluateOrDefault(
3047+
ctx.evaluator,
3048+
RequiresOpaqueModifyCoroutineRequest{
3049+
const_cast<AbstractStorageDecl *>(this), /*isUnderscored=*/false},
3050+
false);
30143051
}
30153052

30163053
AccessorDecl *AbstractStorageDecl::getSynthesizedAccessor(AccessorKind kind) const {
@@ -3109,13 +3146,19 @@ void AbstractStorageDecl::visitExpectedOpaqueAccessors(
31093146
if (requiresOpaqueReadCoroutine())
31103147
visit(AccessorKind::Read);
31113148

3149+
if (requiresOpaqueRead2Coroutine())
3150+
visit(AccessorKind::Read2);
3151+
31123152
// All mutable storage should have a setter.
31133153
if (requiresOpaqueSetter())
31143154
visit(AccessorKind::Set);
31153155

31163156
// Include the modify coroutine if it's required.
31173157
if (requiresOpaqueModifyCoroutine())
31183158
visit(AccessorKind::Modify);
3159+
3160+
if (requiresOpaqueModify2Coroutine())
3161+
visit(AccessorKind::Modify2);
31193162
}
31203163

31213164
void AbstractStorageDecl::visitOpaqueAccessors(

‎lib/AST/TypeCheckRequests.cpp‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -804,15 +804,29 @@ void RequiresOpaqueAccessorsRequest::cacheResult(bool value) const {
804804
std::optional<bool>
805805
RequiresOpaqueModifyCoroutineRequest::getCachedResult() const {
806806
auto *storage = std::get<0>(getStorage());
807-
if (storage->LazySemanticInfo.RequiresOpaqueModifyCoroutineComputed)
808-
return static_cast<bool>(storage->LazySemanticInfo.RequiresOpaqueModifyCoroutine);
807+
auto isUnderscored = std::get<1>(getStorage());
808+
if (isUnderscored) {
809+
if (storage->LazySemanticInfo.RequiresOpaqueModifyCoroutineComputed)
810+
return static_cast<bool>(
811+
storage->LazySemanticInfo.RequiresOpaqueModifyCoroutine);
812+
} else {
813+
if (storage->LazySemanticInfo.RequiresOpaqueModify2CoroutineComputed)
814+
return static_cast<bool>(
815+
storage->LazySemanticInfo.RequiresOpaqueModify2Coroutine);
816+
}
809817
return std::nullopt;
810818
}
811819

812820
void RequiresOpaqueModifyCoroutineRequest::cacheResult(bool value) const {
813821
auto *storage = std::get<0>(getStorage());
814-
storage->LazySemanticInfo.RequiresOpaqueModifyCoroutineComputed = 1;
815-
storage->LazySemanticInfo.RequiresOpaqueModifyCoroutine = value;
822+
auto isUnderscored = std::get<1>(getStorage());
823+
if (isUnderscored) {
824+
storage->LazySemanticInfo.RequiresOpaqueModifyCoroutineComputed = 1;
825+
storage->LazySemanticInfo.RequiresOpaqueModifyCoroutine = value;
826+
} else {
827+
storage->LazySemanticInfo.RequiresOpaqueModify2CoroutineComputed = 1;
828+
storage->LazySemanticInfo.RequiresOpaqueModify2Coroutine = value;
829+
}
816830
}
817831

818832
//----------------------------------------------------------------------------//

‎lib/SILGen/SILGenApply.cpp‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6000,7 +6000,8 @@ SILValue SILGenFunction::emitApplyWithRethrow(SILLocation loc, SILValue fn,
60006000
return normalBB->createPhiArgument(resultType, OwnershipKind::Owned);
60016001
}
60026002

6003-
std::pair<MultipleValueInstructionResult *, CleanupHandle>
6003+
std::tuple<MultipleValueInstructionResult *, CleanupHandle, SILValue,
6004+
CleanupHandle>
60046005
SILGenFunction::emitBeginApplyWithRethrow(SILLocation loc, SILValue fn,
60056006
SILType substFnType,
60066007
SubstitutionMap subs,
@@ -6021,19 +6022,29 @@ SILGenFunction::emitBeginApplyWithRethrow(SILLocation loc, SILValue fn,
60216022

60226023
auto *token = beginApply->getTokenResult();
60236024

6025+
CleanupHandle deallocCleanup;
6026+
auto *allocation = beginApply->getCalleeAllocationResult();
6027+
if (allocation) {
6028+
deallocCleanup = enterDeallocStackCleanup(allocation);
6029+
}
6030+
60246031
Cleanups.pushCleanup<EndCoroutineApply>(token);
60256032
auto abortCleanup = Cleanups.getTopCleanup();
60266033

6027-
return { token, abortCleanup };
6034+
return {token, abortCleanup, allocation, deallocCleanup};
60286035
}
60296036

6030-
void SILGenFunction::emitEndApplyWithRethrow(SILLocation loc,
6031-
MultipleValueInstructionResult *token) {
6037+
void SILGenFunction::emitEndApplyWithRethrow(
6038+
SILLocation loc, MultipleValueInstructionResult *token,
6039+
SILValue allocation) {
60326040
// TODO: adjust this to handle results of TryBeginApplyInst.
60336041
assert(token->isBeginApplyToken());
60346042

60356043
B.createEndApply(loc, token,
60366044
SILType::getEmptyTupleType(getASTContext()));
6045+
if (allocation) {
6046+
B.createDeallocStack(loc, allocation);
6047+
}
60376048
}
60386049

60396050
void SILGenFunction::emitYield(SILLocation loc,

‎lib/SILGen/SILGenFunction.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2185,12 +2185,14 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
21852185
SubstitutionMap subs,
21862186
ArrayRef<SILValue> args);
21872187

2188-
std::pair<MultipleValueInstructionResult *, CleanupHandle>
2188+
std::tuple<MultipleValueInstructionResult *, CleanupHandle, SILValue,
2189+
CleanupHandle>
21892190
emitBeginApplyWithRethrow(SILLocation loc, SILValue fn, SILType substFnType,
21902191
SubstitutionMap subs, ArrayRef<SILValue> args,
21912192
SmallVectorImpl<SILValue> &yields);
21922193
void emitEndApplyWithRethrow(SILLocation loc,
2193-
MultipleValueInstructionResult *token);
2194+
MultipleValueInstructionResult *token,
2195+
SILValue allocation);
21942196

21952197
ManagedValue emitExtractFunctionIsolation(SILLocation loc,
21962198
ArgumentSource &&fnValue);

‎lib/SILGen/SILGenPoly.cpp‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6824,10 +6824,13 @@ SILGenFunction::emitVTableThunk(SILDeclRef base,
68246824
case SILCoroutineKind::YieldOnce:
68256825
case SILCoroutineKind::YieldOnce2: {
68266826
SmallVector<SILValue, 4> derivedYields;
6827-
auto tokenAndCleanup =
6828-
emitBeginApplyWithRethrow(loc, derivedRef,
6829-
SILType::getPrimitiveObjectType(derivedFTy),
6830-
subs, args, derivedYields);
6827+
auto tokenAndCleanups = emitBeginApplyWithRethrow(
6828+
loc, derivedRef, SILType::getPrimitiveObjectType(derivedFTy), subs,
6829+
args, derivedYields);
6830+
auto token = std::get<0>(tokenAndCleanups);
6831+
auto abortCleanup = std::get<1>(tokenAndCleanups);
6832+
auto allocation = std::get<2>(tokenAndCleanups);
6833+
auto deallocCleanup = std::get<3>(tokenAndCleanups);
68316834
auto overrideSubs = SubstitutionMap::getOverrideSubstitutions(
68326835
base.getDecl(), derived.getDecl()).subst(subs);
68336836

@@ -6837,10 +6840,13 @@ SILGenFunction::emitVTableThunk(SILDeclRef base,
68376840
translateYields(*this, loc, derivedYields, derivedYieldInfo, baseYieldInfo);
68386841

68396842
// Kill the abort cleanup without emitting it.
6840-
Cleanups.setCleanupState(tokenAndCleanup.second, CleanupState::Dead);
6843+
Cleanups.setCleanupState(abortCleanup, CleanupState::Dead);
6844+
if (allocation) {
6845+
Cleanups.setCleanupState(deallocCleanup, CleanupState::Dead);
6846+
}
68416847

68426848
// End the inner coroutine normally.
6843-
emitEndApplyWithRethrow(loc, tokenAndCleanup.first);
6849+
emitEndApplyWithRethrow(loc, token, allocation);
68446850

68456851
result = B.createTuple(loc, {});
68466852
break;
@@ -7213,9 +7219,12 @@ void SILGenFunction::emitProtocolWitness(
72137219
case SILCoroutineKind::YieldOnce:
72147220
case SILCoroutineKind::YieldOnce2: {
72157221
SmallVector<SILValue, 4> witnessYields;
7216-
auto tokenAndCleanup =
7217-
emitBeginApplyWithRethrow(loc, witnessFnRef, witnessSILTy, witnessSubs,
7218-
args, witnessYields);
7222+
auto tokenAndCleanups = emitBeginApplyWithRethrow(
7223+
loc, witnessFnRef, witnessSILTy, witnessSubs, args, witnessYields);
7224+
auto token = std::get<0>(tokenAndCleanups);
7225+
auto abortCleanup = std::get<1>(tokenAndCleanups);
7226+
auto allocation = std::get<2>(tokenAndCleanups);
7227+
auto deallocCleanup = std::get<3>(tokenAndCleanups);
72197228

72207229
YieldInfo witnessYieldInfo(SGM, witness, witnessFTy, witnessSubs);
72217230
YieldInfo reqtYieldInfo(SGM, requirement, thunkTy,
@@ -7224,10 +7233,13 @@ void SILGenFunction::emitProtocolWitness(
72247233
translateYields(*this, loc, witnessYields, witnessYieldInfo, reqtYieldInfo);
72257234

72267235
// Kill the abort cleanup without emitting it.
7227-
Cleanups.setCleanupState(tokenAndCleanup.second, CleanupState::Dead);
7236+
Cleanups.setCleanupState(abortCleanup, CleanupState::Dead);
7237+
if (allocation) {
7238+
Cleanups.setCleanupState(deallocCleanup, CleanupState::Dead);
7239+
}
72287240

72297241
// End the inner coroutine normally.
7230-
emitEndApplyWithRethrow(loc, tokenAndCleanup.first);
7242+
emitEndApplyWithRethrow(loc, token, allocation);
72317243

72327244
reqtResultValue = B.createTuple(loc, {});
72337245
break;

0 commit comments

Comments
 (0)