Merge pull request #79087 from gottesmm/actor-isolation-refactoring

[concurrency] Introduce concurrent, swap its meaning with nonisolated and make nonisolated caller inheriting isolation.

Author: gottesmm

include/swift/AST/ActorIsolation.h

@@ -57,8 +57,19 @@ class ActorIsolation {
     /// the actor is isolated to the instance of that actor.
     ActorInstance,
     /// The declaration is explicitly specified to be not isolated to any actor,
-    /// meaning that it can be used from any actor but is also unable to
-    /// refer to the isolated state of any given actor.
+    /// meaning it cannot itself access actor isolated state but /does/ allow
+    /// for indirect access to actor isolated state if the declaration can
+    /// guarantee that all code generated to work with the declaration will run
+    /// on said actor.
+    ///
+    /// E.x.: a nonisolated function can accept actor-isolated values as
+    /// arguments since the caller knows that it will not escape the values to
+    /// another isolation domain and that the function will remain on the
+    /// caller's actor.
+    ///
+    /// NOTE: This used to have the meaning of Concurrent which is a stricter
+    /// definition of nonisolated that prevents the code generated to work with
+    /// the declaration to never touch actor isolated state.
     Nonisolated,
     /// The declaration is explicitly specified to be not isolated and with the
     /// "unsafe" annotation, which means that we do not enforce isolation.
@@ -69,11 +80,20 @@ class ActorIsolation {
     /// The actor isolation iss statically erased, as for a call to
     /// an isolated(any) function.  This is not possible for declarations.
     Erased,
-    /// Inherits isolation from the caller of the given function.
+    /// The declaration is explicitly specified to be not isolated to any actor,
+    /// meaning that it can be used from any actor but is also unable to
+    /// refer to the isolated state of any given actor.
     ///
-    /// DISCUSSION: This is used for nonisolated asynchronous functions that we
-    /// want to inherit from their context the context's actor isolation.
-    CallerIsolationInheriting,
+    /// NOTE: This used to be nonisolated, but we changed nonisolated to have a
+    /// weaker definition of nonisolated that allows for actor isolated state to
+    /// be manipulated by code generated to work with the actor as long as all
+    /// such code generation is guaranteed to always run on whatever actor
+    /// isolation it is invoked in consistently and not escape the value to any
+    /// other isolation domain.
+    Concurrent,
+    /// The declaration is explicitly specified to be concurrent and with the
+    /// "unsafe" annotation, which means that we do not enforce isolation.
+    ConcurrentUnsafe,
   };
 
 private:
@@ -88,7 +108,7 @@ class ActorIsolation {
   /// Set to true if this was parsed from SIL.
   unsigned silParsed : 1;
 
-  unsigned parameterIndex : 27;
+  unsigned parameterIndex : 26;
 
   ActorIsolation(Kind kind, NominalTypeDecl *actor, unsigned parameterIndex);
 
@@ -112,10 +132,8 @@ class ActorIsolation {
     return ActorIsolation(unsafe ? NonisolatedUnsafe : Nonisolated);
   }
 
-  static ActorIsolation forCallerIsolationInheriting() {
-    // NOTE: We do not use parameter indices since the parameter is implicit
-    // from the perspective of the AST.
-    return ActorIsolation(CallerIsolationInheriting);
+  static ActorIsolation forConcurrent(bool unsafe) {
+    return ActorIsolation(unsafe ? ConcurrentUnsafe : Concurrent);
   }
 
   static ActorIsolation forActorInstanceSelf(ValueDecl *decl);
@@ -165,9 +183,10 @@ class ActorIsolation {
                   std::optional<ActorIsolation>(ActorIsolation::GlobalActor))
             .Case("global_actor_unsafe",
                   std::optional<ActorIsolation>(ActorIsolation::GlobalActor))
-            .Case("caller_isolation_inheriting",
-                  std::optional<ActorIsolation>(
-                      ActorIsolation::CallerIsolationInheriting))
+            .Case("concurrent",
+                  std::optional<ActorIsolation>(ActorIsolation::Concurrent))
+            .Case("concurrent_unsafe", std::optional<ActorIsolation>(
+                                           ActorIsolation::ConcurrentUnsafe))
             .Default(std::nullopt);
     if (kind == std::nullopt)
       return std::nullopt;
@@ -180,11 +199,17 @@ class ActorIsolation {
 
   bool isUnspecified() const { return kind == Unspecified; }
 
+  bool isConcurrent() const {
+    return (kind == Concurrent) || (kind == ConcurrentUnsafe);
+  }
+
   bool isNonisolated() const {
     return (kind == Nonisolated) || (kind == NonisolatedUnsafe);
   }
 
-  bool isNonisolatedUnsafe() const { return kind == NonisolatedUnsafe; }
+  bool isUnsafe() const {
+    return kind == NonisolatedUnsafe || kind == ConcurrentUnsafe;
+  }
 
   /// Retrieve the parameter to which actor-instance isolation applies.
   ///
@@ -212,7 +237,8 @@ class ActorIsolation {
     case Unspecified:
     case Nonisolated:
     case NonisolatedUnsafe:
-    case CallerIsolationInheriting:
+    case Concurrent:
+    case ConcurrentUnsafe:
       return false;
     }
   }

lib/AST/ASTDumper.cpp

@@ -3844,18 +3844,19 @@ class PrintExpr : public ExprVisitor<PrintExpr, void, Label>,
     switch (auto isolation = E->getActorIsolation()) {
     case ActorIsolation::Unspecified:
     case ActorIsolation::NonisolatedUnsafe:
+    case ActorIsolation::ConcurrentUnsafe:
       break;
 
     case ActorIsolation::Nonisolated:
       printFlag(true, "nonisolated", CapturesColor);
       break;
 
-    case ActorIsolation::Erased:
-      printFlag(true, "dynamically_isolated", CapturesColor);
+    case ActorIsolation::Concurrent:
+      printFlag(true, "concurrent", CapturesColor);
       break;
 
-    case ActorIsolation::CallerIsolationInheriting:
-      printFlag(true, "isolated_to_caller_isolation", CapturesColor);
+    case ActorIsolation::Erased:
+      printFlag(true, "dynamically_isolated", CapturesColor);
       break;
 
     case ActorIsolation::ActorInstance:

lib/AST/ActorIsolation.cpp

@@ -39,7 +39,10 @@ ActorIsolation::ActorIsolation(Kind kind, Expr *actor, unsigned parameterIndex)
 
 ActorIsolation::ActorIsolation(Kind kind, Type globalActor)
     : globalActor(globalActor), kind(kind), isolatedByPreconcurrency(false),
-      silParsed(false), parameterIndex(0) {}
+      silParsed(false), parameterIndex(0) {
+  assert((silParsed || globalActor) &&
+         "If we are not sil parsed, global actor must be a real type");
+}
 
 ActorIsolation
 ActorIsolation::forActorInstanceParameter(Expr *actor,
@@ -49,7 +52,7 @@ ActorIsolation::forActorInstanceParameter(Expr *actor,
   // An isolated value of `nil` is statically nonisolated.
   // FIXME: Also allow 'Optional.none'
   if (isa<NilLiteralExpr>(actor))
-    return ActorIsolation::forNonisolated(/*unsafe*/ false);
+    return ActorIsolation::forConcurrent(/*unsafe*/ false);
 
   // An isolated value of `<global actor type>.shared` is statically
   // global actor isolated.
@@ -162,8 +165,8 @@ bool ActorIsolation::isEqual(const ActorIsolation &lhs,
     return false;
 
   switch (lhs.getKind()) {
-  case Nonisolated:
-  case NonisolatedUnsafe:
+  case Concurrent:
+  case ConcurrentUnsafe:
   case Unspecified:
     return true;
 
@@ -174,7 +177,8 @@ bool ActorIsolation::isEqual(const ActorIsolation &lhs,
     // to answer.
     return false;
 
-  case CallerIsolationInheriting:
+  case Nonisolated:
+  case NonisolatedUnsafe:
     // This returns false for the same reason as erased. The caller has to check
     // against the actual caller isolation.
     return false;

lib/AST/Decl.cpp

@@ -2757,6 +2757,7 @@ static bool deferMatchesEnclosingAccess(const FuncDecl *defer) {
         switch (isolation) {
           case ActorIsolation::Unspecified:
           case ActorIsolation::NonisolatedUnsafe:
+          case ActorIsolation::ConcurrentUnsafe:
             break;
 
           case ActorIsolation::GlobalActor:
@@ -2765,9 +2766,9 @@ static bool deferMatchesEnclosingAccess(const FuncDecl *defer) {
 
             return true;
 
-          case ActorIsolation::CallerIsolationInheriting:
           case ActorIsolation::ActorInstance:
           case ActorIsolation::Nonisolated:
+          case ActorIsolation::Concurrent:
           case ActorIsolation::Erased: // really can't happen
             return true;
         }
@@ -11461,9 +11462,10 @@ bool VarDecl::isSelfParamCaptureIsolated() const {
       case ActorIsolation::Unspecified:
       case ActorIsolation::Nonisolated:
       case ActorIsolation::NonisolatedUnsafe:
+      case ActorIsolation::Concurrent:
+      case ActorIsolation::ConcurrentUnsafe:
       case ActorIsolation::GlobalActor:
       case ActorIsolation::Erased:
-      case ActorIsolation::CallerIsolationInheriting:
         return false;
 
       case ActorIsolation::ActorInstance:

lib/AST/TypeCheckRequests.cpp

@@ -1856,10 +1856,11 @@ SourceLoc MacroDefinitionRequest::getNearestLoc() const {
 
 bool ActorIsolation::requiresSubstitution() const {
   switch (kind) {
-  case CallerIsolationInheriting:
   case ActorInstance:
   case Nonisolated:
   case NonisolatedUnsafe:
+  case Concurrent:
+  case ConcurrentUnsafe:
   case Unspecified:
     return false;
 
@@ -1872,9 +1873,10 @@ bool ActorIsolation::requiresSubstitution() const {
 ActorIsolation ActorIsolation::subst(SubstitutionMap subs) const {
   switch (kind) {
   case ActorInstance:
-  case CallerIsolationInheriting:
   case Nonisolated:
   case NonisolatedUnsafe:
+  case Concurrent:
+  case ConcurrentUnsafe:
   case Unspecified:
     return *this;
 
@@ -1893,11 +1895,6 @@ void ActorIsolation::printForDiagnostics(llvm::raw_ostream &os,
     os << "actor" << (asNoun ? " isolation" : "-isolated");
     break;
 
-  case ActorIsolation::CallerIsolationInheriting:
-    os << "caller isolation inheriting"
-       << (asNoun ? " isolation" : "-isolated");
-    break;
-
   case ActorIsolation::GlobalActor: {
     if (isMainActor()) {
       os << "main actor" << (asNoun ? " isolation" : "-isolated");
@@ -1912,11 +1909,13 @@ void ActorIsolation::printForDiagnostics(llvm::raw_ostream &os,
     os << "@isolated(any)";
     break;
 
+  case ActorIsolation::Concurrent:
+  case ActorIsolation::ConcurrentUnsafe:
   case ActorIsolation::Nonisolated:
   case ActorIsolation::NonisolatedUnsafe:
   case ActorIsolation::Unspecified:
     os << "nonisolated";
-    if (*this == ActorIsolation::NonisolatedUnsafe) {
+    if (isUnsafe()) {
       os << "(unsafe)";
     }
     break;
@@ -1934,15 +1933,18 @@ void ActorIsolation::print(llvm::raw_ostream &os) const {
       os << ". name: '" << vd->getBaseIdentifier() << "'";
     }
     return;
-  case CallerIsolationInheriting:
-    os << "caller_isolation_inheriting";
-    return;
   case Nonisolated:
     os << "nonisolated";
     return;
   case NonisolatedUnsafe:
     os << "nonisolated_unsafe";
     return;
+  case Concurrent:
+    os << "concurrent";
+    return;
+  case ConcurrentUnsafe:
+    os << "concurrent_unsafe";
+    return;
   case GlobalActor:
     os << "global_actor. type: " << getGlobalActor();
     return;
@@ -1961,15 +1963,18 @@ void ActorIsolation::printForSIL(llvm::raw_ostream &os) const {
   case ActorInstance:
     os << "actor_instance";
     return;
-  case CallerIsolationInheriting:
-    os << "caller_isolation_inheriting";
-    return;
   case Nonisolated:
     os << "nonisolated";
     return;
   case NonisolatedUnsafe:
     os << "nonisolated_unsafe";
     return;
+  case Concurrent:
+    os << "concurrent";
+    return;
+  case ConcurrentUnsafe:
+    os << "concurrent_unsafe";
+    return;
   case GlobalActor:
     os << "global_actor";
     return;
@@ -2009,14 +2014,12 @@ void swift::simple_display(
       }
       break;
 
-    case ActorIsolation::CallerIsolationInheriting:
-      out << "isolated to isolation of caller";
-      break;
-
     case ActorIsolation::Nonisolated:
     case ActorIsolation::NonisolatedUnsafe:
+    case ActorIsolation::Concurrent:
+    case ActorIsolation::ConcurrentUnsafe:
       out << "nonisolated";
-      if (state == ActorIsolation::NonisolatedUnsafe) {
+      if (state.isUnsafe()) {
         out << "(unsafe)";
       }
       break;

lib/IDE/CompletionLookup.cpp

@@ -827,8 +827,9 @@ void CompletionLookup::analyzeActorIsolation(
     break;
   case ActorIsolation::Unspecified:
   case ActorIsolation::Nonisolated:
-  case ActorIsolation::CallerIsolationInheriting:
   case ActorIsolation::NonisolatedUnsafe:
+  case ActorIsolation::Concurrent:
+  case ActorIsolation::ConcurrentUnsafe:
     return;
   }
 }

lib/SIL/IR/SILFunctionType.cpp

@@ -1651,8 +1651,7 @@ class DestructureInputs {
     // enabled.
     if (TC.Context.LangOpts.hasFeature(
             Feature::NonIsolatedAsyncInheritsIsolationFromContext) &&
-        IsolationInfo &&
-        IsolationInfo->getKind() == ActorIsolation::CallerIsolationInheriting &&
+        IsolationInfo && IsolationInfo->isNonisolated() &&
         extInfoBuilder.isAsync()) {
       auto actorProtocol = TC.Context.getProtocol(KnownProtocolKind::Actor);
       auto actorType =
@@ -2530,15 +2529,15 @@ static CanSILFunctionType getSILFunctionType(
     std::optional<ActorIsolation> actorIsolation;
     if (constant) {
       if (constant->kind == SILDeclRef::Kind::Deallocator) {
-        actorIsolation = ActorIsolation::forNonisolated(false);
+        actorIsolation = ActorIsolation::forConcurrent(false);
       } else if (auto *decl = constant->getAbstractFunctionDecl();
                  decl && decl->getExecutionBehavior().has_value()) {
         switch (*decl->getExecutionBehavior()) {
         case ExecutionKind::Concurrent:
-          actorIsolation = ActorIsolation::forNonisolated(false /*unsafe*/);
+          actorIsolation = ActorIsolation::forConcurrent(false /*unsafe*/);
           break;
         case ExecutionKind::Caller:
-          actorIsolation = ActorIsolation::forCallerIsolationInheriting();
+          actorIsolation = ActorIsolation::forNonisolated(false /*unsafe*/);
           break;
         }
       } else {

lib/SILGen/SILGen.cpp

@@ -735,7 +735,7 @@ static ActorIsolation getActorIsolationForFunction(SILFunction &fn) {
     if (constant.kind == SILDeclRef::Kind::Deallocator) {
       // Deallocating destructor is always nonisolated. Isolation of the deinit
       // applies only to isolated deallocator and destroyer.
-      return ActorIsolation::forNonisolated(false);
+      return ActorIsolation::forConcurrent(false);
     }
 
     // If we have actor isolation for our constant, put the isolation onto the

lib/SILGen/SILGenApply.cpp

@@ -3104,8 +3104,9 @@ static void emitDelayedArguments(SILGenFunction &SGF,
 
       case ActorIsolation::Unspecified:
       case ActorIsolation::Nonisolated:
-      case ActorIsolation::CallerIsolationInheriting:
       case ActorIsolation::NonisolatedUnsafe:
+      case ActorIsolation::Concurrent:
+      case ActorIsolation::ConcurrentUnsafe:
         llvm_unreachable("Not isolated");
       }
 
@@ -5832,8 +5833,9 @@ RValue SILGenFunction::emitApply(
 
     case ActorIsolation::Unspecified:
     case ActorIsolation::Nonisolated:
-    case ActorIsolation::CallerIsolationInheriting:
     case ActorIsolation::NonisolatedUnsafe:
+    case ActorIsolation::Concurrent:
+    case ActorIsolation::ConcurrentUnsafe:
       llvm_unreachable("Not isolated");
       break;
     }