Merge pull request swiftlang#76434 from gottesmm/pr-3e00ab647dbc50beececb8c8cf096921bdf7acd3

[concurrency] Represent a SILFunction without isolation as std::optional<ActorIsolation> instead of ActorIsolation::Unspecified.

Author: gottesmm

include/swift/SIL/SILFunction.h

@@ -351,7 +351,7 @@ class SILFunction
   unsigned BlockListChangeIdx = 0;
 
   /// The isolation of this function.
-  ActorIsolation actorIsolation = ActorIsolation::forUnspecified();
+  std::optional<ActorIsolation> actorIsolation;
 
   /// The function's bare attribute. Bare means that the function is SIL-only
   /// and does not require debug info.
@@ -1451,7 +1451,9 @@ class SILFunction
     actorIsolation = newActorIsolation;
   }
 
-  ActorIsolation getActorIsolation() const { return actorIsolation; }
+  std::optional<ActorIsolation> getActorIsolation() const {
+    return actorIsolation;
+  }
 
   /// Return the source file that this SILFunction belongs to if it exists.
   SourceFile *getSourceFile() const;

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -1425,9 +1425,9 @@ struct PartitionOpEvaluator {
       // our transferring operand. If so, we can squelch this.
       if (auto functionIsolation =
               transferringOp->getUser()->getFunction()->getActorIsolation()) {
-        if (functionIsolation.isActorIsolated() &&
+        if (functionIsolation->isActorIsolated() &&
             SILIsolationInfo::get(transferringOp->getUser())
-                .hasSameIsolation(functionIsolation))
+                .hasSameIsolation(*functionIsolation))
           return;
       }
     }

lib/SIL/IR/SILPrinter.cpp

