Skip to content

Commit a008832

Browse files
committed
[concurrency] Represent a SILFunction without isolation as std::optional<ActorIsolation> instead of ActorIsolation::Unspecified.
The reason why is that we want to distinguish inbetween SILFunction's that are marked as unspecified by SILGen and those that are parsed from textual SIL that do not have any specified isolation. This will make it easier to write nice FileCheck tests against SILGen output on what is the inferred isolation for various items. NFCI.
1 parent fad9a5f commit a008832

17 files changed

+173
-77
lines changed

include/swift/SIL/SILFunction.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ class SILFunction
353353
unsigned BlockListChangeIdx = 0;
354354

355355
/// The isolation of this function.
356-
ActorIsolation actorIsolation = ActorIsolation::forUnspecified();
356+
std::optional<ActorIsolation> actorIsolation;
357357

358358
/// The function's bare attribute. Bare means that the function is SIL-only
359359
/// and does not require debug info.
@@ -1453,7 +1453,9 @@ class SILFunction
14531453
actorIsolation = newActorIsolation;
14541454
}
14551455

1456-
ActorIsolation getActorIsolation() const { return actorIsolation; }
1456+
std::optional<ActorIsolation> getActorIsolation() const {
1457+
return actorIsolation;
1458+
}
14571459

14581460
/// Return the source file that this SILFunction belongs to if it exists.
14591461
SourceFile *getSourceFile() const;

include/swift/SILOptimizer/Utils/PartitionUtils.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1425,9 +1425,9 @@ struct PartitionOpEvaluator {
14251425
// our transferring operand. If so, we can squelch this.
14261426
if (auto functionIsolation =
14271427
transferringOp->getUser()->getFunction()->getActorIsolation()) {
1428-
if (functionIsolation.isActorIsolated() &&
1428+
if (functionIsolation->isActorIsolated() &&
14291429
SILIsolationInfo::get(transferringOp->getUser())
1430-
.hasSameIsolation(functionIsolation))
1430+
.hasSameIsolation(*functionIsolation))
14311431
return;
14321432
}
14331433
}

