Skip to content

Commit 7697a49

Browse files
nate-chandlernate-chandler
committed
[CoroutineAccessors] Only reference when available
Don't bind references to storage to use (new ABI) coroutine accessors unless they're guaranteed to be available. For example, when building against a resilient module that has coroutine accessors, they can only be used if the deployment target is >= the version of Swift that includes the feature. rdar://148783895
1 parent 347689f commit 7697a49

File tree

10 files changed

+487
-41
lines changed

10 files changed

+487
-41
lines changed

‎include/swift/AST/Decl.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6239,10 +6239,11 @@ class AbstractStorageDecl : public ValueDecl {
62396239
bool forConformance=false) const;
62406240

62416241
/// Determine how this storage declaration should actually be accessed.
6242-
AccessStrategy getAccessStrategy(AccessSemantics semantics,
6243-
AccessKind accessKind, ModuleDecl *module,
6244-
ResilienceExpansion expansion,
6245-
bool useOldABI) const;
6242+
AccessStrategy getAccessStrategy(
6243+
AccessSemantics semantics, AccessKind accessKind, ModuleDecl *module,
6244+
ResilienceExpansion expansion,
6245+
std::optional<std::pair<SourceRange, const DeclContext *>> location,
6246+
bool useOldABI) const;
62466247