@@ -3376,9 +3376,10 @@ void SILFunction::print(SILPrintContext &PrintCtx) const {
 
   if (auto functionIsolation = getActorIsolation()) {
     OS << "// Isolation: ";
-    functionIsolation.print(OS);
+    functionIsolation->print(OS);
     OS << '\n';
   }
+
   printClangQualifiedNameCommentIfPresent(OS, getClangDecl());
 
   OS << "sil ";

lib/SILGen/SILGen.cpp

@@ -754,10 +754,7 @@ SILFunction *SILGenModule::getFunction(SILDeclRef constant,
 
   // If we have global actor isolation for our constant, put the isolation onto
   // the function.
-  if (auto isolation =
-          getActorIsolationOfContext(constant.getInnermostDeclContext())) {
-    F->setActorIsolation(isolation);
-  }
+  F->setActorIsolation(getActorIsolationOfContext(constant.getInnermostDeclContext()));
 
   assert(F && "SILFunction should have been defined");
 
@@ -1248,12 +1245,7 @@ void SILGenModule::preEmitFunction(SILDeclRef constant, SILFunction *F,
     F->setGenericEnvironment(genericEnv, capturedEnvs, forwardingSubs);
   }
 
-  // If we have global actor isolation for our constant, put the isolation onto
-  // the function.
-  if (auto isolation =
-          getActorIsolationOfContext(constant.getInnermostDeclContext())) {
-    F->setActorIsolation(isolation);
-  }
+  F->setActorIsolation(getActorIsolationOfContext(constant.getInnermostDeclContext()));
 
   // Create a debug scope for the function using astNode as source location.
   F->setDebugScope(new (M) SILDebugScope(Loc, F));

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

@@ -509,59 +509,62 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
 
   // Treat function ref as either actor isolated or sendable.
   if (auto *fri = dyn_cast<FunctionRefInst>(inst)) {
-    auto isolation = fri->getReferencedFunction()->getActorIsolation();
+    if (auto optIsolation = fri->getReferencedFunction()->getActorIsolation()) {
+      auto isolation = *optIsolation;
 
-    // First check if we are actor isolated at the AST level... if we are, then
-    // create the relevant actor isolated.
-    if (isolation.isActorIsolated()) {
-      if (isolation.isGlobalActor()) {
-        return SILIsolationInfo::getGlobalActorIsolated(
-            fri, isolation.getGlobalActor());
-      }
+      // First check if we are actor isolated at the AST level... if we are,
+      // then create the relevant actor isolated.
+      if (isolation.isActorIsolated()) {
+        if (isolation.isGlobalActor()) {
+          return SILIsolationInfo::getGlobalActorIsolated(
+              fri, isolation.getGlobalActor());
+        }
 
-      // TODO: We need to be able to support flow sensitive actor instances like
-      // we do for partial apply. Until we do so, just store SILValue() for
-      // this. This could cause a problem if we can construct a function ref and
-      // invoke it with two different actor instances of the same type and pass
-      // in the same parameters to both. We should error and we would not with
-      // this impl since we could not distinguish the two.
-      if (isolation.getKind() == ActorIsolation::ActorInstance) {
-        return SILIsolationInfo::getFlowSensitiveActorIsolated(fri, isolation);
-      }
+        // TODO: We need to be able to support flow sensitive actor instances
+        // like we do for partial apply. Until we do so, just store SILValue()
+        // for this. This could cause a problem if we can construct a function
+        // ref and invoke it with two different actor instances of the same type
+        // and pass in the same parameters to both. We should error and we would
+        // not with this impl since we could not distinguish the two.
+        if (isolation.getKind() == ActorIsolation::ActorInstance) {
+          return SILIsolationInfo::getFlowSensitiveActorIsolated(fri,
+                                                                 isolation);
+        }
 
-      assert(isolation.getKind() != ActorIsolation::Erased &&
-             "Implement this!");
-    }
+        assert(isolation.getKind() != ActorIsolation::Erased &&
+               "Implement this!");
+      }
 
-    // Then check if we have something that is nonisolated unsafe.
-    if (isolation.isNonisolatedUnsafe()) {
-      // First check if our function_ref is a method of a global actor isolated
-      // type. In such a case, we create a global actor isolated
-      // nonisolated(unsafe) so that if we assign the value to another variable,
-      // the variable still says that it is the appropriate global actor
-      // isolated thing.
-      //
-      // E.x.:
-      //
-      // @MainActor
-      // struct X { nonisolated(unsafe) var x: NonSendableThing { ... } }
-      //
-      // We want X.x to be safe to use... but to have that 'z' in the following
-      // is considered MainActor isolated.
-      //
-      // let z = X.x
-      //
-      auto *func = fri->getReferencedFunction();
-      auto funcType = func->getLoweredFunctionType();
-      if (funcType->hasSelfParam()) {
-        auto selfParam = funcType->getSelfInstanceType(
-            fri->getModule(), func->getTypeExpansionContext());
-        if (auto *nomDecl = selfParam->getNominalOrBoundGenericNominal()) {
-          auto isolation = swift::getActorIsolation(nomDecl);
-          if (isolation.isGlobalActor()) {
-            return SILIsolationInfo::getGlobalActorIsolated(
-                fri, isolation.getGlobalActor())
-              .withUnsafeNonIsolated(true);
+      // Then check if we have something that is nonisolated unsafe.
+      if (isolation.isNonisolatedUnsafe()) {
+        // First check if our function_ref is a method of a global actor
+        // isolated type. In such a case, we create a global actor isolated
+        // nonisolated(unsafe) so that if we assign the value to another
+        // variable, the variable still says that it is the appropriate global
+        // actor isolated thing.
+        //
+        // E.x.:
+        //
+        // @MainActor
+        // struct X { nonisolated(unsafe) var x: NonSendableThing { ... } }
+        //
+        // We want X.x to be safe to use... but to have that 'z' in the
+        // following is considered MainActor isolated.
+        //
+        // let z = X.x
+        //
+        auto *func = fri->getReferencedFunction();
+        auto funcType = func->getLoweredFunctionType();
+        if (funcType->hasSelfParam()) {
+          auto selfParam = funcType->getSelfInstanceType(
+              fri->getModule(), func->getTypeExpansionContext());
+          if (auto *nomDecl = selfParam->getNominalOrBoundGenericNominal()) {
+            auto nomDeclIsolation = swift::getActorIsolation(nomDecl);
+            if (nomDeclIsolation.isGlobalActor()) {
+              return SILIsolationInfo::getGlobalActorIsolated(
+                         fri, nomDeclIsolation.getGlobalActor())
+                  .withUnsafeNonIsolated(true);
+            }
           }
         }
       }
@@ -868,8 +871,11 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
     // code. In the case of a non-actor, we can only have an allocator that is
     // global actor isolated, so we will never hit this code path.
     if (declRef.kind == SILDeclRef::Kind::Allocator) {
-      if (fArg->getFunction()->getActorIsolation().isActorInstanceIsolated()) {
-        return SILIsolationInfo::getDisconnected(false /*nonisolated(unsafe)*/);
+      if (auto isolation = fArg->getFunction()->getActorIsolation()) {
+        if (isolation->isActorInstanceIsolated()) {
+          return SILIsolationInfo::getDisconnected(
+              false /*nonisolated(unsafe)*/);
+        }
       }
     }
 
@@ -878,13 +884,13 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
     // we need to pass in a "fake" ActorInstance that users know is a sentinel
     // for the self value.
     if (auto functionIsolation = fArg->getFunction()->getActorIsolation()) {
-      if (functionIsolation.isActorInstanceIsolated() && declRef.getDecl()) {
+      if (functionIsolation->isActorInstanceIsolated() && declRef.getDecl()) {
         if (auto *accessor =
                 dyn_cast_or_null<AccessorDecl>(declRef.getFuncDecl())) {
           if (accessor->isInitAccessor()) {
             return SILIsolationInfo::getActorInstanceIsolated(
                 fArg, ActorInstance::getForActorAccessorInit(),
-                functionIsolation.getActor());
+                functionIsolation->getActor());
           }
         }
       }
@@ -894,15 +900,15 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
   // Otherwise, if we do not have an isolated argument and are not in an
   // allocator, then we might be isolated via global isolation.
   if (auto functionIsolation = fArg->getFunction()->getActorIsolation()) {
-    if (functionIsolation.isActorIsolated()) {
-      if (functionIsolation.isGlobalActor()) {
+    if (functionIsolation->isActorIsolated()) {
+      if (functionIsolation->isGlobalActor()) {
         return SILIsolationInfo::getGlobalActorIsolated(
-            fArg, functionIsolation.getGlobalActor());
+            fArg, functionIsolation->getGlobalActor());
       }
 
       return SILIsolationInfo::getActorInstanceIsolated(
           fArg, ActorInstance::getForActorAccessorInit(),
-          functionIsolation.getActor());
+          functionIsolation->getActor());
     }
   }
 

test/Concurrency/actor_isolation_filecheck.swift

@@ -0,0 +1,34 @@
+// RUN: %target-swift-frontend -swift-version 6 -disable-availability-checking %s -emit-silgen -o - | %FileCheck %s
+// RUN: %target-swift-frontend -swift-version 6 -disable-availability-checking %s -emit-sil -o /dev/null -verify
+
+// README: This file contains FileCheck tests that validate that specific Swift
+// entities have their respective SILFunctions assigned the correct actor
+// isolation by FileChecking against SILGen.
+
+////////////////////////
+// MARK: Declarations //
+////////////////////////
+
+func useValueAsync<T>(_ t: T) async {}
+
+/////////////////
+// MARK: Tests //
+/////////////////
+
+// CHECK: // synchronousActorIsolatedFinalClassMethodError()
+// CHECK-NEXT: // Isolation: global_actor. type: MainActor
+// CHECK-NEXT: sil hidden [ossa] @$s25actor_isolation_filecheck45synchronousActorIsolatedFinalClassMethodErroryyYaF : $@convention(thin) @async () -> () {
+@MainActor func synchronousActorIsolatedFinalClassMethodError() async {
+  @MainActor final class Test {
+    // CHECK: // foo() in Test #1 in synchronousActorIsolatedFinalClassMethodError()
+    // CHECK-NEXT: // Isolation: global_actor. type: MainActor
+    // CHECK-NEXT: sil private [ossa] @$s25actor_isolation_filecheck45synchronousActorIsolatedFinalClassMethodErroryyYaF4TestL_C3fooyyF : $@convention(method) (@guaranteed Test) -> () {
+    func foo() {}
+  }
+
+  let t = Test()
+  let erased: () -> Void = t.foo
+
+  await useValueAsync(erased) // expected-error {{sending 'erased' risks causing data races}}
+  // expected-note @-1 {{sending main actor-isolated 'erased' to nonisolated global function 'useValueAsync' risks causing data races between nonisolated and main actor-isolated uses}}
+}

test/SILGen/default_constructor.swift

@@ -97,6 +97,7 @@ struct G {
 
 // CHECK-NOT: default_constructor.G.init()
 // CHECK-LABEL: default_constructor.G.init(bar: Swift.Optional<Swift.Int32>)
+// CHECK-NEXT: // Isolation:
 // CHECK-NEXT: sil hidden [ossa] @$s19default_constructor1GV{{[_0-9a-zA-Z]*}}fC
 // CHECK-NOT: default_constructor.G.init()
 

test/SILGen/functions.swift

@@ -481,6 +481,7 @@ func testNoescape() {
 }
 
 // CHECK-LABEL: functions.testNoescape() -> ()
+// CHECK-NEXT: // Isolation:
 // CHECK-NEXT: sil hidden [ossa] @$s9functions12testNoescapeyyF : $@convention(thin) () -> ()
 // CHECK: function_ref closure #1 () -> () in functions.testNoescape() -> ()
 // CHECK-NEXT: function_ref @$s9functions12testNoescapeyyFyyXEfU_ : $@convention(thin) (@guaranteed { var Int }) -> ()

test/SILGen/global_init_attribute.swift

@@ -10,6 +10,7 @@ import def_global
 let InternalConst = 42
 // CHECK-NOT: [global_init]
 // CHECK: // global_init_attribute.InternalConst.unsafeMutableAddressor : Swift.Int
+// CHECK-NEXT: // Isolation:
 // CHECK-NEXT: sil hidden [global_init] [ossa] @$s21global_init_attribute13InternalConstSivau
 
 func foo() -> Int {
@@ -22,10 +23,12 @@ func bar(i: Int) {
 
 // CHECK-NOT: [global_init]
 // CHECK: // def_global.ExportedVar.unsafeMutableAddressor : Swift.Int
+// CHECK-NEXT: // Isolation:
 // CHECK-NEXT: sil [global_init] @$s10def_global11ExportedVarSivau
 
 var InternalFoo = foo()
 
 // CHECK-NOT: [global_init]
 // CHECK: // global_init_attribute.InternalFoo.unsafeMutableAddressor : Swift.Int
+// CHECK-NEXT: // Isolation:
 // CHECK-NEXT: sil hidden [global_init] [ossa] @$s21global_init_attribute11InternalFooSivau