lib/SIL/IR/SILPrinter.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3376,9 +3376,10 @@ void SILFunction::print(SILPrintContext &PrintCtx) const {
33763376

33773377
if (auto functionIsolation = getActorIsolation()) {
33783378
OS << "// Isolation: ";
3379-
functionIsolation.print(OS);
3379+
functionIsolation->print(OS);
33803380
OS << '\n';
33813381
}
3382+
33823383
printClangQualifiedNameCommentIfPresent(OS, getClangDecl());
33833384

33843385
OS << "sil ";

lib/SILGen/SILGen.cpp

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -754,10 +754,7 @@ SILFunction *SILGenModule::getFunction(SILDeclRef constant,
754754

755755
// If we have global actor isolation for our constant, put the isolation onto
756756
// the function.
757-
if (auto isolation =
758-
getActorIsolationOfContext(constant.getInnermostDeclContext())) {
759-
F->setActorIsolation(isolation);
760-
}
757+
F->setActorIsolation(getActorIsolationOfContext(constant.getInnermostDeclContext()));
761758

762759
assert(F && "SILFunction should have been defined");
763760

@@ -1248,12 +1245,7 @@ void SILGenModule::preEmitFunction(SILDeclRef constant, SILFunction *F,
12481245
F->setGenericEnvironment(genericEnv, capturedEnvs, forwardingSubs);
12491246
}
12501247

1251-
// If we have global actor isolation for our constant, put the isolation onto
1252-
// the function.
1253-
if (auto isolation =
1254-
getActorIsolationOfContext(constant.getInnermostDeclContext())) {
1255-
F->setActorIsolation(isolation);
1256-
}
1248+
F->setActorIsolation(getActorIsolationOfContext(constant.getInnermostDeclContext()));
12571249

12581250
// Create a debug scope for the function using astNode as source location.
12591251
F->setDebugScope(new (M) SILDebugScope(Loc, F));

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

Lines changed: 63 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -509,59 +509,62 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
509509

510510
// Treat function ref as either actor isolated or sendable.
511511
if (auto *fri = dyn_cast<FunctionRefInst>(inst)) {
512-
auto isolation = fri->getReferencedFunction()->getActorIsolation();
512+
if (auto optIsolation = fri->getReferencedFunction()->getActorIsolation()) {
513+
auto isolation = *optIsolation;
513514

514-
// First check if we are actor isolated at the AST level... if we are, then
515-
// create the relevant actor isolated.
516-
if (isolation.isActorIsolated()) {
517-
if (isolation.isGlobalActor()) {
518-
return SILIsolationInfo::getGlobalActorIsolated(
519-
fri, isolation.getGlobalActor());
520-
}
515+
// First check if we are actor isolated at the AST level... if we are,
516+
// then create the relevant actor isolated.
517+
if (isolation.isActorIsolated()) {
518+
if (isolation.isGlobalActor()) {
519+
return SILIsolationInfo::getGlobalActorIsolated(
520+
fri, isolation.getGlobalActor());
521+
}
521522

522-
// TODO: We need to be able to support flow sensitive actor instances like
523-
// we do for partial apply. Until we do so, just store SILValue() for
524-
// this. This could cause a problem if we can construct a function ref and
525-
// invoke it with two different actor instances of the same type and pass
526-
// in the same parameters to both. We should error and we would not with
527-
// this impl since we could not distinguish the two.
528-
if (isolation.getKind() == ActorIsolation::ActorInstance) {
529-
return SILIsolationInfo::getFlowSensitiveActorIsolated(fri, isolation);
530-
}
523+
// TODO: We need to be able to support flow sensitive actor instances
524+
// like we do for partial apply. Until we do so, just store SILValue()
525+
// for this. This could cause a problem if we can construct a function
526+
// ref and invoke it with two different actor instances of the same type
527+
// and pass in the same parameters to both. We should error and we would
528+
// not with this impl since we could not distinguish the two.
529+
if (isolation.getKind() == ActorIsolation::ActorInstance) {
530+
return SILIsolationInfo::getFlowSensitiveActorIsolated(fri,
531+
isolation);
532+
}
531533

532-
assert(isolation.getKind() != ActorIsolation::Erased &&
533-
"Implement this!");
534-
}
534+
assert(isolation.getKind() != ActorIsolation::Erased &&
535+
"Implement this!");
536+
}
535537

536-
// Then check if we have something that is nonisolated unsafe.
537-
if (isolation.isNonisolatedUnsafe()) {
538-
// First check if our function_ref is a method of a global actor isolated
539-
// type. In such a case, we create a global actor isolated
540-
// nonisolated(unsafe) so that if we assign the value to another variable,
541-
// the variable still says that it is the appropriate global actor
542-
// isolated thing.
543-
//
544-
// E.x.:
545-
//
546-
// @MainActor
547-
// struct X { nonisolated(unsafe) var x: NonSendableThing { ... } }
548-
//
549-
// We want X.x to be safe to use... but to have that 'z' in the following
550-
// is considered MainActor isolated.
551-
//
552-
// let z = X.x
553-
//
554-
auto *func = fri->getReferencedFunction();
555-
auto funcType = func->getLoweredFunctionType();
556-
if (funcType->hasSelfParam()) {
557-
auto selfParam = funcType->getSelfInstanceType(
558-
fri->getModule(), func->getTypeExpansionContext());
559-
if (auto *nomDecl = selfParam->getNominalOrBoundGenericNominal()) {
560-
auto isolation = swift::getActorIsolation(nomDecl);
561-
if (isolation.isGlobalActor()) {
562-
return SILIsolationInfo::getGlobalActorIsolated(
563-
fri, isolation.getGlobalActor())
564-
.withUnsafeNonIsolated(true);
538+
// Then check if we have something that is nonisolated unsafe.
539+
if (isolation.isNonisolatedUnsafe()) {
540+
// First check if our function_ref is a method of a global actor
541+
// isolated type. In such a case, we create a global actor isolated
542+
// nonisolated(unsafe) so that if we assign the value to another
543+
// variable, the variable still says that it is the appropriate global
544+
// actor isolated thing.
545+
//
546+
// E.x.:
547+
//
548+
// @MainActor
549+
// struct X { nonisolated(unsafe) var x: NonSendableThing { ... } }
550+
//
551+
// We want X.x to be safe to use... but to have that 'z' in the
552+
// following is considered MainActor isolated.
553+
//
554+
// let z = X.x
555+
//
556+
auto *func = fri->getReferencedFunction();
557+
auto funcType = func->getLoweredFunctionType();
558+
if (funcType->hasSelfParam()) {
559+
auto selfParam = funcType->getSelfInstanceType(
560+
fri->getModule(), func->getTypeExpansionContext());
561+
if (auto *nomDecl = selfParam->getNominalOrBoundGenericNominal()) {
562+
auto nomDeclIsolation = swift::getActorIsolation(nomDecl);
563+
if (nomDeclIsolation.isGlobalActor()) {
564+
return SILIsolationInfo::getGlobalActorIsolated(
565+
fri, nomDeclIsolation.getGlobalActor())
566+
.withUnsafeNonIsolated(true);
567+
}
565568
}
566569
}
567570
}
@@ -868,8 +871,11 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
868871
// code. In the case of a non-actor, we can only have an allocator that is
869872
// global actor isolated, so we will never hit this code path.
870873
if (declRef.kind == SILDeclRef::Kind::Allocator) {
871-
if (fArg->getFunction()->getActorIsolation().isActorInstanceIsolated()) {
872-
return SILIsolationInfo::getDisconnected(false /*nonisolated(unsafe)*/);
874+
if (auto isolation = fArg->getFunction()->getActorIsolation()) {
875+
if (isolation->isActorInstanceIsolated()) {
876+
return SILIsolationInfo::getDisconnected(
877+
false /*nonisolated(unsafe)*/);
878+
}
873879
}
874880
}
875881

@@ -878,13 +884,13 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
878884
// we need to pass in a "fake" ActorInstance that users know is a sentinel
879885
// for the self value.
880886
if (auto functionIsolation = fArg->getFunction()->getActorIsolation()) {
881-
if (functionIsolation.isActorInstanceIsolated() && declRef.getDecl()) {
887+
if (functionIsolation->isActorInstanceIsolated() && declRef.getDecl()) {
882888
if (auto *accessor =
883889
dyn_cast_or_null<AccessorDecl>(declRef.getFuncDecl())) {
884890
if (accessor->isInitAccessor()) {
885891
return SILIsolationInfo::getActorInstanceIsolated(
886892
fArg, ActorInstance::getForActorAccessorInit(),
887-
functionIsolation.getActor());
893+
functionIsolation->getActor());
888894
}
889895
}
890896
}
@@ -894,15 +900,15 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
894900
// Otherwise, if we do not have an isolated argument and are not in an
895901
// allocator, then we might be isolated via global isolation.
896902
if (auto functionIsolation = fArg->getFunction()->getActorIsolation()) {
897-
if (functionIsolation.isActorIsolated()) {
898-
if (functionIsolation.isGlobalActor()) {
903+
if (functionIsolation->isActorIsolated()) {
904+
if (functionIsolation->isGlobalActor()) {
899905
return SILIsolationInfo::getGlobalActorIsolated(
900-
fArg, functionIsolation.getGlobalActor());
906+
fArg, functionIsolation->getGlobalActor());
901907
}
902908

903909
return SILIsolationInfo::getActorInstanceIsolated(
904910
fArg, ActorInstance::getForActorAccessorInit(),
905-
functionIsolation.getActor());
911+
functionIsolation->getActor());
906912
}
907913
}
908914

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// RUN: %target-swift-frontend -swift-version 6 -disable-availability-checking %s -emit-silgen -o - | %FileCheck %s
2+
// RUN: %target-swift-frontend -swift-version 6 -disable-availability-checking %s -emit-sil -o /dev/null -verify
3+
4+
// README: This file contains FileCheck tests that validate that specific Swift
5+
// entities have their respective SILFunctions assigned the correct actor
6+
// isolation by FileChecking against SILGen.
7+
8+
////////////////////////
9+
// MARK: Declarations //
10+
////////////////////////
11+
12+
func useValueAsync<T>(_ t: T) async {}
13+
14+
/////////////////
15+
// MARK: Tests //
16+
/////////////////
17+
18+
// CHECK: // synchronousActorIsolatedFinalClassMethodError()
19+
// CHECK-NEXT: // Isolation: global_actor. type: MainActor
20+
// CHECK-NEXT: sil hidden [ossa] @$s25actor_isolation_filecheck45synchronousActorIsolatedFinalClassMethodErroryyYaF : $@convention(thin) @async () -> () {
21+
@MainActor func synchronousActorIsolatedFinalClassMethodError() async {
22+
@MainActor final class Test {
23+
// CHECK: // foo() in Test #1 in synchronousActorIsolatedFinalClassMethodError()
24+
// CHECK-NEXT: // Isolation: global_actor. type: MainActor
25+
// CHECK-NEXT: sil private [ossa] @$s25actor_isolation_filecheck45synchronousActorIsolatedFinalClassMethodErroryyYaF4TestL_C3fooyyF : $@convention(method) (@guaranteed Test) -> () {
26+
func foo() {}
27+
}
28+
29+
let t = Test()
30+
let erased: () -> Void = t.foo
31+
32+
await useValueAsync(erased) // expected-error {{sending 'erased' risks causing data races}}
33+
// expected-note @-1 {{sending main actor-isolated 'erased' to nonisolated global function 'useValueAsync' risks causing data races between nonisolated and main actor-isolated uses}}
34+
}

test/SILGen/default_constructor.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ struct G {
9797

9898
// CHECK-NOT: default_constructor.G.init()
9999
// CHECK-LABEL: default_constructor.G.init(bar: Swift.Optional<Swift.Int32>)
100+
// CHECK-NEXT: // Isolation:
100101
// CHECK-NEXT: sil hidden [ossa] @$s19default_constructor1GV{{[_0-9a-zA-Z]*}}fC
101102
// CHECK-NOT: default_constructor.G.init()
102103

test/SILGen/functions.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,7 @@ func testNoescape() {
481481
}
482482

483483
// CHECK-LABEL: functions.testNoescape() -> ()
484+
// CHECK-NEXT: // Isolation:
484485
// CHECK-NEXT: sil hidden [ossa] @$s9functions12testNoescapeyyF : $@convention(thin) () -> ()
485486
// CHECK: function_ref closure #1 () -> () in functions.testNoescape() -> ()
486487
// CHECK-NEXT: function_ref @$s9functions12testNoescapeyyFyyXEfU_ : $@convention(thin) (@guaranteed { var Int }) -> ()

test/SILGen/global_init_attribute.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import def_global
1010
let InternalConst = 42
1111
// CHECK-NOT: [global_init]
1212
// CHECK: // global_init_attribute.InternalConst.unsafeMutableAddressor : Swift.Int
13+
// CHECK-NEXT: // Isolation:
1314
// CHECK-NEXT: sil hidden [global_init] [ossa] @$s21global_init_attribute13InternalConstSivau
1415

1516
func foo() -> Int {
@@ -22,10 +23,12 @@ func bar(i: Int) {
2223

2324
// CHECK-NOT: [global_init]
2425
// CHECK: // def_global.ExportedVar.unsafeMutableAddressor : Swift.Int
26+
// CHECK-NEXT: // Isolation:
2527
// CHECK-NEXT: sil [global_init] @$s10def_global11ExportedVarSivau
2628

2729
var InternalFoo = foo()
2830

2931
// CHECK-NOT: [global_init]
3032
// CHECK: // global_init_attribute.InternalFoo.unsafeMutableAddressor : Swift.Int
33+
// CHECK-NEXT: // Isolation:
3134
// CHECK-NEXT: sil hidden [global_init] [ossa] @$s21global_init_attribute11InternalFooSivau

0 commit comments

Comments
 (0)