Skip to content

Commit 3e637b0

Browse files
slavapestovslavapestov
authored
Merge pull request swiftlang#73129 from slavapestov/local-function-isolation-change
Local functions inherit isolation from context instead of captures
2 parents 7ef1701 + 05bcc25 commit 3e637b0

File tree

10 files changed

+122
-72
lines changed

10 files changed

+122
-72
lines changed

‎include/swift/AST/AnyFunctionRef.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,6 @@ class AnyFunctionRef {
8383
TheFunction.get<AbstractClosureExpr *>()->setCaptureInfo(captures);
8484
}
8585

86-
void getLocalCaptures(SmallVectorImpl<CapturedValue> &Result) const {
87-
getCaptureInfo().getLocalCaptures(Result);
88-
}
89-
9086
bool hasType() const {
9187
if (auto *AFD = TheFunction.dyn_cast<AbstractFunctionDecl *>())
9288
return AFD->hasInterfaceType();

‎include/swift/AST/CaptureInfo.h

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -197,15 +197,6 @@ class CaptureInfo {
197197
return StorageAndFlags.getPointer()->getCaptures();
198198
}
199199

200-
/// Return a filtered list of the captures for this function,
201-
/// filtering out global variables. This function returns the list that
202-
/// actually needs to be closed over.
203-
///
204-
void getLocalCaptures(SmallVectorImpl<CapturedValue> &Result) const;
205-
206-
/// \returns true if getLocalCaptures() will return a non-empty list.
207-
bool hasLocalCaptures() const;
208-
209200
/// \returns true if the function captures any generic type parameters.
210201
bool hasGenericParamCaptures() const {
211202
// FIXME: Ideally, everywhere that synthesizes a function should include

‎include/swift/AST/Decl.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8033,10 +8033,6 @@ class FuncDecl : public AbstractFunctionDecl {
80338033
/// prior to type checking.
80348034
bool isBinaryOperator() const;
80358035

8036-
void getLocalCaptures(SmallVectorImpl<CapturedValue> &Result) const {
8037-
return getCaptureInfo().getLocalCaptures(Result);
8038-
}
8039-
80408036
ParamDecl **getImplicitSelfDeclStorage();
80418037

80428038
/// Get the supertype method this method overrides, if any.

‎lib/AST/CaptureInfo.cpp

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -59,34 +59,7 @@ CaptureInfo CaptureInfo::empty() {
5959
return result;
6060
}
6161

62-
bool CaptureInfo::hasLocalCaptures() const {
63-
for (auto capture : getCaptures()) {
64-
if (capture.isLocalCapture())
65-
return true;
66-
}
67-
return false;
68-
}
69-
70-
71-
void CaptureInfo::
72-
getLocalCaptures(SmallVectorImpl<CapturedValue> &Result) const {
73-
if (!hasLocalCaptures()) return;
74-
75-
Result.reserve(getCaptures().size());
76-
77-
// Filter out global variables.
78-
for (auto capture : getCaptures()) {
79-
if (!capture.isLocalCapture())
80-
continue;
81-
82-
Result.push_back(capture);
83-
}
84-
}
85-
8662
VarDecl *CaptureInfo::getIsolatedParamCapture() const {
87-
if (!hasLocalCaptures())
88-
return nullptr;
89-
9063
for (const auto &capture : getCaptures()) {
9164
// NOTE: isLocalCapture() returns false if we have dynamic self metadata
9265
// since dynamic self metadata is never an isolated capture. So we can just

‎lib/SIL/IR/TypeLowering.cpp

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4154,11 +4154,21 @@ TypeConverter::getLoweredLocalCaptures(SILDeclRef fn) {
41544154
DynamicSelfType *capturesDynamicSelf = nullptr;
41554155
OpaqueValueExpr *capturesOpaqueValue = nullptr;
41564156

4157-
std::function<void (CaptureInfo captureInfo, DeclContext *dc)> collectCaptures;
4157+
std::function<void (CaptureInfo captureInfo)> collectCaptures;
41584158
std::function<void (AnyFunctionRef)> collectFunctionCaptures;
41594159
std::function<void (SILDeclRef)> collectConstantCaptures;
41604160

4161-
collectCaptures = [&](CaptureInfo captureInfo, DeclContext *dc) {
4161+
auto recordCapture = [&](CapturedValue capture) {
4162+
ValueDecl *value = capture.getDecl();
4163+
auto existing = captures.find(value);
4164+
if (existing != captures.end()) {
4165+
existing->second = existing->second.mergeFlags(capture);
4166+
} else {
4167+
captures.insert(std::pair<ValueDecl *, CapturedValue>(value, capture));
4168+
}
4169+
};
4170+
4171+
collectCaptures = [&](CaptureInfo captureInfo) {
41624172
assert(captureInfo.hasBeenComputed());
41634173

41644174
if (captureInfo.hasGenericParamCaptures())
@@ -4168,9 +4178,10 @@ TypeConverter::getLoweredLocalCaptures(SILDeclRef fn) {
41684178
if (captureInfo.hasOpaqueValueCapture())
41694179
capturesOpaqueValue = captureInfo.getOpaqueValue();
41704180

4171-
SmallVector<CapturedValue, 4> localCaptures;
4172-
captureInfo.getLocalCaptures(localCaptures);
4173-
for (auto capture : localCaptures) {
4181+
for (auto capture : captureInfo.getCaptures()) {
4182+
if (!capture.isLocalCapture())
4183+
continue;
4184+
41744185
// If the capture is of another local function, grab its transitive
41754186
// captures instead.
41764187
if (auto capturedFn = getAnyFunctionRefFromCapture(capture)) {
@@ -4302,13 +4313,7 @@ TypeConverter::getLoweredLocalCaptures(SILDeclRef fn) {
43024313
}
43034314

43044315
// Collect non-function captures.
4305-
ValueDecl *value = capture.getDecl();
4306-
auto existing = captures.find(value);
4307-
if (existing != captures.end()) {
4308-
existing->second = existing->second.mergeFlags(capture);
4309-
} else {
4310-
captures.insert(std::pair<ValueDecl *, CapturedValue>(value, capture));
4311-
}
4316+
recordCapture(capture);
43124317
}
43134318
};
43144319

@@ -4320,8 +4325,21 @@ TypeConverter::getLoweredLocalCaptures(SILDeclRef fn) {
43204325
return;
43214326

43224327
PrettyStackTraceAnyFunctionRef("lowering local captures", curFn);
4323-
auto dc = curFn.getAsDeclContext();
4324-
collectCaptures(curFn.getCaptureInfo(), dc);
4328+
collectCaptures(curFn.getCaptureInfo());
4329+
4330+
if (auto *afd = curFn.getAbstractFunctionDecl()) {
4331+
// If a local function inherits isolation from the enclosing context,
4332+
// make sure we capture the isolated parameter, if we haven't already.
4333+
if (afd->isLocalCapture()) {
4334+
auto actorIsolation = getActorIsolation(afd);
4335+
if (actorIsolation.getKind() == ActorIsolation::ActorInstance) {
4336+
if (auto *var = actorIsolation.getActorInstance()) {
4337+
assert(isa<ParamDecl>(var));
4338+
recordCapture(CapturedValue(var, 0, afd->getLoc()));
4339+
}
4340+
}
4341+
}
4342+
}
43254343

43264344
// A function's captures also include its default arguments, because
43274345
// when we reference a function we don't track which default arguments
@@ -4332,7 +4350,7 @@ TypeConverter::getLoweredLocalCaptures(SILDeclRef fn) {
43324350
if (auto *AFD = curFn.getAbstractFunctionDecl()) {
43334351
for (auto *P : *AFD->getParameters()) {
43344352
if (P->hasDefaultExpr())
4335-
collectCaptures(P->getDefaultArgumentCaptureInfo(), dc);
4353+
collectCaptures(P->getDefaultArgumentCaptureInfo());
43364354
}
43374355
}
43384356
};
@@ -4345,10 +4363,8 @@ TypeConverter::getLoweredLocalCaptures(SILDeclRef fn) {
43454363
if (auto *afd = dyn_cast<AbstractFunctionDecl>(curFn.getDecl())) {
43464364
auto *param = getParameterAt(static_cast<ValueDecl *>(afd),
43474365
curFn.defaultArgIndex);
4348-
if (param->hasDefaultExpr()) {
4349-
auto dc = afd->getInnermostDeclContext();
4350-
collectCaptures(param->getDefaultArgumentCaptureInfo(), dc);
4351-
}
4366+
if (param->hasDefaultExpr())
4367+
collectCaptures(param->getDefaultArgumentCaptureInfo());
43524368
return;
43534369
}
43544370

‎lib/SILGen/SILGenConcurrency.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ void SILGenFunction::emitExpectedExecutor() {
107107
// completely.
108108
if (F.isAsync() ||
109109
(wantDataRaceChecks && funcDecl->isLocalCapture())) {
110-
if (auto isolatedParam = funcDecl->getCaptureInfo()
111-
.getIsolatedParamCapture()) {
110+
auto loweredCaptures = SGM.Types.getLoweredLocalCaptures(SILDeclRef(funcDecl));
111+
if (auto isolatedParam = loweredCaptures.getIsolatedParamCapture()) {
112112
loadExpectedExecutorForLocalVar(isolatedParam);
113113
} else {
114114
auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());

‎lib/SILOptimizer/Differentiation/Common.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,6 @@ findMinimalDerivativeConfiguration(AbstractFunctionDecl *original,
475475
original->getInterfaceType()->castTo<AnyFunctionType>());
476476

477477
if (silParameterIndices->getCapacity() < parameterIndices->getCapacity()) {
478-
assert(original->getCaptureInfo().hasLocalCaptures());
479478
silParameterIndices =
480479
silParameterIndices->extendingCapacity(original->getASTContext(),
481480
parameterIndices->getCapacity());

‎lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2478,9 +2478,9 @@ namespace {
24782478

24792479
/// Check closure captures for Sendable violations.
24802480
void checkLocalCaptures(AnyFunctionRef localFunc) {
2481-
SmallVector<CapturedValue, 2> captures;
2482-
localFunc.getCaptureInfo().getLocalCaptures(captures);
2483-
for (const auto &capture : captures) {
2481+
for (const auto &capture : localFunc.getCaptureInfo().getCaptures()) {
2482+
if (!capture.isLocalCapture())
2483+
continue;
24842484
if (capture.isDynamicSelfMetadata())
24852485
continue;
24862486
if (capture.isOpaqueValue())
@@ -5174,9 +5174,7 @@ ActorIsolation ActorIsolationRequest::evaluate(
51745174
llvm_unreachable("context cannot have erased isolation");
51755175

51765176
case ActorIsolation::ActorInstance:
5177-
if (auto param = func->getCaptureInfo().getIsolatedParamCapture())
5178-
return inferredIsolation(enclosingIsolation);
5179-
break;
5177+
return inferredIsolation(enclosingIsolation);
51805178

51815179
case ActorIsolation::GlobalActor:
51825180
return inferredIsolation(enclosingIsolation);

‎test/Concurrency/actor_isolation.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,8 @@ func checkLocalFunctions() async {
755755
print(k)
756756
}
757757

758+
func callee(_: () -> ()) {}
759+
758760
@available(SwiftStdlib 5.1, *)
759761
actor LocalFunctionIsolatedActor {
760762
func a() -> Bool { // expected-note{{calls to instance method 'a()' from outside of its actor context are implicitly asynchronous}}
@@ -774,6 +776,30 @@ actor LocalFunctionIsolatedActor {
774776
}
775777
return c()
776778
}
779+
780+
func hasRecursiveLocalFunction() {
781+
func recursiveLocalFunction(n: Int) {
782+
_ = a()
783+
callee { _ = a() }
784+
if n > 0 { recursiveLocalFunction(n: n - 1) }
785+
}
786+
787+
recursiveLocalFunction(n: 10)
788+
}
789+
790+
func hasRecursiveLocalFunctions() {
791+
recursiveLocalFunction()
792+
793+
func recursiveLocalFunction() {
794+
anotherRecursiveLocalFunction()
795+
}
796+
797+
func anotherRecursiveLocalFunction() {
798+
callee { _ = a() }
799+
_ = a()
800+
}
801+
}
802+
777803
}
778804

779805
// ----------------------------------------------------------------------

‎test/SILGen/local_function_isolation.swift

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// RUN: %target-swift-frontend -emit-silgen %s -disable-availability-checking | %FileCheck %s
2+
3+
// REQUIRES: concurrency
4+
5+
class NotSendable {}
6+
7+
func callee(_ ns: NotSendable) {}
8+
9+
actor MyActor {
10+
func isolatedToSelf(ns: NotSendable) {
11+
// CHECK-LABEL: sil private [ossa] @$s24local_function_isolation7MyActorC14isolatedToSelf2nsyAA11NotSendableC_tF08implicitH7CaptureL_yyYaF : $@convention(thin) @async (@guaranteed NotSendable, @sil_isolated @guaranteed MyActor) -> () {
12+
func implicitSelfCapture() async {
13+
14+
// CHECK: [[COPY:%.*]] = copy_value %1 : $MyActor
15+
// CHECK-NEXT: [[BORROW:%.*]] = begin_borrow [[COPY]] : $MyActor
16+
// CHECK-NEXT: hop_to_executor [[BORROW]] : $MyActor
17+
18+
// CHECK: [[FN:%.*]] = function_ref @$s24local_function_isolation4testyyYaF : $@convention(thin) @async () -> ()
19+
// CHECK-NEXT: apply [[FN]]() : $@convention(thin) @async () -> ()
20+
await test()
21+
22+
// CHECK: hop_to_executor [[BORROW]] : $MyActor
23+
// CHECK: [[FN:%.*]] = function_ref @$s24local_function_isolation6calleeyyAA11NotSendableCF : $@convention(thin) (@guaranteed NotSendable) -> ()
24+
// CHECK-NEXT: apply [[FN]](%0) : $@convention(thin) (@guaranteed NotSendable) -> ()
25+
26+
// we need to hop back to 'self' here
27+
callee(ns)
28+
29+
// CHECK: end_borrow [[BORROW]] : $MyActor
30+
// CHECK-NEXT: destroy_value [[COPY]] : $MyActor
31+
}
32+
}
33+
}
34+
35+
func f(isolation: isolated MyActor, ns: NotSendable) {
36+
// CHECK-LABEL: sil private [ossa] @$s24local_function_isolation1f0C02nsyAA7MyActorCYi_AA11NotSendableCtF23implicitIsolatedCaptureL_yyYaF : $@convention(thin) @async (@guaranteed NotSendable, @sil_isolated @guaranteed MyActor) -> () {
37+
func implicitIsolatedCapture() async {
38+
39+
// CHECK: [[COPY:%.*]] = copy_value %1 : $MyActor
40+
// CHECK-NEXT: [[BORROW:%.*]] = begin_borrow [[COPY]] : $MyActor
41+
// CHECK-NEXT: hop_to_executor [[BORROW]] : $MyActor
42+
43+
// CHECK: [[FN:%.*]] = function_ref @$s24local_function_isolation4testyyYaF : $@convention(thin) @async () -> ()
44+
// CHECK-NEXT: apply [[FN]]() : $@convention(thin) @async () -> ()
45+
await test()
46+
47+
// we need to hop back to 'isolation' here
48+
callee(ns)
49+
50+
// CHECK: end_borrow [[BORROW]] : $MyActor
51+
// CHECK-NEXT: destroy_value [[COPY]] : $MyActor
52+
}
53+
}
54+
55+
func test() async {}

0 commit comments

Comments
 (0)