Skip to content

Commit 92a3e53

Browse files
authored
Merge pull request swiftlang#76434 from gottesmm/pr-3e00ab647dbc50beececb8c8cf096921bdf7acd3
[concurrency] Represent a SILFunction without isolation as std::optional<ActorIsolation> instead of ActorIsolation::Unspecified.
2 parents 5512d83 + a008832 commit 92a3e53

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
@@ -351,7 +351,7 @@ class SILFunction
351351
unsigned BlockListChangeIdx = 0;
352352

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

356356
/// The function's bare attribute. Bare means that the function is SIL-only
357357
/// and does not require debug info.
@@ -1451,7 +1451,9 @@ class SILFunction
14511451
actorIsolation = newActorIsolation;
14521452
}
14531453

1454-
ActorIsolation getActorIsolation() const { return actorIsolation; }
1454+
std::optional<ActorIsolation> getActorIsolation() const {
1455+
return actorIsolation;
1456+
}
14551457

14561458
/// Return the source file that this SILFunction belongs to if it exists.
14571459
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)