Switch non-member actor isolation checking to the new implementation

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -4543,7 +4543,7 @@ ERROR(override_implicit_unowned_executor,none,
 ERROR(actor_isolated_non_self_reference,none,
         "%5 %0 %1 can not be "
         "%select{referenced|mutated|used 'inout'}2 "
-        "%select{on a different actor instance|"
+        "%select{from outside the actor|on a different actor instance|"
         "on a non-isolated actor instance|"
         "from a Sendable function|from a Sendable closure|"
         "from an 'async let' initializer|from global actor %4|"
@@ -4565,18 +4565,6 @@ ERROR(actor_isolated_inout_state,none,
 ERROR(actor_isolated_mutating_func,none,
       "cannot call mutating async function %0 on actor-isolated %1 %2",
       (DeclName, DescriptiveDeclKind, DeclName))
-ERROR(global_actor_from_instance_actor_context,none,
-      "%0 %1 isolated to global actor %2 can not be %select{referenced|mutated|used 'inout'}4"
-      " from actor %3 %select{|in a synchronous context}5",
-      (DescriptiveDeclKind, DeclName, Type, DeclName, unsigned, bool))
-ERROR(global_actor_from_other_global_actor_context,none,
-      "%0 %1 isolated to global actor %2 can not be %select{referenced|mutated|used 'inout'}4"
-      " from different global actor %3 %select{|in a synchronous context}5",
-      (DescriptiveDeclKind, DeclName, Type, Type, unsigned, bool))
-ERROR(global_actor_from_nonactor_context,none,
-      "%0 %1 isolated to global actor %2 can not be %select{referenced|mutated|used 'inout'}4"
-      " from %select{this|a non-isolated}3%select{| synchronous}5 context",
-      (DescriptiveDeclKind, DeclName, Type, bool, unsigned, bool))
 ERROR(actor_isolated_call,none,
       "call to %0 function in a synchronous %1 context",
       (ActorIsolation, ActorIsolation))

lib/Sema/TypeCheckConcurrency.cpp

@@ -1447,7 +1447,7 @@ static bool memberAccessHasSpecialPermissionInSwift5(DeclContext const *refCxt,
         member->getDescriptiveKind(),
         member->getName(),
         useKindInt,
-        baseActor.kind,
+        baseActor.kind + 1,
         baseActor.globalActor,
         getActorIsolation(const_cast<ValueDecl *>(member)))
     .warnUntilSwiftVersion(6);
@@ -1881,16 +1881,23 @@ namespace {
         recordMutableVarParent(load, load->getSubExpr());
 
       if (auto lookup = dyn_cast<LookupExpr>(expr)) {
-        checkMemberReference(lookup->getBase(), lookup->getMember(),
+        checkReference(lookup->getBase(), lookup->getMember(),
                              lookup->getLoc(),
                              /*partialApply*/None,
                              lookup);
         return { true, expr };
       }
 
       if (auto declRef = dyn_cast<DeclRefExpr>(expr)) {
-        checkNonMemberReference(
-            declRef->getDeclRef(), declRef->getLoc(), declRef);
+        auto valueRef = declRef->getDeclRef();
+        auto value = valueRef.getDecl();
+        auto loc = declRef->getLoc();
+
+        //FIXME: Should this be subsumed in reference checking?
+        if (value->isLocalCapture())
+          checkLocalCapture(valueRef, loc, declRef);
+        else
+          checkReference(nullptr, valueRef, loc, None, declRef);
         return { true, expr };
       }
 
@@ -1907,7 +1914,7 @@ namespace {
           if (auto memberRef = findMemberReference(partialApply->fn)) {
             // NOTE: partially-applied thunks are never annotated as
             // implicitly async, regardless of whether they are escaping.
-            checkMemberReference(
+            checkReference(
                 partialApply->base, memberRef->first, memberRef->second,
                 partialApply);
 
@@ -1926,7 +1933,7 @@ namespace {
       if (auto call = dyn_cast<SelfApplyExpr>(expr)) {
         Expr *fn = call->getFn()->getValueProvidingExpr();
         if (auto memberRef = findMemberReference(fn)) {
-          checkMemberReference(
+          checkReference(
               call->getBase(), memberRef->first, memberRef->second,
               /*partialApply=*/None, call);
 
@@ -2215,7 +2222,15 @@ namespace {
       if (!var || var->isLet())
         return false;
 
-      if (!var->getDeclContext()->isModuleScopeContext() && !var->isStatic())
+      if (!var->getDeclContext()->isModuleScopeContext() &&
+          !(var->getDeclContext()->isTypeContext() && !var->isInstanceMember()))
+        return false;
+
+      if (!var->hasStorage())
+        return false;
+
+      // If it's actor-isolated, it's already been dealt with.
+      if (getActorIsolation(value).isActorIsolated())
         return false;
 
       ctx.Diags.diagnose(
@@ -2604,97 +2619,6 @@ namespace {
       return false;
     }
 
-    /// Check a reference to an entity within a global actor.
-    bool checkGlobalActorReference(
-        ConcreteDeclRef valueRef, SourceLoc loc, Type globalActor,
-        bool isCrossActor,
-        Expr *context) {
-      ValueDecl *value = valueRef.getDecl();
-      auto declContext = const_cast<DeclContext *>(getDeclContext());
-
-      // Check whether we are within the same isolation context, in which
-      // case there is nothing further to check,
-      auto contextIsolation = getInnermostIsolatedContext(declContext);
-      if (contextIsolation.isGlobalActor() &&
-          contextIsolation.getGlobalActor()->isEqual(globalActor)) {
-        return false;
-      }
-
-      // A cross-actor access requires types to be concurrent-safe.
-      if (isCrossActor) {
-        return diagnoseNonSendableTypesInReference(
-            valueRef, getDeclContext(), loc,
-            SendableCheckReason::CrossActor);
-      }
-
-      // Call is implicitly asynchronous.
-      auto result = tryMarkImplicitlyAsync(
-        loc, valueRef, context,
-        ImplicitActorHopTarget::forGlobalActor(globalActor),
-        /*FIXME if we get global distributed actors*/false);
-      if (result == AsyncMarkingResult::FoundAsync)
-        return false;
-
-      // Diagnose failures.
-      switch (contextIsolation) {
-      case ActorIsolation::ActorInstance: {
-        auto useKind = static_cast<unsigned>(
-            kindOfUsage(value, context).getValueOr(VarRefUseEnv::Read));
-
-        ctx.Diags.diagnose(loc, diag::global_actor_from_instance_actor_context,
-                           value->getDescriptiveKind(), value->getName(),
-                           globalActor, contextIsolation.getActor()->getName(),
-                           useKind, result == AsyncMarkingResult::SyncContext);
-        noteIsolatedActorMember(value, context);
-        return true;
-      }
-
-      case ActorIsolation::GlobalActor:
-      case ActorIsolation::GlobalActorUnsafe: {
-        auto useKind = static_cast<unsigned>(
-            kindOfUsage(value, context).getValueOr(VarRefUseEnv::Read));
-
-        // Otherwise, this is a problematic global actor decl reference.
-        ctx.Diags.diagnose(
-            loc, diag::global_actor_from_other_global_actor_context,
-            value->getDescriptiveKind(), value->getName(), globalActor,
-            contextIsolation.getGlobalActor(), useKind,
-            result == AsyncMarkingResult::SyncContext);
-        noteIsolatedActorMember(value, context);
-        return true;
-      }
-
-      case ActorIsolation::Independent: {
-        auto useKind = static_cast<unsigned>(
-            kindOfUsage(value, context).getValueOr(VarRefUseEnv::Read));
-
-        ctx.Diags.diagnose(loc, diag::global_actor_from_nonactor_context,
-                           value->getDescriptiveKind(), value->getName(),
-                           globalActor,
-                           /*actorIndependent=*/true, useKind,
-                           result == AsyncMarkingResult::SyncContext);
-        noteIsolatedActorMember(value, context);
-        return true;
-      }
-
-      case ActorIsolation::Unspecified: {
-        // Diagnose the reference.
-        auto useKind = static_cast<unsigned>(
-            kindOfUsage(value, context).getValueOr(VarRefUseEnv::Read));
-        ctx.Diags.diagnose(
-          loc, diag::global_actor_from_nonactor_context,
-          value->getDescriptiveKind(), value->getName(), globalActor,
-          /*actorIndependent=*/false, useKind,
-          result == AsyncMarkingResult::SyncContext);
-        noteGlobalActorOnContext(declContext, globalActor);
-        noteIsolatedActorMember(value, context);
-
-        return true;
-      } // end Unspecified case
-      } // end switch
-      llvm_unreachable("unhandled actor isolation kind!");
-    }
-
     /// Find the innermost context in which this declaration was explicitly
     /// captured.
     const DeclContext *findCapturedDeclContext(ValueDecl *value) {
@@ -2874,65 +2798,38 @@ namespace {
       return diagnosed;
     }
 
-    /// Check a reference to a local or global.
-    bool checkNonMemberReference(
-        ConcreteDeclRef valueRef, SourceLoc loc, DeclRefExpr *declRefExpr) {
-      if (!valueRef)
-        return false;
-
-      auto value = valueRef.getDecl();
-
-      if (value->isLocalCapture())
-        return checkLocalCapture(valueRef, loc, declRefExpr);
-
-      switch (auto isolation =
-                  ActorIsolationRestriction::forDeclaration(
-                    valueRef, getDeclContext())) {
-      case ActorIsolationRestriction::Unrestricted:
-        return false;
-
-      case ActorIsolationRestriction::CrossActorSelf:
-      case ActorIsolationRestriction::ActorSelf:
-        llvm_unreachable("non-member reference into an actor");
-
-      case ActorIsolationRestriction::GlobalActorUnsafe:
-        // Only complain if we're in code that's adopted concurrency features.
-        if (!shouldDiagnoseExistingDataRaces(getDeclContext()))
-          return false;
-
-        LLVM_FALLTHROUGH;
-
-      case ActorIsolationRestriction::GlobalActor:
-        return checkGlobalActorReference(
-            valueRef, loc, isolation.getGlobalActor(), isolation.isCrossActor,
-            declRefExpr);
-
-      case ActorIsolationRestriction::Unsafe:
-        return diagnoseReferenceToUnsafeGlobal(value, loc);
-      }
-      llvm_unreachable("unhandled actor isolation kind!");
-    }
-
-    /// Check a reference with the given base expression to the given member.
-    /// Returns true iff the member reference refers to actor-isolated state
-    /// in an invalid or unsafe way such that a diagnostic was emitted.
-    bool checkMemberReference(
-        Expr *base, ConcreteDeclRef memberRef, SourceLoc memberLoc,
+    /// Check a reference to the given declaration.
+    ///
+    /// \param base For a reference to a member, the base expression. May be
+    /// nullptr for non-member referenced.
+    ///
+    /// \returns true if the reference is invalid, in which case a diagnostic
+    /// has already been emitted.
+    bool checkReference(
+        Expr *base, ConcreteDeclRef declRef, SourceLoc loc,
         Optional<PartialApplyThunkInfo> partialApply = None,
         Expr *context = nullptr) {
-      if (!base || !memberRef)
+      if (!declRef)
         return false;
 
-      auto member = memberRef.getDecl();
-      auto isolatedActor = getIsolatedActor(base);
+      auto decl = declRef.getDecl();
+      Optional<ReferencedActor> isolatedActor;
+      if (base)
+        isolatedActor.emplace(getIsolatedActor(base));
       auto result = ActorReferenceResult::forReference(
-          memberRef, memberLoc, getDeclContext(),
-          kindOfUsage(member, context), isolatedActor);
+          declRef, loc, getDeclContext(),
+          kindOfUsage(decl, context), isolatedActor);
       switch (result) {
       case ActorReferenceResult::SameConcurrencyDomain:
+        if (diagnoseReferenceToUnsafeGlobal(decl, loc))
+          return true;
+
         return false;
 
       case ActorReferenceResult::ExitsActorToNonisolated:
+        if (diagnoseReferenceToUnsafeGlobal(decl, loc))
+          return true;
+
         // FIXME: SE-0338 would trigger Sendable checks here.
         return false;
 
@@ -2949,24 +2846,24 @@ namespace {
 
       // A call to a global-actor-isolated function is diagnosed elsewhere.
       if (!partialApply && result.isolation.isGlobalActor() &&
-          isa<AbstractFunctionDecl>(member))
+          isa<AbstractFunctionDecl>(decl))
         return false;
 
       // An escaping partial application of something that is part of
       // the actor's isolated state is never permitted.
-      if (partialApply && partialApply->isEscaping && !isAsyncDecl(memberRef)) {
+      if (partialApply && partialApply->isEscaping && !isAsyncDecl(declRef)) {
         ctx.Diags.diagnose(
-            memberLoc, diag::actor_isolated_partial_apply,
-            member->getDescriptiveKind(),
-            member->getName());
+            loc, diag::actor_isolated_partial_apply,
+            decl->getDescriptiveKind(),
+            decl->getName());
         return true;
       }
 
       // If we do not need any async/throws/distributed checks, just perform
       // Sendable checking and we're done.
       if (!result.options) {
         return diagnoseNonSendableTypesInReference(
-                   memberRef, getDeclContext(), memberLoc,
+                   declRef, getDeclContext(), loc,
                    SendableCheckReason::CrossActor);
       }
 
@@ -2980,7 +2877,7 @@ namespace {
               result.isolation.getGlobalActor())
           : ImplicitActorHopTarget::forInstanceSelf();
       auto implicitAsyncResult = tryMarkImplicitlyAsync(
-          memberLoc, memberRef, context, target, isDistributed);
+          loc, declRef, context, target, isDistributed);
       switch (implicitAsyncResult) {
       case AsyncMarkingResult::FoundAsync:
         // Success! We're done.
@@ -2995,18 +2892,44 @@ namespace {
       case AsyncMarkingResult::NotFound:
         // Complain about access outside of the isolation domain.
         auto useKind = static_cast<unsigned>(
-            kindOfUsage(member, context).getValueOr(VarRefUseEnv::Read));
+            kindOfUsage(decl, context).getValueOr(VarRefUseEnv::Read));
+
+        ReferencedActor::Kind refKind;
+        Type refGlobalActor;
+        if (isolatedActor) {
+          refKind = isolatedActor->kind;
+          refGlobalActor = isolatedActor->globalActor;
+        } else {
+          auto contextIsolation = getInnermostIsolatedContext(getDeclContext());
+          switch (contextIsolation) {
+          case ActorIsolation::ActorInstance:
+            refKind = ReferencedActor::Isolated;
+            break;
+
+          case ActorIsolation::GlobalActor:
+          case ActorIsolation::GlobalActorUnsafe:
+            refGlobalActor = contextIsolation.getGlobalActor();
+            refKind = isMainActor(refGlobalActor)
+                ? ReferencedActor::MainActor
+                : ReferencedActor::GlobalActor;
+            break;
+
+          case ActorIsolation::Unspecified:
+          case ActorIsolation::Independent:
+            refKind = ReferencedActor::NonIsolatedContext;
+            break;
+          }
+        }
 
         ctx.Diags.diagnose(
-            memberLoc, diag::actor_isolated_non_self_reference,
-            member->getDescriptiveKind(),
-            member->getName(),
+            loc, diag::actor_isolated_non_self_reference,
+            decl->getDescriptiveKind(),
+            decl->getName(),
             useKind,
-            isolatedActor.kind,
-            isolatedActor.globalActor,
+            refKind + 1, refGlobalActor,
             result.isolation);
 
-        noteIsolatedActorMember(member, context);
+        noteIsolatedActorMember(decl, context);
 
         if (result.isolation.isGlobalActor()) {
           noteGlobalActorOnContext(

test/Concurrency/actor_call_implicitly_async.swift

@@ -278,11 +278,11 @@ func blender(_ peeler : () -> Void) {
   var money = await dollarsInBananaStand
   money -= 1200
 
-  dollarsInBananaStand = money // expected-error{{var 'dollarsInBananaStand' isolated to global actor 'BananaActor' can not be mutated from different global actor 'OrangeActor'}}
+  dollarsInBananaStand = money // expected-error{{global actor 'BananaActor'-isolated var 'dollarsInBananaStand' can not be mutated from global actor 'OrangeActor'}}
 
   // FIXME: these two errors seem a bit redundant.
   // expected-error@+2 {{actor-isolated var 'dollarsInBananaStand' cannot be passed 'inout' to implicitly 'async' function call}}
-  // expected-error@+1 {{var 'dollarsInBananaStand' isolated to global actor 'BananaActor' can not be used 'inout' from different global actor 'OrangeActor'}}
+  // expected-error@+1 {{global actor 'BananaActor'-isolated var 'dollarsInBananaStand' can not be used 'inout' from global actor 'OrangeActor'}}
   await takeInout(&dollarsInBananaStand)
 
   _ = wisk
@@ -459,7 +459,7 @@ func tryEffPropsFromSync() {
   _ = effPropA // expected-error{{'async' property access in a function that does not support concurrency}}
 
   // expected-error@+1 {{property access can throw, but it is not marked with 'try' and the error is not handled}}
-  _ = effPropT // expected-error{{var 'effPropT' isolated to global actor 'BananaActor' can not be referenced from this synchronous context}}
+  _ = effPropT // expected-error{{global actor 'BananaActor'-isolated var 'effPropT' can not be referenced from a non-isolated context}}
   // NOTE: that we don't complain about async access on `effPropT` because it's not declared async, and we're not in an async context!
 
   // expected-error@+1 {{property access can throw, but it is not marked with 'try' and the error is not handled}}