62476248
/// Whether access is via physical storage.
62486249
bool isAccessedViaPhysicalStorage(AccessSemantics semantics,

‎lib/AST/Decl.cpp

Lines changed: 87 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
//===--- Decl.cpp - Swift Language Decl ASTs ------------------------------===//
32
//
43
// This source file is part of the Swift.org open source project
@@ -15,7 +14,6 @@
1514
//
1615
//===----------------------------------------------------------------------===//
1716

18-
#include "swift/Strings.h"
1917
#include "swift/AST/Decl.h"
2018
#include "swift/AST/ASTContext.h"
2119
#include "swift/AST/ASTMangler.h"
@@ -24,6 +22,7 @@
2422
#include "swift/AST/AccessRequests.h"
2523
#include "swift/AST/AccessScope.h"
2624
#include "swift/AST/Attr.h"
25+
#include "swift/AST/AvailabilityContext.h"
2726
#include "swift/AST/AvailabilityInference.h"
2827
#include "swift/AST/CaptureInfo.h"
2928
#include "swift/AST/ConformanceLookup.h"
@@ -64,6 +63,7 @@
6463
#include "swift/ClangImporter/ClangModule.h"
6564
#include "swift/Demangling/ManglingMacros.h"
6665
#include "swift/Parse/Lexer.h" // FIXME: Bad dependency
66+
#include "swift/Strings.h"
6767
#include "clang/Lex/MacroInfo.h"
6868
#include "llvm/ADT/DenseMap.h"
6969
#include "llvm/ADT/SmallPtrSet.h"
@@ -3026,9 +3026,11 @@ getDirectWriteAccessStrategy(const AbstractStorageDecl *storage) {
30263026
llvm_unreachable("bad impl kind");
30273027
}
30283028

3029-
static AccessStrategy
3030-
getOpaqueReadAccessStrategy(const AbstractStorageDecl *storage, bool dispatch,
3031-
bool useOldABI);
3029+
static AccessStrategy getOpaqueReadAccessStrategy(
3030+
const AbstractStorageDecl *storage, bool dispatch, ModuleDecl *module,
3031+
ResilienceExpansion expansion,
3032+
std::optional<std::pair<SourceRange, const DeclContext *>> location,
3033+
bool useOldABI);
30323034
static AccessStrategy
30333035
getOpaqueWriteAccessStrategy(const AbstractStorageDecl *storage, bool dispatch);
30343036

@@ -3043,7 +3045,9 @@ getDirectReadWriteAccessStrategy(const AbstractStorageDecl *storage) {
30433045
// If the storage isDynamic (and not @objc) use the accessors.
30443046
if (storage->shouldUseNativeDynamicDispatch())
30453047
return AccessStrategy::getMaterializeToTemporary(
3046-
getOpaqueReadAccessStrategy(storage, false, false),
3048+
getOpaqueReadAccessStrategy(storage, false, nullptr,
3049+
ResilienceExpansion::Minimal,
3050+
std::nullopt, false),
30473051
getOpaqueWriteAccessStrategy(storage, false));
30483052
return AccessStrategy::getStorage();
30493053
}
@@ -3080,15 +3084,61 @@ getDirectReadWriteAccessStrategy(const AbstractStorageDecl *storage) {
30803084
llvm_unreachable("bad impl kind");
30813085
}
30823086

3083-
static AccessStrategy
3084-
getOpaqueReadAccessStrategy(const AbstractStorageDecl *storage, bool dispatch,
3085-
bool useOldABI) {
3087+
static bool mayReferenceUseCoroutineAccessorOnStorage(
3088+
ModuleDecl *module, ResilienceExpansion expansion,
3089+
std::optional<std::pair<SourceRange, const DeclContext *>> reference,
3090+
const AbstractStorageDecl *storage) {
3091+
assert(storage);
3092+
ASTContext &ctx = storage->getASTContext();
3093+
assert(ctx.LangOpts.hasFeature(Feature::CoroutineAccessors));
3094+
3095+
// For triples without platforms, coroutine accessors are always available.
3096+
auto domain = ctx.getTargetAvailabilityDomain();
3097+
if (domain.isUniversal())
3098+
return true;
3099+
3100+
// A non-resilient access to storage can always use the coroutine accessor,
3101+
// provided it exists. Such an access is compiled with the version of the
3102+
// module that includes the accessor.
3103+
bool resilient = [&] {
3104+
if (module)
3105+
return storage->isResilient(module, expansion);
3106+
else
3107+
return storage->isResilient();
3108+
}();
3109+
if (!resilient)
3110+
return true;
3111+
3112+
// Without knowing where the storage is referenced, it can't be known that
3113+
// a coroutine accessor is available.
3114+
if (!reference) {
3115+
return false;
3116+
}
3117+
3118+
// A resilient access to storage may only use a coroutine accessor if the
3119+
// storage became available no earlier than the feature.
3120+
auto referenceAvailability = AvailabilityContext::forLocation(
3121+
reference->first.Start, reference->second)
3122+
.getPlatformRange();
3123+
auto featureAvailability =
3124+
storage->getASTContext().getCoroutineAccessorsAvailability();
3125+
3126+
return referenceAvailability.isContainedIn(featureAvailability);
3127+
}
3128+
3129+
static AccessStrategy getOpaqueReadAccessStrategy(
3130+
const AbstractStorageDecl *storage, bool dispatch, ModuleDecl *module,
3131+
ResilienceExpansion expansion,
3132+
std::optional<std::pair<SourceRange, const DeclContext *>> location,
3133+
bool useOldABI) {
30863134
if (useOldABI) {
30873135
assert(storage->requiresOpaqueRead2Coroutine());
30883136
assert(storage->requiresOpaqueReadCoroutine());
30893137
return AccessStrategy::getAccessor(AccessorKind::Read, dispatch);
30903138
}
3091-
if (storage->requiresOpaqueRead2Coroutine())
3139+
if (storage->requiresOpaqueRead2Coroutine() &&
3140+
mayReferenceUseCoroutineAccessorOnStorage(module, expansion, location,
3141+
storage))
30923142
return AccessStrategy::getAccessor(AccessorKind::Read2, dispatch);
30933143
if (storage->requiresOpaqueReadCoroutine())
30943144
return AccessStrategy::getAccessor(AccessorKind::Read, dispatch);
@@ -3102,41 +3152,53 @@ getOpaqueWriteAccessStrategy(const AbstractStorageDecl *storage, bool dispatch)
31023152
return AccessStrategy::getAccessor(AccessorKind::Set, dispatch);
31033153
}
31043154

3105-
static AccessStrategy
3106-
getOpaqueReadWriteAccessStrategy(const AbstractStorageDecl *storage,
3107-
bool dispatch, bool useOldABI) {
3155+
static AccessStrategy getOpaqueReadWriteAccessStrategy(
3156+
const AbstractStorageDecl *storage, bool dispatch, ModuleDecl *module,
3157+
ResilienceExpansion expansion,
3158+
std::optional<std::pair<SourceRange, const DeclContext *>> location,
3159+
bool useOldABI) {
31083160
if (useOldABI) {
31093161
assert(storage->requiresOpaqueModify2Coroutine());
31103162
assert(storage->requiresOpaqueModifyCoroutine());
31113163
return AccessStrategy::getAccessor(AccessorKind::Modify, dispatch);
31123164
}
3113-
if (storage->requiresOpaqueModify2Coroutine())
3165+
if (storage->requiresOpaqueModify2Coroutine() &&
3166+
mayReferenceUseCoroutineAccessorOnStorage(module, expansion, location,
3167+
storage))
31143168
return AccessStrategy::getAccessor(AccessorKind::Modify2, dispatch);
31153169
if (storage->requiresOpaqueModifyCoroutine())
31163170
return AccessStrategy::getAccessor(AccessorKind::Modify, dispatch);
31173171
return AccessStrategy::getMaterializeToTemporary(
3118-
getOpaqueReadAccessStrategy(storage, dispatch, false),
3172+
getOpaqueReadAccessStrategy(storage, dispatch, nullptr,
3173+
ResilienceExpansion::Minimal, location,
3174+
false),
31193175
getOpaqueWriteAccessStrategy(storage, dispatch));
31203176
}
31213177

3122-
static AccessStrategy
3123-
getOpaqueAccessStrategy(const AbstractStorageDecl *storage,
3124-
AccessKind accessKind, bool dispatch, bool useOldABI) {
3178+
static AccessStrategy getOpaqueAccessStrategy(
3179+
const AbstractStorageDecl *storage, AccessKind accessKind, bool dispatch,
3180+
ModuleDecl *module, ResilienceExpansion expansion,
3181+
std::optional<std::pair<SourceRange, const DeclContext *>> location,
3182+
bool useOldABI) {
31253183
switch (accessKind) {
31263184
case AccessKind::Read:
3127-
return getOpaqueReadAccessStrategy(storage, dispatch, useOldABI);
3185+
return getOpaqueReadAccessStrategy(storage, dispatch, module, expansion,
3186+
location, useOldABI);
31283187
case AccessKind::Write:
31293188
assert(!useOldABI);
31303189
return getOpaqueWriteAccessStrategy(storage, dispatch);
31313190
case AccessKind::ReadWrite:
3132-
return getOpaqueReadWriteAccessStrategy(storage, dispatch, useOldABI);
3191+
return getOpaqueReadWriteAccessStrategy(storage, dispatch, module,
3192+
expansion, location, useOldABI);
31333193
}
31343194
llvm_unreachable("bad access kind");
31353195
}
31363196

31373197
AccessStrategy AbstractStorageDecl::getAccessStrategy(
31383198
AccessSemantics semantics, AccessKind accessKind, ModuleDecl *module,
3139-
ResilienceExpansion expansion, bool useOldABI) const {
3199+
ResilienceExpansion expansion,
3200+
std::optional<std::pair<SourceRange, const DeclContext *>> location,
3201+
bool useOldABI) const {
31403202
switch (semantics) {
31413203
case AccessSemantics::DirectToStorage:
31423204
assert(hasStorage() || getASTContext().Diags.hadAnyError());
@@ -3153,11 +3215,11 @@ AccessStrategy AbstractStorageDecl::getAccessStrategy(
31533215
// accessors are dynamically dispatched, and we cannot do direct access.
31543216
if (isPolymorphic(this))
31553217
return getOpaqueAccessStrategy(this, accessKind, /*dispatch*/ true,
3156-
useOldABI);
3218+
module, expansion, location, useOldABI);
31573219

31583220
if (shouldUseNativeDynamicDispatch())
31593221
return getOpaqueAccessStrategy(this, accessKind, /*dispatch*/ false,
3160-
useOldABI);
3222+
module, expansion, location, useOldABI);
31613223

31623224
// If the storage is resilient from the given module and resilience
31633225
// expansion, we cannot use direct access.
@@ -3180,7 +3242,7 @@ AccessStrategy AbstractStorageDecl::getAccessStrategy(
31803242

31813243
if (resilient)
31823244
return getOpaqueAccessStrategy(this, accessKind, /*dispatch*/ false,
3183-
useOldABI);
3245+
module, expansion, location, useOldABI);
31843246
}
31853247

31863248
LLVM_FALLTHROUGH;
@@ -3204,6 +3266,7 @@ bool AbstractStorageDecl::isAccessedViaPhysicalStorage(
32043266
AccessSemantics semantics, AccessKind accessKind, ModuleDecl *module,
32053267
ResilienceExpansion expansion) const {
32063268
return getAccessStrategy(semantics, accessKind, module, expansion,
3269+
/*location=*/std::nullopt,
32073270
/*useOldABI=*/false)
32083271
.getKind() == AccessStrategy::Kind::Storage;
32093272
}

‎lib/SILGen/SILGen.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1963,7 +1963,7 @@ SILGenModule::canStorageUseStoredKeyPathComponent(AbstractStorageDecl *decl,
19631963
auto strategy = decl->getAccessStrategy(
19641964
AccessSemantics::Ordinary,
19651965
decl->supportsMutation() ? AccessKind::ReadWrite : AccessKind::Read,
1966-
M.getSwiftModule(), expansion,
1966+
M.getSwiftModule(), expansion, std::nullopt,
19671967
/*useOldABI=*/false);
19681968
switch (strategy.getKind()) {
19691969
case AccessStrategy::Storage: {

‎lib/SILGen/SILGenApply.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3334,12 +3334,15 @@ Expr *SILGenFunction::findStorageReferenceExprForMoveOnly(Expr *argExpr,
33343334

33353335
if (!storage)
33363336
return nullptr;
3337+
33373338
// This should match the strategy computation used in
33383339
// SILGenLValue::visit{DeclRef,Member}RefExpr.
3339-
auto strategy = storage->getAccessStrategy(accessSemantics,
3340-
AccessKind::Read, SGM.M.getSwiftModule(), F.getResilienceExpansion(),
3341-
/* old abi*/ false);
3342-
3340+
auto strategy = storage->getAccessStrategy(
3341+
accessSemantics, AccessKind::Read, SGM.M.getSwiftModule(),
3342+
F.getResilienceExpansion(),
3343+
std::make_pair<>(argExpr->getSourceRange(), FunctionDC),
3344+
/* old abi*/ false);
3345+
33433346
switch (strategy.getKind()) {
33443347
case AccessStrategy::Storage:
33453348
// Storage can benefit from direct borrowing/consumption.

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3815,6 +3815,7 @@ static SILFunction *getOrCreateKeyPathSetter(
38153815
auto semantics = AccessSemantics::Ordinary;
38163816
auto strategy = property->getAccessStrategy(semantics, AccessKind::Write,
38173817
SGM.M.getSwiftModule(), expansion,
3818+
std::nullopt,
38183819
/*useOldABI=*/false);
38193820

38203821
LValueOptions lvOptions;
@@ -4633,7 +4634,7 @@ KeyPathPatternComponent SILGenModule::emitKeyPathComponentForDecl(
46334634
auto strategy = storage->getAccessStrategy(
46344635
AccessSemantics::Ordinary,
46354636
storage->supportsMutation() ? AccessKind::ReadWrite : AccessKind::Read,
4636-
M.getSwiftModule(), expansion,
4637+
M.getSwiftModule(), expansion, std::nullopt,
46374638
/*useOldABI=*/false);
46384639

46394640
AbstractStorageDecl *externalDecl = nullptr;

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3115,10 +3115,12 @@ class LLVM_LIBRARY_VISIBILITY SILGenBorrowedBaseVisitor
31153115

31163116
assert(!e->getType()->is<LValueType>());
31173117

3118+
auto pair = std::make_pair<>(e->getSourceRange(), SGF.FunctionDC);
3119+
31183120
auto accessSemantics = e->getAccessSemantics();
31193121
AccessStrategy strategy = var->getAccessStrategy(
31203122
accessSemantics, getFormalAccessKind(accessKind),
3121-
SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(),
3123+
SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(), pair,
31223124
/*useOldABI=*/false);
31233125

31243126
auto baseFormalType = getBaseFormalType(e->getBase());
@@ -3396,7 +3398,7 @@ static LValue emitLValueForNonMemberVarDecl(
33963398
auto access = getFormalAccessKind(accessKind);
33973399
auto strategy = var->getAccessStrategy(
33983400
semantics, access, SGF.SGM.M.getSwiftModule(),
3399-
SGF.F.getResilienceExpansion(), /*useOldABI=*/false);
3401+
SGF.F.getResilienceExpansion(), std::nullopt, /*useOldABI=*/false);
34003402

34013403
lv.addNonMemberVarComponent(SGF, loc, var, subs, options, accessKind,
34023404
strategy, formalRValueType, actorIso);
@@ -4032,6 +4034,7 @@ LValue SILGenLValue::visitMemberRefExpr(MemberRefExpr *e,
40324034
AccessStrategy strategy = var->getAccessStrategy(
40334035
accessSemantics, getFormalAccessKind(accessKind),
40344036
SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(),
4037+
std::make_pair<>(e->getSourceRange(), SGF.FunctionDC),
40354038
/*useOldABI=*/isSynthesizedDefaultImplementionThunk(SGF));
40364039

40374040
bool isOnSelfParameter = isCallToSelfOfCurrentFunction(SGF, e);
@@ -4053,6 +4056,7 @@ LValue SILGenLValue::visitMemberRefExpr(MemberRefExpr *e,
40534056
strategy = var->getAccessStrategy(
40544057
accessSemantics, getFormalAccessKind(accessKind),
40554058
SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(),
4059+
std::make_pair<>(e->getSourceRange(), SGF.FunctionDC),
40564060
/*useOldABI=*/false);
40574061
}
40584062
}
@@ -4254,6 +4258,7 @@ LValue SILGenLValue::visitSubscriptExpr(SubscriptExpr *e,
42544258
auto strategy = decl->getAccessStrategy(
42554259
accessSemantics, getFormalAccessKind(accessKind),
42564260
SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(),
4261+
std::make_pair<>(e->getSourceRange(), SGF.FunctionDC),
42574262
/*useOldABI=*/isSynthesizedDefaultImplementionThunk(SGF));
42584263

42594264
bool isOnSelfParameter = isCallToSelfOfCurrentFunction(SGF, e);
@@ -4274,6 +4279,7 @@ LValue SILGenLValue::visitSubscriptExpr(SubscriptExpr *e,
42744279
strategy = decl->getAccessStrategy(
42754280
accessSemantics, getFormalAccessKind(accessKind),
42764281
SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(),
4282+
std::make_pair<>(e->getSourceRange(), SGF.FunctionDC),
42774283
/*useOldABI=*/false);
42784284
}
42794285
}
@@ -4744,7 +4750,7 @@ LValue SILGenFunction::emitPropertyLValue(SILLocation loc, ManagedValue base,
47444750

47454751
AccessStrategy strategy = ivar->getAccessStrategy(
47464752
semantics, getFormalAccessKind(accessKind), SGM.M.getSwiftModule(),
4747-
F.getResilienceExpansion(), /*useOldABI=*/false);
4753+
F.getResilienceExpansion(), std::nullopt, /*useOldABI=*/false);
47484754

47494755
auto baseAccessKind =
47504756
getBaseAccessKind(SGM, ivar, accessKind, strategy, baseFormalType,
@@ -5460,7 +5466,7 @@ RValue SILGenFunction::emitRValueForStorageLoad(
54605466
bool isBaseGuaranteed) {
54615467
AccessStrategy strategy = storage->getAccessStrategy(
54625468
semantics, AccessKind::Read, SGM.M.getSwiftModule(),
5463-
F.getResilienceExpansion(), /*useOldABI=*/false);
5469+
F.getResilienceExpansion(), std::nullopt, /*useOldABI=*/false);
54645470

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

‎lib/SILGen/SILGenPattern.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3323,11 +3323,13 @@ static bool isBorrowableSubject(SILGenFunction &SGF,
33233323
if (!storage) {
33243324
return false;
33253325
}
3326-
3326+
3327+
auto pair = std::make_pair<>(subjectExpr->getSourceRange(), SGF.FunctionDC);
3328+
33273329
// Check the access strategy used to read the storage.
33283330
auto strategy =
33293331
storage->getAccessStrategy(access, AccessKind::Read, SGF.SGM.SwiftModule,
3330-
SGF.F.getResilienceExpansion(),
3332+
SGF.F.getResilienceExpansion(), pair,
33313333
/*useOldABI=*/false);
33323334

33333335
switch (strategy.getKind()) {

‎lib/Sema/TypeCheckStorage.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2700,7 +2700,7 @@ bool AbstractStorageDecl::requiresCorrespondingUnderscoredCoroutineAccessor(
27002700
return true;
27012701

27022702
auto modifyAvailability = AvailabilityContext::forLocation({}, accessor);
2703-
auto featureAvailability = ctx.getCoroutineAccessorsRuntimeAvailability();
2703+
auto featureAvailability = ctx.getCoroutineAccessorsAvailability();
27042704
// If accessor was introduced only after the feature was, there's no old ABI
27052705
// to maintain.
27062706
if (modifyAvailability.getPlatformRange().isContainedIn(featureAvailability))

0 commit comments

Comments
 (0)